diff --git a/server/src/csrf.rs b/server/src/csrf.rs index c997a82..f40472e 100644 --- a/server/src/csrf.rs +++ b/server/src/csrf.rs @@ -101,10 +101,17 @@ where /// state-changing but carry no other user-submitted data /// (close/reopen/reset/analyze/logout). Using one shared type beats /// five wordless copy-paste structs. +/// +/// `return_to` is optional and only consulted by handlers that route +/// the user back to the source page (analyze, reset). Other handlers +/// ignore it. Validation (must start with `/web/`) lives at the call +/// site. #[derive(serde::Deserialize)] pub struct CsrfOnlyForm { #[serde(default)] pub csrf_token: String, + #[serde(default)] + pub return_to: Option, } impl HasCsrfToken for CsrfOnlyForm { diff --git a/server/src/routes/case_actions.rs b/server/src/routes/case_actions.rs index b02612a..52a6e45 100644 --- a/server/src/routes/case_actions.rs +++ b/server/src/routes/case_actions.rs @@ -43,8 +43,7 @@ pub async fn handle_analyze_case( State(analyze_tx): State, State(events_tx): State, CaseIdPath(case_id): CaseIdPath, - headers: HeaderMap, - _csrf: CsrfForm, + CsrfForm(form): CsrfForm, ) -> Result { if !config.llm_configured() { return Err(AppError::ServiceUnavailable( @@ -95,7 +94,23 @@ pub async fn handle_analyze_case( CaseEventKind::AnalysisQueued, ); - Ok(Redirect::to(&resolve_return_path(&headers))) + Ok(Redirect::to(&validated_return_path( + form.return_to, + format!("/web/cases/{case_id}"), + ))) +} + +/// Validate a `return_to` value submitted via a hidden form field. +/// +/// Must start with `/web/` to be accepted; anything else (absolute +/// URLs, schema-relative `//evil.com/...`, paths outside the web area) +/// is rejected and the supplied fallback is used. This keeps the +/// resulting redirect strictly same-origin and inside the user-facing +/// area, so a tampered hidden field cannot cause an open redirect. +fn validated_return_path(supplied: Option, fallback: String) -> String { + supplied + .filter(|s| s.starts_with("/web/")) + .unwrap_or(fallback) } /// Resolve the post-action redirect target from the `Referer` header. @@ -417,8 +432,7 @@ pub async fn handle_reset_case( State(config): State>, State(events_tx): State, CaseIdPath(case_id): CaseIdPath, - headers: HeaderMap, - _csrf: CsrfForm, + CsrfForm(form): CsrfForm, ) -> Result { if !user.is_admin() { return Err(AppError::Forbidden("Nur für Admins".into())); @@ -438,7 +452,10 @@ pub async fn handle_reset_case( case_id.to_string(), CaseEventKind::CaseReset, ); - Ok(Redirect::to(&resolve_return_path(&headers))) + Ok(Redirect::to(&validated_return_path( + form.return_to, + format!("/web/cases/{case_id}"), + ))) } #[derive(Deserialize)] @@ -558,7 +575,6 @@ pub async fn handle_delete_recording( State(config): State>, State(events_tx): State, CaseIdPath(case_id): CaseIdPath, - headers: HeaderMap, CsrfForm(form): CsrfForm, ) -> Result { validate_filename(&form.filename)?; @@ -600,7 +616,7 @@ pub async fn handle_delete_recording( CaseEventKind::RecordingDeleted, ); - Ok(Redirect::to(&resolve_return_path(&headers))) + Ok(Redirect::to(&format!("/web/cases/{case_id}/recordings"))) } async fn remove_if_exists(path: &Path) -> Result<(), AppError> { diff --git a/server/templates/case_page.html b/server/templates/case_page.html index 84200a9..89eb5f9 100644 --- a/server/templates/case_page.html +++ b/server/templates/case_page.html @@ -385,6 +385,7 @@ {% if can_analyze && !has_document %}
{% call csrf::field(csrf_token) %} +
{% else if llm_missing && !has_document %} @@ -396,6 +397,7 @@ action="/web/cases/{{ case_id }}/reset" > {% call csrf::field(csrf_token) %} + {% endif %} @@ -407,6 +409,7 @@
{% call csrf::field(csrf_token) %} +