From c2a1a53b4df1e42371adf5b74894521fd1593ba1 Mon Sep 17 00:00:00 2001 From: ospab Date: Sat, 18 Jul 2026 17:27:58 +0300 Subject: [PATCH] fix(server): audit-log API endpoints had no auth check at all GET/POST/DELETE /api/audit were the only three handlers in the whole Management API that never called check_token() - every other endpoint (status, users, rules, config) does. Concretely, with the panel's credentials configured, an unauthenticated request could still: - read the full audit log (GET) - inject arbitrary forged entries, e.g. fake "success" events to cover tracks (POST) - wipe the entire audit log (DELETE) - the exact mechanism meant to detect and investigate unauthorized actions, erasable with zero auth Added the same check_token() gate the rest of the file uses, and fixed these three handlers' raw .unwrap() on the audit_logs lock to the poison-recovery pattern (unwrap_or_else(|e| e.into_inner())) used everywhere else, for consistency. Added focused unit tests on check_token() itself (missing header, correct/ wrong bearer, raw token, session token, and the documented open-panel mode when no credentials are configured) - it's the single gate every sensitive handler depends on, worth pinning down independently of any one handler. --- ostp-server/src/api.rs | 92 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 83 insertions(+), 9 deletions(-) diff --git a/ostp-server/src/api.rs b/ostp-server/src/api.rs index 229c5be..13a2996 100644 --- a/ostp-server/src/api.rs +++ b/ostp-server/src/api.rs @@ -881,15 +881,83 @@ mod tests { let state = make_test_state(""); let _router = create_api_router(state); } + + fn headers_with_bearer(token: &str) -> axum::http::HeaderMap { + let mut h = axum::http::HeaderMap::new(); + h.insert("authorization", format!("Bearer {token}").parse().unwrap()); + h + } + + // These pin down check_token's behavior directly: it's the single gate + // every mutating/sensitive handler (including the audit-log ones - see + // the missing-auth fix) relies on, so its logic must be independently + // verified rather than only exercised incidentally through handlers. + #[test] + fn test_check_token_rejects_missing_header_when_configured() { + let state = make_test_state("panel"); + assert!(!check_token(&state, &axum::http::HeaderMap::new())); + } + + #[test] + fn test_check_token_accepts_matching_api_token_as_bearer() { + let state = make_test_state("panel"); + assert!(check_token(&state, &headers_with_bearer("test-token"))); + } + + #[test] + fn test_check_token_accepts_matching_api_token_raw() { + let state = make_test_state("panel"); + let mut h = axum::http::HeaderMap::new(); + h.insert("authorization", "test-token".parse().unwrap()); + assert!(check_token(&state, &h)); + } + + #[test] + fn test_check_token_rejects_wrong_token() { + let state = make_test_state("panel"); + assert!(!check_token(&state, &headers_with_bearer("wrong-token"))); + } + + #[test] + fn test_check_token_accepts_matching_session_token() { + let state = make_test_state("panel"); + *state.session_token.write().unwrap() = Some("live-session".to_string()); + assert!(check_token(&state, &headers_with_bearer("live-session"))); + } + + #[test] + fn test_check_token_open_when_no_credentials_configured() { + let mut state = make_test_state("panel"); + state.api_token = None; + state.username.clear(); + state.password_hash.clear(); + // Documented "unsafe but possible" open-panel mode: no credentials + // configured at all means every request passes, including with no + // Authorization header. + assert!(check_token(&state, &axum::http::HeaderMap::new())); + } } -async fn handle_get_audit(State(state): State) -> impl IntoResponse { - let logs = state.audit_logs.read().unwrap(); - ApiResponse::success(logs.clone()) +async fn handle_get_audit( + State(state): State, + headers: axum::http::HeaderMap, +) -> impl IntoResponse { + if !check_token(&state, &headers) { + return api_unauthorized::>(); + } + let logs = state.audit_logs.read().unwrap_or_else(|e| e.into_inner()); + (StatusCode::OK, ApiResponse::success(logs.clone())) } -async fn handle_create_audit(State(state): State, Json(req): Json) -> impl IntoResponse { - let mut logs = state.audit_logs.write().unwrap(); +async fn handle_create_audit( + State(state): State, + headers: axum::http::HeaderMap, + Json(req): Json, +) -> impl IntoResponse { + if !check_token(&state, &headers) { + return api_unauthorized::(); + } + let mut logs = state.audit_logs.write().unwrap_or_else(|e| e.into_inner()); let id = format!("{:x}", rand::random::()); let now = chrono::Local::now(); let entry = AuditLogEntry { @@ -904,7 +972,7 @@ async fn handle_create_audit(State(state): State, Json(req): Json) -> impl IntoResponse { - let mut logs = state.audit_logs.write().unwrap(); +async fn handle_clear_audit( + State(state): State, + headers: axum::http::HeaderMap, +) -> impl IntoResponse { + if !check_token(&state, &headers) { + return api_unauthorized::<()>(); + } + let mut logs = state.audit_logs.write().unwrap_or_else(|e| e.into_inner()); logs.clear(); - ApiResponse::success(()) + (StatusCode::OK, ApiResponse::success(())) }