From 1182f4817e3956083621d27ac51b03861e58bd44 Mon Sep 17 00:00:00 2001 From: Brummel Date: Tue, 21 Apr 2026 19:48:24 +0200 Subject: [PATCH] Enforce admin-only for bulk and purge actions Move admin checks from individual bulk actions to the main handler for `bulk.rs` and `case_actions.rs`. This consolidates the authorization logic for these sensitive operations. Additionally, refine the HTML template to conditionally render bulk action UI elements and the purge form only when the user is an admin. This ensures that non-admin users do not see or have access to these administrative functions. Update tests to reflect these changes, ensuring that non-admin users are correctly rejected for these actions and that the UI is properly hidden. --- server/src/routes/bulk.rs | 16 +++-- server/src/routes/case_actions.rs | 6 ++ server/templates/my_cases.html | 20 +++--- server/tests/analyze_test.rs | 97 ++++++++++++++++++++++++++-- server/tests/close_watermark_test.rs | 10 ++- 5 files changed, 127 insertions(+), 22 deletions(-) diff --git a/server/src/routes/bulk.rs b/server/src/routes/bulk.rs index e9c07bd..0b28f9f 100644 --- a/server/src/routes/bulk.rs +++ b/server/src/routes/bulk.rs @@ -37,6 +37,14 @@ pub async fn handle_bulk_action( State(events_tx): State, Form(form): Form, ) -> Result { + // Bulk actions are admin-only across the board: the UI hides the + // selection checkboxes and action bar from non-admins, and this + // server-side gate defends against hand-crafted POSTs. + if !user.is_admin() { + warn!(slug = %user.slug, action = %form.action, "bulk: not admin"); + return Err(AppError::Forbidden("Nur für Admins".into())); + } + let user_root = config.data_path.join(&user.slug); match form.action.as_str() { @@ -52,13 +60,7 @@ pub async fn handle_bulk_action( .await } "close" => bulk_close(&events_tx, &user.slug, &user_root, &form.case_ids).await, - "reset" => { - if !user.is_admin() { - warn!(slug = %user.slug, "bulk-reset: not admin"); - return Err(AppError::Forbidden("Nur für Admins".into())); - } - bulk_reset(&events_tx, &user.slug, &user_root, &form.case_ids).await - } + "reset" => bulk_reset(&events_tx, &user.slug, &user_root, &form.case_ids).await, other => { warn!(slug = %user.slug, action = %other, "bulk: unknown action"); return Err(AppError::BadRequest("Unbekannte Aktion".into())); diff --git a/server/src/routes/case_actions.rs b/server/src/routes/case_actions.rs index 5aa17c1..78c08e2 100644 --- a/server/src/routes/case_actions.rs +++ b/server/src/routes/case_actions.rs @@ -460,6 +460,12 @@ pub async fn handle_purge_closed( State(events_tx): State, Form(form): Form, ) -> Result { + // Destructive, irreversible — admin-only. UI hides the button for + // non-admins; this is the server-side defense-in-depth gate. + if !user.is_admin() { + warn!(slug = %user.slug, "purge-closed: not admin"); + return Err(AppError::Forbidden("Nur für Admins".into())); + } if form.confirm != "yes" { warn!(slug = %user.slug, "purge-closed: missing confirm=yes"); return Err(AppError::BadRequest("Missing confirm=yes".into())); diff --git a/server/templates/my_cases.html b/server/templates/my_cases.html index 498f3a9..b1d2f9f 100644 --- a/server/templates/my_cases.html +++ b/server/templates/my_cases.html @@ -15,7 +15,9 @@ section h2 .count { font-weight: normal; font-size: 0.85em; color: #888; } .pending { color: #888; font-style: italic; } .case-list { list-style: none; padding: 0; margin: 0; } -.case-row { display: grid; grid-template-columns: auto 1fr auto; gap: 0.6em 1em; align-items: center; padding: 0.7em 1em; border: 1px solid #ccc; border-radius: 4px; margin: 0.5em 0; } +.case-row { display: grid; grid-template-columns: 1fr auto; gap: 0.6em 1em; align-items: center; padding: 0.7em 1em; border: 1px solid #ccc; border-radius: 4px; margin: 0.5em 0; } +.case-list.has-bulk .case-row { grid-template-columns: auto 1fr auto; } +html.admin-view-off .case-list.has-bulk .case-row { grid-template-columns: 1fr auto; } .case-row.done { border-left: 4px solid #7ed321; } .case-row.open { border-left: 4px solid #4a90e2; } .case-row .check { align-self: start; padding-top: 0.25em; } @@ -101,10 +103,10 @@ try { {% for g in groups %}

{{ g.label }}{{ g.open_count }}/{{ g.total_count }} Fälle

-
{% endfor %} -
+{% if is_admin %}
Auswahl: -{% if is_admin %} -{% endif %} + + -
+
{% endif %} -{% if show_closed && any_closed %} -
+{% if is_admin && show_closed && any_closed %} + Geschlossene Fälle: diff --git a/server/tests/analyze_test.rs b/server/tests/analyze_test.rs index e4cc0b7..9a577d7 100644 --- a/server/tests/analyze_test.rs +++ b/server/tests/analyze_test.rs @@ -988,7 +988,12 @@ async fn reopen_is_noop_on_open_case() { #[tokio::test] async fn purge_closed_removes_directory() { - let config = config_with_llm(unique_tmp("purge-1"), "http://unused".into()); + // purge-closed is admin-only (matches the UI, which hides the bar). + let config = config_with_llm_users( + unique_tmp("purge-1"), + "http://unused".into(), + vec![make_admin("dr_a")], + ); let case_id = "11111111-1111-1111-1111-111111111111"; let case_dir = seed_case(&config.data_path, "dr_a", case_id); seed_recording(&case_dir, "10-00-00", Some("ok")); @@ -1016,7 +1021,11 @@ async fn purge_closed_removes_directory() { #[tokio::test] async fn purge_requires_confirm_param() { - let config = config_with_llm(unique_tmp("purge-2"), "http://unused".into()); + let config = config_with_llm_users( + unique_tmp("purge-2"), + "http://unused".into(), + vec![make_admin("dr_a")], + ); let case_id = "11111111-1111-1111-1111-111111111111"; let case_dir = seed_case(&config.data_path, "dr_a", case_id); seed_recording(&case_dir, "10-00-00", Some("ok")); @@ -1044,7 +1053,11 @@ async fn purge_requires_confirm_param() { #[tokio::test] async fn purge_closed_keeps_open_cases() { - let config = config_with_llm(unique_tmp("purge-3"), "http://unused".into()); + let config = config_with_llm_users( + unique_tmp("purge-3"), + "http://unused".into(), + vec![make_admin("dr_a")], + ); let closed_id = "11111111-1111-1111-1111-111111111111"; let open_id = "22222222-2222-2222-2222-222222222222"; let dir_closed = seed_case(&config.data_path, "dr_a", closed_id); @@ -1070,9 +1083,44 @@ async fn purge_closed_keeps_open_cases() { assert!(dir_open.exists(), "open case must survive the purge"); } +/// Regression guard: purge-closed is destructive and admin-only. A +/// non-admin POST must be rejected with 403 and leave the closed case +/// directory intact. +#[tokio::test] +async fn purge_closed_rejected_for_non_admin() { + // Default `make_user` is role=doctor. + let config = config_with_llm(unique_tmp("purge-nonadmin"), "http://unused".into()); + let case_id = "11111111-1111-1111-1111-111111111111"; + let case_dir = seed_case(&config.data_path, "dr_a", case_id); + seed_recording(&case_dir, "10-00-00", Some("ok")); + + let app = doctate_server::create_router(config); + let cookie = login(app.clone(), "dr_a").await; + + // Close the case first so there is something for purge to target. + let _ = app + .clone() + .oneshot(close_request(case_id, &cookie)) + .await + .unwrap(); + + let resp = app + .oneshot(purge_closed_request(&cookie, Some("yes"))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); + assert!(case_dir.exists(), "non-admin must not be able to purge"); +} + #[tokio::test] async fn bulk_close_shares_one_closed_at_timestamp() { - let config = config_with_llm(unique_tmp("bulk-c"), "http://unused".into()); + // Bulk actions are admin-only since the UI hides the selection + // from non-admins; the handler enforces the same gate. + let config = config_with_llm_users( + unique_tmp("bulk-c"), + "http://unused".into(), + vec![make_admin("dr_a")], + ); let case_a = "11111111-1111-1111-1111-111111111111"; let case_b = "22222222-2222-2222-2222-222222222222"; let dir_a = seed_case(&config.data_path, "dr_a", case_a); @@ -1110,7 +1158,12 @@ async fn bulk_close_shares_one_closed_at_timestamp() { #[tokio::test] async fn bulk_analyze_processes_each_selected_case() { - let config = config_with_llm(unique_tmp("bulk-a"), "http://unused".into()); + // Bulk analyze is admin-only (UI + handler). + let config = config_with_llm_users( + unique_tmp("bulk-a"), + "http://unused".into(), + vec![make_admin("dr_a")], + ); let case_a = "11111111-1111-1111-1111-111111111111"; let case_b = "22222222-2222-2222-2222-222222222222"; let dir_a = seed_case(&config.data_path, "dr_a", case_a); @@ -1300,6 +1353,40 @@ async fn bulk_reset_processes_multiple_cases_admin_only() { assert!(dir_c.join("2026-04-15T11-00-00Z.transcript.txt").exists()); } +/// Regression guard: the bulk route handles *all* actions (including +/// `close`) and is admin-only. A non-admin POST must be rejected with +/// 403 and the case must remain open. Matches the UI which no longer +/// renders the bulk bar for non-admins. +#[tokio::test] +async fn bulk_close_rejected_for_non_admin() { + let tmp = unique_tmp("bulk-close-nonadmin"); + // Default `make_user` is role=doctor. + let config = config_with_llm(tmp.clone(), "http://unused".into()); + let case_id = "11111111-1111-1111-1111-111111111111"; + let dir = seed_case(&config.data_path, "dr_a", case_id); + seed_recording(&dir, "10-00-00", Some("a")); + + let app = doctate_server::create_router(config); + let cookie = login(app.clone(), "dr_a").await; + + let body = format!("action=close&case_id={case_id}"); + let resp = app + .oneshot( + Request::builder() + .method("POST") + .uri("/web/cases/bulk") + .header(header::COOKIE, &cookie) + .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") + .body(Body::from(body)) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); + // Close marker must NOT have been written. + assert!(!dir.join(".closed").exists()); +} + // --------------------------------------------------------------------- // Ollama-style (no-auth) provider // --------------------------------------------------------------------- diff --git a/server/tests/close_watermark_test.rs b/server/tests/close_watermark_test.rs index 112eaf0..bee5ff1 100644 --- a/server/tests/close_watermark_test.rs +++ b/server/tests/close_watermark_test.rs @@ -29,6 +29,12 @@ fn make_user(slug: &str, password_plain: &str) -> User { } } +fn make_admin(slug: &str, password_plain: &str) -> User { + let mut u = make_user(slug, password_plain); + u.role = "admin".into(); + u +} + fn test_config_with_users(users: Vec) -> Arc { let data_path = std::env::temp_dir().join(format!( "doctate-close-wm-test-{}-{}", @@ -221,7 +227,9 @@ async fn reopen_bumps_watermark() { #[tokio::test] async fn bulk_close_bumps_watermark() { - let config = test_config_with_users(vec![make_user("dr_a", "s")]); + // Bulk actions are admin-only: UI hides them from non-admins and + // the handler rejects non-admin POSTs with 403. + let config = test_config_with_users(vec![make_admin("dr_a", "s")]); let data_path = config.data_path.clone(); let case_a = "770e8400-e29b-41d4-a716-446655440000"; let case_b = "880e8400-e29b-41d4-a716-446655440000";