security(search): move DELETE /search/cache to protected path

This commit is contained in:
Edouard Vanbelle
2026-07-17 00:43:15 +02:00
parent eb884f6c8f
commit dd72b77c22
6 changed files with 104 additions and 31 deletions
+7 -3
View File
@@ -9,9 +9,10 @@ OxiCloud provides authenticated file and folder search with simple query paramet
| `GET` | `/api/search/` | Simple search using query parameters | | `GET` | `/api/search/` | Simple search using query parameters |
| `POST` | `/api/search/advanced` | Advanced search with a JSON body | | `POST` | `/api/search/advanced` | Advanced search with a JSON body |
| `GET` | `/api/search/suggest` | Lightweight autocomplete suggestions | | `GET` | `/api/search/suggest` | Lightweight autocomplete suggestions |
| `DELETE` | `/api/search/cache` | Clear the search results cache | | `DELETE` | `/api/admin/search/cache` | Flush the shared search results cache (admin only) |
All search endpoints require authentication. All search endpoints require authentication. The cache flush is
additionally restricted to administrators — see [Result Caching](#result-caching).
## Simple Search Parameters ## Simple Search Parameters
@@ -59,7 +60,10 @@ Search results are cached in memory using the search criteria and user ID as the
- Cache TTL: 5 minutes - Cache TTL: 5 minutes
- Max entries: 1000 - Max entries: 1000
- Manual invalidation: `DELETE /api/search/cache` - Manual invalidation: `DELETE /api/admin/search/cache` — admin-only.
The endpoint calls `invalidate_all()` on the shared moka cache, so
one call cold-starts every subsequent search for every tenant; it's
an operator debug lever, not a per-user affordance.
## Feature Flag ## Feature Flag
+7 -2
View File
@@ -69,9 +69,14 @@ export function searchSuggest(
}); });
} }
/** Clear the server-side search cache (`DELETE /api/search/cache`). */ /**
* Clear the shared server-side search cache
* (`DELETE /api/admin/search/cache`). Admin-only — moved from
* `/api/search/cache` on 2026-07-17 because the underlying
* `invalidate_all()` touches every tenant (see AuthZ audit #14).
*/
export async function clearSearchCache(): Promise<void> { export async function clearSearchCache(): Promise<void> {
const res = await apiFetch('/api/search/cache', { const res = await apiFetch('/api/admin/search/cache', {
method: 'DELETE', method: 'DELETE',
credentials: 'same-origin' credentials: 'same-origin'
}); });
@@ -27,6 +27,7 @@ use crate::application::ports::storage_ports::StorageUsagePort;
use crate::common::di::AppState; use crate::common::di::AppState;
use crate::domain::repositories::drive_repository::DriveRepository; use crate::domain::repositories::drive_repository::DriveRepository;
use crate::domain::services::authorization::{Resource, Subject}; use crate::domain::services::authorization::{Resource, Subject};
use crate::interfaces::api::handlers::search_handler::clear_search_cache;
use crate::interfaces::errors::AppError; use crate::interfaces::errors::AppError;
use crate::interfaces::middleware::admin::require_admin; use crate::interfaces::middleware::admin::require_admin;
use std::sync::Arc; use std::sync::Arc;
@@ -89,6 +90,12 @@ pub fn admin_routes() -> Router<Arc<AppState>> {
.route("/plugins/{id}/logs/stream", get(stream_plugin_logs)) .route("/plugins/{id}/logs/stream", get(stream_plugin_logs))
.route("/plugins/{id}/retention", get(get_plugin_retention)) .route("/plugins/{id}/retention", get(get_plugin_retention))
.route("/plugins/{id}/retention", put(set_plugin_retention)) .route("/plugins/{id}/retention", put(set_plugin_retention))
// Search — operator flush of the shared moka results cache
// (AuthZ audit #14, 2026-07-16). `invalidate_all()` semantics
// touch every tenant, so this is admin-only. Lived at
// `/api/search/cache` pre-2026-07-17; the URL now declares
// its admin intent up front.
.route("/search/cache", delete(clear_search_cache))
// SMTP diagnostics // SMTP diagnostics
.route("/smtp/info", get(get_smtp_info)) .route("/smtp/info", get(get_smtp_info))
.route("/smtp/test", post(send_smtp_test)) .route("/smtp/test", post(send_smtp_test))
+40 -19
View File
@@ -1,7 +1,7 @@
use axum::{ use axum::{
extract::{Json, Query, State}, extract::{Json, Query, State},
http::StatusCode, http::{HeaderMap, StatusCode},
response::IntoResponse, response::{IntoResponse, Response},
}; };
use serde_json::json; use serde_json::json;
use tracing::{error, info}; use tracing::{error, info};
@@ -11,6 +11,8 @@ use crate::application::dtos::search_dto::{
}; };
use crate::application::ports::inbound::SearchUseCase; use crate::application::ports::inbound::SearchUseCase;
use crate::common::di::AppState; use crate::common::di::AppState;
use crate::interfaces::errors::AppError;
use crate::interfaces::middleware::admin::require_admin;
use crate::interfaces::middleware::auth::AuthUser; use crate::interfaces::middleware::auth::AuthUser;
use std::sync::Arc; use std::sync::Arc;
@@ -187,40 +189,54 @@ impl SearchHandler {
} }
} }
/// DELETE /search/cache — clears the search results cache. /// `DELETE /search/cache` — flush the shared moka search results
/// cache. Admin-only.
///
/// AuthZ audit #14 (2026-07-12): pre-fix this endpoint required
/// only a valid JWT (via the top-level auth middleware) — any
/// authenticated user, including external / magic-link accounts,
/// could DELETE it in a loop and keep the results cache cold
/// indefinitely (sustained DoS on every subsequent `/api/search`
/// query). Now gated by `require_admin` (401 for missing token,
/// 403 for non-admin caller, 200 for admin). Audit line on success
/// so operator-driven flushes are traceable in security reviews.
pub(super) async fn clear_search_cache_impl( pub(super) async fn clear_search_cache_impl(
State(state): State<Arc<AppState>>, State(state): State<Arc<AppState>>,
) -> impl IntoResponse { headers: HeaderMap,
) -> Result<Response, AppError> {
let (caller_id, _) = require_admin(&state, &headers).await?;
info!("API: Clearing search cache"); info!("API: Clearing search cache");
let search_service = match &state.applications.search_service { let Some(search_service) = &state.applications.search_service else {
Some(service) => service,
None => {
error!("Search service not available"); error!("Search service not available");
return ( return Ok((
StatusCode::SERVICE_UNAVAILABLE, StatusCode::SERVICE_UNAVAILABLE,
Json(json!({ "error": "Search service is not available" })), Json(json!({ "error": "Search service is not available" })),
) )
.into_response(); .into_response());
}
}; };
match search_service.clear_search_cache().await { match search_service.clear_search_cache().await {
Ok(_) => { Ok(_) => {
info!("Search cache cleared successfully"); tracing::info!(
( target: "audit",
event = "search.cache_cleared",
caller_id = %caller_id,
"🧹 search results cache flushed by admin",
);
Ok((
StatusCode::OK, StatusCode::OK,
Json(json!({ "message": "Search cache cleared successfully" })), Json(json!({ "message": "Search cache cleared successfully" })),
) )
.into_response() .into_response())
} }
Err(err) => { Err(err) => {
error!("Error clearing search cache: {}", err); error!("Error clearing search cache: {}", err);
( Ok((
StatusCode::INTERNAL_SERVER_ERROR, StatusCode::INTERNAL_SERVER_ERROR,
Json(json!({ "error": "Error clearing search cache" })), Json(json!({ "error": "Error clearing search cache" })),
) )
.into_response() .into_response())
} }
} }
} }
@@ -368,14 +384,19 @@ pub async fn suggest_files(
#[utoipa::path( #[utoipa::path(
delete, delete,
path = "/api/search/cache", path = "/api/admin/search/cache",
responses( responses(
(status = 200, description = "Cache cleared"), (status = 200, description = "Cache cleared"),
(status = 401, description = "Missing or invalid token"),
(status = 403, description = "Caller is not an admin"),
(status = 503, description = "Search service unavailable"), (status = 503, description = "Search service unavailable"),
), ),
security(("bearerAuth" = [])), security(("bearerAuth" = [])),
tag = "search" tag = "admin"
)] )]
pub async fn clear_search_cache(state: State<Arc<AppState>>) -> impl IntoResponse { pub async fn clear_search_cache(
SearchHandler::clear_search_cache_impl(state).await state: State<Arc<AppState>>,
headers: HeaderMap,
) -> Result<Response, AppError> {
SearchHandler::clear_search_cache_impl(state, headers).await
} }
+6 -3
View File
@@ -70,7 +70,7 @@ use crate::interfaces::api::handlers::i18n_handler::{
get_locales, get_translations_by_locale, translate, get_locales, get_translations_by_locale, translate,
}; };
use crate::interfaces::api::handlers::search_handler::{ use crate::interfaces::api::handlers::search_handler::{
clear_search_cache, search_files_get, search_files_post, suggest_files, search_files_get, search_files_post, suggest_files,
}; };
use crate::interfaces::api::handlers::trash_handler; use crate::interfaces::api::handlers::trash_handler;
@@ -275,8 +275,11 @@ pub fn create_api_routes(app_state: &Arc<AppState>) -> Router<Arc<AppState>> {
.route("/suggest", get(suggest_files)) .route("/suggest", get(suggest_files))
// Advanced search with full criteria object // Advanced search with full criteria object
.route("/advanced", post(search_files_post)) .route("/advanced", post(search_files_post))
// Clear search cache // `DELETE /api/search/cache` used to live here as a per-user-
.route("/cache", delete(clear_search_cache)) // reachable endpoint. It's an operator-only debug lever
// (moka `invalidate_all()` — nukes every tenant), so it
// moved to `/api/admin/search/cache` where the URL declares
// intent. AuthZ audit #14 (2026-07-16).
.with_state(app_state.clone()) .with_state(app_state.clone())
} else { } else {
Router::new() Router::new()
+33
View File
@@ -251,6 +251,39 @@ jsonpath "$.filtered" not exists
jsonpath "$.total" not exists jsonpath "$.total" not exists
# ─────────────────────────────────────────────────────────────
# 6b — Regression pin for AuthZ audit #14 (2026-07-12).
# `DELETE /api/admin/search/cache` calls moka `invalidate_all()`
# on the shared results cache — one call cold-starts every
# subsequent search for every tenant. Pre-fix, this lived at
# `/api/search/cache` gated only by the top-level auth
# middleware: any authenticated caller (including external /
# magic-link accounts) could DELETE it in a loop and hold the
# results cache empty indefinitely (sustained DoS). Fix: gate
# on `require_admin` AND move the URL to `/api/admin/...` so
# the taxonomy declares the intent up front. Moved 2026-07-17.
#
# Bob (regular user) → 403; missing token → 401; admin → 200.
# The 200 confirms the admin path still works (no regression
# on the operator debug lever the endpoint remains for).
# ─────────────────────────────────────────────────────────────
DELETE {{base_url}}/api/admin/search/cache
Authorization: Bearer {{bob_token}}
HTTP 403
DELETE {{base_url}}/api/admin/search/cache
HTTP 401
DELETE {{base_url}}/api/admin/search/cache
Authorization: Bearer {{admin_token}}
HTTP 200
# ───────────────────────────────────────────────────────────── # ─────────────────────────────────────────────────────────────
# 7 — Teardown: removing the folder recursively takes the files # 7 — Teardown: removing the folder recursively takes the files
# with it, so a single DELETE is enough. # with it, so a single DELETE is enough.