refactor: drop /web/ URL prefix from browser routes
The /web/ prefix predated the /api/ split; today it just clutters every URL
without disambiguating anything. All 16 browser routes move to the apex
(/cases, /login, /magic, /events, /audio/...). The 6 /api/* routes are
unchanged. A new /->>/cases redirect closes the apex 404.
The open-redirect guard in magic.rs and case_actions.rs flips from a
positive whitelist (starts_with("/web/")) to a deny-list: same-origin path,
not protocol-relative, not under /api/, no \. The /api/ exclusion is now
load-bearing and covered by tests.
Pre-production: no transition redirects.
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
//! Opportunistic auto-trigger for LLM analysis.
|
||||
//!
|
||||
//! Called by the `/web/cases` and `/web/cases/{id}` handlers on every
|
||||
//! Called by the `/cases` and `/cases/{id}` handlers on every
|
||||
//! request. The SSE-driven `location.reload()` in the UI turns every
|
||||
//! relevant state change (upload, transcript, document) into a fresh
|
||||
//! handler invocation, so no background timer is needed.
|
||||
@@ -154,7 +154,7 @@ pub async fn try_enqueue(case_dir: &Path, tx: &AnalyzeSender, events_tx: &EventS
|
||||
|
||||
/// Iterate over every non-deleted UUID-named case directory under
|
||||
/// `user_root` and run `try_enqueue` on each. Safe to call on every
|
||||
/// `/web/cases` render — skips are cheap filesystem stats.
|
||||
/// `/cases` render — skips are cheap filesystem stats.
|
||||
pub async fn try_enqueue_all_for_user(
|
||||
user_root: &Path,
|
||||
tx: &AnalyzeSender,
|
||||
|
||||
+5
-5
@@ -70,7 +70,7 @@ where
|
||||
}
|
||||
|
||||
/// Extracted from the session cookie on web UI requests.
|
||||
/// On missing/expired session, returns `AppError::Redirect("/web/login")`.
|
||||
/// On missing/expired session, returns `AppError::Redirect("/login")`.
|
||||
pub struct AuthenticatedWebUser {
|
||||
pub slug: String,
|
||||
pub role: String,
|
||||
@@ -112,7 +112,7 @@ where
|
||||
let token = jar
|
||||
.get(SESSION_COOKIE)
|
||||
.map(|c| c.value().to_owned())
|
||||
.ok_or_else(|| AppError::Redirect("/web/login".into()))?;
|
||||
.ok_or_else(|| AppError::Redirect("/login".into()))?;
|
||||
|
||||
// Read lock first to check validity. If expired, upgrade to write and remove.
|
||||
let expired = {
|
||||
@@ -121,7 +121,7 @@ where
|
||||
Some(s) => s.expires_at <= Instant::now(),
|
||||
None => {
|
||||
warn!(token_prefix = %token_prefix(&token), "session lookup failed (unknown or already expired)");
|
||||
return Err(AppError::Redirect("/web/login".into()));
|
||||
return Err(AppError::Redirect("/login".into()));
|
||||
}
|
||||
}
|
||||
};
|
||||
@@ -132,13 +132,13 @@ where
|
||||
if let Some(s) = removed {
|
||||
warn!(slug = %s.slug, "session expired");
|
||||
}
|
||||
return Err(AppError::Redirect("/web/login".into()));
|
||||
return Err(AppError::Redirect("/login".into()));
|
||||
}
|
||||
|
||||
let r = store.read().await;
|
||||
let session = r
|
||||
.get(&token)
|
||||
.ok_or_else(|| AppError::Redirect("/web/login".into()))?;
|
||||
.ok_or_else(|| AppError::Redirect("/login".into()))?;
|
||||
|
||||
let data_dir = config.data_path.join(&session.slug);
|
||||
// Look up the TOML user record for the policy fields the session
|
||||
|
||||
+8
-8
@@ -1,4 +1,4 @@
|
||||
//! CSRF-token validation for state-changing POSTs under `/web/`.
|
||||
//! CSRF-token validation for state-changing browser POSTs.
|
||||
//!
|
||||
//! The `WebSession` carries a `csrf_token` minted once at login or
|
||||
//! magic-link consume. Browser forms include a hidden `csrf_token`
|
||||
@@ -32,12 +32,12 @@ pub trait HasCsrfToken {
|
||||
}
|
||||
|
||||
/// Wrapping extractor — a drop-in replacement for `Form<T>` on
|
||||
/// state-changing `/web/` POST handlers. Validates the form body's
|
||||
/// state-changing browser POST handlers. Validates the form body's
|
||||
/// `csrf_token` against the session's csrf_token via a constant-time
|
||||
/// comparison.
|
||||
///
|
||||
/// Rejection mapping:
|
||||
/// - Missing or expired session → 302 redirect to `/web/login` (so a
|
||||
/// - Missing or expired session → 302 redirect to `/login` (so a
|
||||
/// second-tab logout shows "bitte neu anmelden", not "csrf failed").
|
||||
/// - Form body malformed → 400 `BadRequest`.
|
||||
/// - Token present but mismatched → 403 `Forbidden`.
|
||||
@@ -60,15 +60,15 @@ where
|
||||
let cookie_token = CookieJar::from_headers(req.headers())
|
||||
.get(SESSION_COOKIE)
|
||||
.map(|c| c.value().to_owned())
|
||||
.ok_or_else(|| AppError::Redirect("/web/login".into()))?;
|
||||
.ok_or_else(|| AppError::Redirect("/login".into()))?;
|
||||
|
||||
let expected = {
|
||||
let r = store.read().await;
|
||||
let session = r
|
||||
.get(&cookie_token)
|
||||
.ok_or_else(|| AppError::Redirect("/web/login".into()))?;
|
||||
.ok_or_else(|| AppError::Redirect("/login".into()))?;
|
||||
if session.expires_at <= Instant::now() {
|
||||
return Err(AppError::Redirect("/web/login".into()));
|
||||
return Err(AppError::Redirect("/login".into()));
|
||||
}
|
||||
session.csrf_token.clone()
|
||||
};
|
||||
@@ -104,8 +104,8 @@ where
|
||||
///
|
||||
/// `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.
|
||||
/// ignore it. Validation (same-origin, not under `/api/`) lives at the
|
||||
/// call site.
|
||||
#[derive(serde::Deserialize)]
|
||||
pub struct CsrfOnlyForm {
|
||||
#[serde(default)]
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
//! Broadcast bus for UI live-update events.
|
||||
//!
|
||||
//! The web UI subscribes via the `/web/events` SSE route (see
|
||||
//! The web UI subscribes via the `/events` SSE route (see
|
||||
//! `routes::events`). Background workers and request handlers call
|
||||
//! [`emit`] whenever they mutate case data on disk; subscribed browsers
|
||||
//! then schedule a debounced reload. The filesystem remains the source
|
||||
|
||||
+2
-2
@@ -207,8 +207,8 @@ pub fn create_router(config: Arc<Config>) -> Router {
|
||||
/// Same as [`create_router`], but also returns a handle to the newly
|
||||
/// created session store so tests can peek at `WebSession::csrf_token`
|
||||
/// values without rendering and parsing HTML. The store handle is
|
||||
/// shared with the router (`Arc`), so any session created via `/web/login`
|
||||
/// or `/web/magic` becomes observable through it.
|
||||
/// shared with the router (`Arc`), so any session created via `/login`
|
||||
/// or `/magic` becomes observable through it.
|
||||
pub fn create_router_and_session_store(config: Arc<Config>) -> (Router, SessionStore) {
|
||||
create_router_and_session_store_with_settings(config, Arc::new(Settings::default()))
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
//! browser session without an interactive password login.
|
||||
//!
|
||||
//! Lifecycle: the desktop client calls `POST /api/auth/magic-link` (API-key
|
||||
//! authenticated), gets a token, opens the browser at `/web/magic?token=…`.
|
||||
//! authenticated), gets a token, opens the browser at `/magic?token=…`.
|
||||
//! The web handler removes the token (one-time-use) and installs a regular
|
||||
//! `WebSession`. TTL is intentionally short — the token only needs to
|
||||
//! survive the click → browser-launch → first-request round trip.
|
||||
|
||||
+1
-1
@@ -131,7 +131,7 @@ async fn main() {
|
||||
});
|
||||
|
||||
// Live-update bus: background workers and handlers publish case
|
||||
// events here; the SSE route at `/web/events` fans them out to
|
||||
// events here; the SSE route at `/events` fans them out to
|
||||
// subscribed browsers.
|
||||
let events_tx = events::channel();
|
||||
|
||||
|
||||
@@ -39,7 +39,7 @@ impl HasCsrfToken for BulkForm {
|
||||
}
|
||||
}
|
||||
|
||||
/// POST /web/cases/bulk — apply `action` (analyze | close | reset) to
|
||||
/// POST /cases/bulk — apply `action` (analyze | close | reset) to
|
||||
/// every selected case. Errors per case are logged but do not abort the
|
||||
/// batch: a single bad/missing case must not block the rest.
|
||||
pub async fn handle_bulk_action(
|
||||
@@ -85,7 +85,7 @@ pub async fn handle_bulk_action(
|
||||
}
|
||||
}
|
||||
|
||||
Ok(Redirect::to("/web/cases"))
|
||||
Ok(Redirect::to("/cases"))
|
||||
}
|
||||
|
||||
async fn bulk_analyze(
|
||||
|
||||
@@ -33,7 +33,7 @@ use crate::paths::{
|
||||
use crate::routes::user_web::locate_case_or_404;
|
||||
use crate::routes::web::validate_filename;
|
||||
|
||||
/// Form body for `POST /web/cases/{id}/analyze`. The `backend` value is set
|
||||
/// Form body for `POST /cases/{id}/analyze`. The `backend` value is set
|
||||
/// by the clicked submit button (`<button name="backend" value="…">`); an
|
||||
/// empty value falls back to the default backend.
|
||||
#[derive(Deserialize)]
|
||||
@@ -52,7 +52,7 @@ impl HasCsrfToken for AnalyzeForm {
|
||||
}
|
||||
}
|
||||
|
||||
/// POST /web/cases/{case_id}/analyze
|
||||
/// POST /cases/{case_id}/analyze
|
||||
///
|
||||
/// Trigger an LLM analysis for the case. If no document exists yet, writes
|
||||
/// `analysis_input_v1.json`; otherwise writes `analysis_input_v{N+1}.json`
|
||||
@@ -149,30 +149,32 @@ pub async fn handle_analyze_case(
|
||||
|
||||
Ok(Redirect::to(&validated_return_path(
|
||||
form.return_to,
|
||||
format!("/web/cases/{case_id}"),
|
||||
format!("/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)
|
||||
/// Accepts only same-origin browser paths (single leading slash, not
|
||||
/// protocol-relative, not under `/api/`, no `\`). Anything else
|
||||
/// (absolute URLs, `//evil.com/...`, JSON endpoints, traversal hacks)
|
||||
/// 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<String>, fallback: String) -> String {
|
||||
supplied
|
||||
.filter(|s| s.starts_with("/web/"))
|
||||
.filter(|s| is_safe_browser_path(s))
|
||||
.unwrap_or(fallback)
|
||||
}
|
||||
|
||||
/// Resolve the post-action redirect target from the `Referer` header.
|
||||
///
|
||||
/// Extracts only the path component and requires it to start with `/web/`.
|
||||
/// Schema and host are dropped entirely, so the resulting redirect is always
|
||||
/// Extracts only the path component and applies the same same-origin
|
||||
/// browser-path predicate as `validated_return_path`. Schema and host
|
||||
/// are dropped entirely, so the resulting redirect is always
|
||||
/// same-origin — a hostile `Referer` cannot cause an open redirect.
|
||||
fn resolve_return_path(headers: &HeaderMap) -> String {
|
||||
const FALLBACK: &str = "/web/cases";
|
||||
const FALLBACK: &str = "/cases";
|
||||
let Some(referer) = headers
|
||||
.get(axum::http::header::REFERER)
|
||||
.and_then(|v| v.to_str().ok())
|
||||
@@ -188,22 +190,47 @@ fn resolve_return_path(headers: &HeaderMap) -> String {
|
||||
},
|
||||
None => referer,
|
||||
};
|
||||
if path_and_query.starts_with("/web/") {
|
||||
// The predicate looks at the path part only; a `?query` tail is fine
|
||||
// because none of the deny-rules trigger on `?`.
|
||||
if is_safe_browser_path(path_and_query) {
|
||||
path_and_query.to_string()
|
||||
} else {
|
||||
FALLBACK.into()
|
||||
}
|
||||
}
|
||||
|
||||
/// Like `resolve_return_path`, but collapses a `/web/cases/{uuid}` detail
|
||||
/// path to `/web/cases` while preserving the query string. Used by
|
||||
/// Same-origin browser-path predicate. Mirrors `magic::is_safe_return_to`
|
||||
/// in spirit but is duplicated locally — the two call sites operate on
|
||||
/// different input shapes and are independent enough that a shared
|
||||
/// helper would be premature abstraction.
|
||||
fn is_safe_browser_path(path: &str) -> bool {
|
||||
if path.is_empty() || path == "/" {
|
||||
return false;
|
||||
}
|
||||
if !path.starts_with('/') {
|
||||
return false;
|
||||
}
|
||||
if path.starts_with("//") {
|
||||
return false;
|
||||
}
|
||||
if path.starts_with("/api/") {
|
||||
return false;
|
||||
}
|
||||
if path.contains('\\') {
|
||||
return false;
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// Like `resolve_return_path`, but collapses a `/cases/{uuid}` detail
|
||||
/// path to `/cases` while preserving the query string. Used by
|
||||
/// handlers that remove the case from the detail view (close, purge) —
|
||||
/// a redirect back to the now-404 detail page would be a dead end, but
|
||||
/// losing `?show_closed=1` would eject the user from the show-closed
|
||||
/// view they were in.
|
||||
fn resolve_list_return_path(headers: &HeaderMap) -> String {
|
||||
let raw = resolve_return_path(headers);
|
||||
let Some(tail) = raw.strip_prefix("/web/cases/") else {
|
||||
let Some(tail) = raw.strip_prefix("/cases/") else {
|
||||
return raw;
|
||||
};
|
||||
let (case_part, query) = match tail.split_once('?') {
|
||||
@@ -214,8 +241,8 @@ fn resolve_list_return_path(headers: &HeaderMap) -> String {
|
||||
return raw;
|
||||
}
|
||||
match query {
|
||||
Some(q) => format!("/web/cases?{q}"),
|
||||
None => "/web/cases".into(),
|
||||
Some(q) => format!("/cases?{q}"),
|
||||
None => "/cases".into(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -407,7 +434,7 @@ pub(crate) async fn reset_case_artefacts(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// POST /web/cases/{case_id}/close
|
||||
/// POST /cases/{case_id}/close
|
||||
///
|
||||
/// Close the case: writes a `.closed` JSON marker into the case directory.
|
||||
/// The case immediately disappears from listings and detail views (404).
|
||||
@@ -451,7 +478,7 @@ pub async fn handle_close_case(
|
||||
Ok(Redirect::to(&resolve_list_return_path(&headers)))
|
||||
}
|
||||
|
||||
/// POST /web/cases/{case_id}/reopen
|
||||
/// POST /cases/{case_id}/reopen
|
||||
///
|
||||
/// Reopen a closed case: removes the `.closed` marker. The case is
|
||||
/// immediately back in the default listing. A reopened case is
|
||||
@@ -504,7 +531,7 @@ pub async fn handle_reopen_case(
|
||||
Ok(Redirect::to(&return_path))
|
||||
}
|
||||
|
||||
/// POST /web/cases/{case_id}/reset
|
||||
/// POST /cases/{case_id}/reset
|
||||
///
|
||||
/// Admin-only. Wipes all derived artefacts (transcripts, oneliner,
|
||||
/// analysis input, document) and un-fails audio (`.m4a.failed` → `.m4a`)
|
||||
@@ -539,7 +566,7 @@ pub async fn handle_reset_case(
|
||||
);
|
||||
Ok(Redirect::to(&validated_return_path(
|
||||
form.return_to,
|
||||
format!("/web/cases/{case_id}"),
|
||||
format!("/cases/{case_id}"),
|
||||
)))
|
||||
}
|
||||
|
||||
@@ -559,7 +586,7 @@ impl HasCsrfToken for PurgeClosedForm {
|
||||
}
|
||||
}
|
||||
|
||||
/// POST /web/cases/purge-closed
|
||||
/// POST /cases/purge-closed
|
||||
///
|
||||
/// Hard-delete every case under the user's data dir that carries a
|
||||
/// `.closed` marker. Requires form field `confirm=yes` as a server-side
|
||||
@@ -593,7 +620,7 @@ pub async fn handle_purge_closed(
|
||||
Ok(r) => r,
|
||||
Err(e) => {
|
||||
warn!(slug = %user.slug, error = %e, "purge-closed: read_dir failed");
|
||||
return Ok(Redirect::to("/web/cases"));
|
||||
return Ok(Redirect::to("/cases"));
|
||||
}
|
||||
};
|
||||
while let Ok(Some(entry)) = entries.next_entry().await {
|
||||
@@ -626,7 +653,7 @@ pub async fn handle_purge_closed(
|
||||
ok += 1;
|
||||
}
|
||||
info!(slug = %user.slug, ok, skipped, "purge-closed completed");
|
||||
Ok(Redirect::to("/web/cases"))
|
||||
Ok(Redirect::to("/cases"))
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
@@ -642,7 +669,7 @@ impl HasCsrfToken for DeleteRecordingForm {
|
||||
}
|
||||
}
|
||||
|
||||
/// POST /web/cases/{case_id}/recordings/delete
|
||||
/// POST /cases/{case_id}/recordings/delete
|
||||
///
|
||||
/// Hard-delete a single recording plus its transcript / duration sidecars
|
||||
/// and invalidate derived artefacts (`oneliner.json`, `document.md`,
|
||||
@@ -706,7 +733,7 @@ pub async fn handle_delete_recording(
|
||||
CaseEventKind::RecordingDeleted,
|
||||
);
|
||||
|
||||
Ok(Redirect::to(&format!("/web/cases/{case_id}/recordings")))
|
||||
Ok(Redirect::to(&format!("/cases/{case_id}/recordings")))
|
||||
}
|
||||
|
||||
async fn remove_if_exists(path: &Path) -> Result<(), AppError> {
|
||||
@@ -733,52 +760,70 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn no_referer_falls_back_to_case_list() {
|
||||
assert_eq!(resolve_return_path(&HeaderMap::new()), "/web/cases");
|
||||
assert_eq!(resolve_return_path(&HeaderMap::new()), "/cases");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn absolute_referer_returns_path_only() {
|
||||
let h = headers_with_referer("https://doctate.internal/web/cases/ABCD");
|
||||
assert_eq!(resolve_return_path(&h), "/web/cases/ABCD");
|
||||
let h = headers_with_referer("https://doctate.internal/cases/ABCD");
|
||||
assert_eq!(resolve_return_path(&h), "/cases/ABCD");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn query_string_is_preserved() {
|
||||
let h = headers_with_referer("https://doctate.internal/web/cases/ABCD?x=1");
|
||||
assert_eq!(resolve_return_path(&h), "/web/cases/ABCD?x=1");
|
||||
let h = headers_with_referer("https://doctate.internal/cases/ABCD?x=1");
|
||||
assert_eq!(resolve_return_path(&h), "/cases/ABCD?x=1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hostile_host_is_stripped() {
|
||||
// Open-redirect protection: the host component is discarded; only the
|
||||
// path is reused. A crafted referer cannot redirect off-site.
|
||||
let h = headers_with_referer("https://evil.example/web/cases/ABCD");
|
||||
assert_eq!(resolve_return_path(&h), "/web/cases/ABCD");
|
||||
let h = headers_with_referer("https://evil.example/cases/ABCD");
|
||||
assert_eq!(resolve_return_path(&h), "/cases/ABCD");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_web_path_falls_back() {
|
||||
fn arbitrary_browser_path_is_accepted() {
|
||||
// The predicate is a deny-list: any same-origin path that is not
|
||||
// under `/api/` and not protocol-relative passes through. A future
|
||||
// route under `/admin/...` would not need a code change here.
|
||||
let h = headers_with_referer("https://doctate.internal/admin/secret");
|
||||
assert_eq!(resolve_return_path(&h), "/web/cases");
|
||||
assert_eq!(resolve_return_path(&h), "/admin/secret");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn api_referer_falls_back() {
|
||||
// A Referer that points at a JSON endpoint must not become the
|
||||
// redirect target — would land the user on a JSON response.
|
||||
let h = headers_with_referer("https://doctate.internal/api/upload");
|
||||
assert_eq!(resolve_return_path(&h), "/cases");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn protocol_relative_referer_falls_back() {
|
||||
// `//evil.example/...` is protocol-relative — would jump host.
|
||||
let h = headers_with_referer("//evil.example/cases/ABCD");
|
||||
assert_eq!(resolve_return_path(&h), "/cases");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn relative_referer_passes_through() {
|
||||
let h = headers_with_referer("/web/cases/XYZ");
|
||||
assert_eq!(resolve_return_path(&h), "/web/cases/XYZ");
|
||||
let h = headers_with_referer("/cases/XYZ");
|
||||
assert_eq!(resolve_return_path(&h), "/cases/XYZ");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bare_host_without_path_falls_back() {
|
||||
let h = headers_with_referer("https://doctate.internal");
|
||||
assert_eq!(resolve_return_path(&h), "/web/cases");
|
||||
assert_eq!(resolve_return_path(&h), "/cases");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_ascii_header_falls_back() {
|
||||
let mut h = HeaderMap::new();
|
||||
h.insert(REFERER, HeaderValue::from_bytes(&[0xff, 0xfe]).unwrap());
|
||||
assert_eq!(resolve_return_path(&h), "/web/cases");
|
||||
assert_eq!(resolve_return_path(&h), "/cases");
|
||||
}
|
||||
|
||||
// `resolve_list_return_path` is `resolve_return_path` plus a case-
|
||||
@@ -787,27 +832,26 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn list_return_path_preserves_list_referer() {
|
||||
let h = headers_with_referer("/web/cases?show_closed=1");
|
||||
assert_eq!(resolve_list_return_path(&h), "/web/cases?show_closed=1");
|
||||
let h = headers_with_referer("/cases?show_closed=1");
|
||||
assert_eq!(resolve_list_return_path(&h), "/cases?show_closed=1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn list_return_path_collapses_detail_with_query() {
|
||||
let h =
|
||||
headers_with_referer("/web/cases/11111111-1111-1111-1111-111111111111?show_closed=1");
|
||||
assert_eq!(resolve_list_return_path(&h), "/web/cases?show_closed=1");
|
||||
let h = headers_with_referer("/cases/11111111-1111-1111-1111-111111111111?show_closed=1");
|
||||
assert_eq!(resolve_list_return_path(&h), "/cases?show_closed=1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn list_return_path_collapses_bare_detail() {
|
||||
let h = headers_with_referer("/web/cases/11111111-1111-1111-1111-111111111111");
|
||||
assert_eq!(resolve_list_return_path(&h), "/web/cases");
|
||||
let h = headers_with_referer("/cases/11111111-1111-1111-1111-111111111111");
|
||||
assert_eq!(resolve_list_return_path(&h), "/cases");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn list_return_path_leaves_non_uuid_segment_alone() {
|
||||
// Non-UUID `/web/cases/<x>` is not a detail page — don't touch it.
|
||||
let h = headers_with_referer("/web/cases/undo-delete");
|
||||
assert_eq!(resolve_list_return_path(&h), "/web/cases/undo-delete");
|
||||
// Non-UUID `/cases/<x>` is not a detail page — don't touch it.
|
||||
let h = headers_with_referer("/cases/undo-delete");
|
||||
assert_eq!(resolve_list_return_path(&h), "/cases/undo-delete");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,7 +18,7 @@ use tokio_stream::wrappers::errors::BroadcastStreamRecvError;
|
||||
use crate::auth::AuthenticatedWebUser;
|
||||
use crate::events::EventSender;
|
||||
|
||||
/// `GET /web/events` — `text/event-stream`, one connection per tab.
|
||||
/// `GET /events` — `text/event-stream`, one connection per tab.
|
||||
pub async fn handle_events(
|
||||
user: AuthenticatedWebUser,
|
||||
State(events_tx): State<EventSender>,
|
||||
|
||||
@@ -90,7 +90,7 @@ pub async fn handle_login_submit(
|
||||
|
||||
let cookie = build_session_cookie(token, ttl, config.cookie_secure);
|
||||
let jar = jar.add(cookie);
|
||||
Ok((jar, Redirect::to("/web/cases")).into_response())
|
||||
Ok((jar, Redirect::to("/cases")).into_response())
|
||||
}
|
||||
|
||||
pub async fn handle_logout(
|
||||
@@ -113,7 +113,7 @@ pub async fn handle_logout(
|
||||
build_session_cookie(String::new(), Duration::from_secs(0), config.cookie_secure);
|
||||
cookie.make_removal();
|
||||
let jar = jar.add(cookie);
|
||||
Ok((jar, Redirect::to("/web/login")).into_response())
|
||||
Ok((jar, Redirect::to("/login")).into_response())
|
||||
}
|
||||
|
||||
fn render_login(error: Option<&'static str>) -> Result<Html<String>, AppError> {
|
||||
|
||||
+25
-17
@@ -6,8 +6,8 @@
|
||||
//! desired `return_to` path. Server stores a one-time token (TTL 60s)
|
||||
//! and returns it.
|
||||
//! 2. Desktop client opens the system browser at
|
||||
//! `{server_url}/web/magic?token=…`.
|
||||
//! 3. The browser hits `GET /web/magic`. The server consumes the token
|
||||
//! `{server_url}/magic?token=…`.
|
||||
//! 3. The browser hits `GET /magic`. The server consumes the token
|
||||
//! (one-time-use), installs a regular `WebSession`, sets the session
|
||||
//! cookie, and 303-redirects to `return_to`.
|
||||
|
||||
@@ -34,7 +34,7 @@ use crate::web_session::{SessionStore, WebSession, build_session_cookie, generat
|
||||
const MAGIC_LINK_TTL_SECS: u64 = 60;
|
||||
|
||||
/// Default landing page when the desktop client does not specify one.
|
||||
const DEFAULT_RETURN_TO: &str = "/web/cases";
|
||||
const DEFAULT_RETURN_TO: &str = "/cases";
|
||||
|
||||
#[derive(Deserialize, Default)]
|
||||
pub struct MagicLinkRequest {
|
||||
@@ -60,12 +60,13 @@ pub async fn handle_create(
|
||||
.return_to
|
||||
.unwrap_or_else(|| DEFAULT_RETURN_TO.to_owned());
|
||||
|
||||
// Open-redirect guard: only accept paths under our own /web/ tree.
|
||||
// Reject schemes, hosts, protocol-relative URLs, and traversal.
|
||||
// Open-redirect guard: only accept same-origin browser paths.
|
||||
// Reject schemes, hosts, protocol-relative URLs, traversal, and
|
||||
// anything under `/api/` (would land the user on a JSON endpoint).
|
||||
if !is_safe_return_to(&return_to) {
|
||||
warn!(slug = %user.slug, return_to = %return_to, "magic-link rejected: unsafe return_to");
|
||||
return Err(AppError::BadRequest(
|
||||
"return_to must be a path under /web/".into(),
|
||||
"return_to must be a same-origin browser path".into(),
|
||||
));
|
||||
}
|
||||
|
||||
@@ -162,24 +163,28 @@ pub async fn handle_consume(
|
||||
fn redirect_to_login() -> Response {
|
||||
Response::builder()
|
||||
.status(StatusCode::SEE_OTHER)
|
||||
.header(header::LOCATION, "/web/login")
|
||||
.header(header::LOCATION, "/login")
|
||||
.header(header::REFERRER_POLICY, "no-referrer")
|
||||
.body(axum::body::Body::empty())
|
||||
.expect("static redirect response")
|
||||
}
|
||||
|
||||
/// Allow only same-origin paths under `/web/`. Rejects schemes, host
|
||||
/// authorities, protocol-relative URLs (`//evil.com/...`), and the
|
||||
/// trivially malformed empty string.
|
||||
/// Allow only same-origin browser paths. Rejects schemes, host
|
||||
/// authorities, protocol-relative URLs (`//evil.com/...`), the bare
|
||||
/// root and empty string, anything under `/api/` (JSON endpoints
|
||||
/// must never be a magic-link target), and `\` traversal hacks.
|
||||
fn is_safe_return_to(path: &str) -> bool {
|
||||
if path.is_empty() {
|
||||
if path.is_empty() || path == "/" {
|
||||
return false;
|
||||
}
|
||||
if !path.starts_with('/') {
|
||||
return false; // schemes, hosts, relative paths
|
||||
}
|
||||
if path.starts_with("//") {
|
||||
return false; // protocol-relative — would jump host
|
||||
}
|
||||
if !path.starts_with("/web/") {
|
||||
return false;
|
||||
if path.starts_with("/api/") {
|
||||
return false; // keep magic-links inside the browser-facing area
|
||||
}
|
||||
// Defensive: a `\` anywhere can confuse some legacy parsers into
|
||||
// treating the rest as an authority. Reject outright.
|
||||
@@ -194,11 +199,12 @@ mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn safe_return_to_accepts_web_paths() {
|
||||
assert!(is_safe_return_to("/web/cases"));
|
||||
fn safe_return_to_accepts_browser_paths() {
|
||||
assert!(is_safe_return_to("/cases"));
|
||||
assert!(is_safe_return_to(
|
||||
"/web/cases/123e4567-e89b-12d3-a456-426614174000"
|
||||
"/cases/123e4567-e89b-12d3-a456-426614174000"
|
||||
));
|
||||
assert!(is_safe_return_to("/login"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -208,6 +214,8 @@ mod tests {
|
||||
assert!(!is_safe_return_to("//evil.com/path"));
|
||||
assert!(!is_safe_return_to("https://evil.com/"));
|
||||
assert!(!is_safe_return_to("/api/upload"));
|
||||
assert!(!is_safe_return_to("/web\\evil"));
|
||||
assert!(!is_safe_return_to("/api/cases/abc/oneliner"));
|
||||
assert!(!is_safe_return_to("/cases\\evil"));
|
||||
assert!(!is_safe_return_to("relative/path"));
|
||||
}
|
||||
}
|
||||
|
||||
+18
-19
@@ -12,12 +12,14 @@ pub(crate) mod user_web;
|
||||
pub(crate) mod web;
|
||||
|
||||
use axum::Router;
|
||||
use axum::response::Redirect;
|
||||
use axum::routing::{get, post, put};
|
||||
|
||||
use crate::AppState;
|
||||
|
||||
pub fn api_router() -> Router<AppState> {
|
||||
Router::new()
|
||||
.route("/", get(|| async { Redirect::permanent("/cases") }))
|
||||
.route("/api/health", get(health::handle_health))
|
||||
.route("/api/debug/whoami", get(debug::handle_whoami))
|
||||
.route("/api/upload", post(upload::handle_upload))
|
||||
@@ -27,50 +29,47 @@ pub fn api_router() -> Router<AppState> {
|
||||
put(oneliner_override::handle_put_oneliner),
|
||||
)
|
||||
.route("/api/auth/magic-link", post(magic::handle_create))
|
||||
.route("/web/magic", get(magic::handle_consume))
|
||||
.route("/magic", get(magic::handle_consume))
|
||||
.route(
|
||||
"/web/login",
|
||||
"/login",
|
||||
get(login::handle_login_page).post(login::handle_login_submit),
|
||||
)
|
||||
.route("/web/logout", post(login::handle_logout))
|
||||
.route("/web/cases", get(user_web::handle_my_cases))
|
||||
.route("/web/cases/{case_id}", get(user_web::handle_case_page))
|
||||
.route("/logout", post(login::handle_logout))
|
||||
.route("/cases", get(user_web::handle_my_cases))
|
||||
.route("/cases/{case_id}", get(user_web::handle_case_page))
|
||||
.route(
|
||||
"/web/cases/{case_id}/recordings",
|
||||
"/cases/{case_id}/recordings",
|
||||
get(user_web::handle_case_recordings),
|
||||
)
|
||||
.route(
|
||||
"/web/cases/{case_id}/recordings/delete",
|
||||
"/cases/{case_id}/recordings/delete",
|
||||
post(case_actions::handle_delete_recording),
|
||||
)
|
||||
.route(
|
||||
"/web/cases/{case_id}/analyze",
|
||||
"/cases/{case_id}/analyze",
|
||||
post(case_actions::handle_analyze_case),
|
||||
)
|
||||
.route(
|
||||
"/web/cases/{case_id}/close",
|
||||
"/cases/{case_id}/close",
|
||||
post(case_actions::handle_close_case),
|
||||
)
|
||||
.route(
|
||||
"/web/cases/{case_id}/reopen",
|
||||
"/cases/{case_id}/reopen",
|
||||
post(case_actions::handle_reopen_case),
|
||||
)
|
||||
.route(
|
||||
"/web/cases/{case_id}/reset",
|
||||
"/cases/{case_id}/reset",
|
||||
post(case_actions::handle_reset_case),
|
||||
)
|
||||
.route(
|
||||
"/web/cases/{case_id}/oneliner",
|
||||
"/cases/{case_id}/oneliner",
|
||||
post(oneliner_override::handle_web_put_oneliner),
|
||||
)
|
||||
.route(
|
||||
"/web/cases/purge-closed",
|
||||
"/cases/purge-closed",
|
||||
post(case_actions::handle_purge_closed),
|
||||
)
|
||||
.route("/web/cases/bulk", post(bulk::handle_bulk_action))
|
||||
.route(
|
||||
"/web/audio/{user}/{case_id}/{filename}",
|
||||
get(web::handle_audio),
|
||||
)
|
||||
.route("/web/events", get(events::handle_events))
|
||||
.route("/cases/bulk", post(bulk::handle_bulk_action))
|
||||
.route("/audio/{user}/{case_id}/{filename}", get(web::handle_audio))
|
||||
.route("/events", get(events::handle_events))
|
||||
}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
//! Doctor's manual oneliner override — two entry points sharing one core.
|
||||
//!
|
||||
//! - `PUT /api/cases/{case_id}/oneliner` (X-API-Key) for native clients.
|
||||
//! - `POST /web/cases/{case_id}/oneliner` (session + CSRF) for the
|
||||
//! - `POST /cases/{case_id}/oneliner` (session + CSRF) for the
|
||||
//! browser-based web UI.
|
||||
//!
|
||||
//! Both routes funnel into [`apply_manual_override`], which validates
|
||||
@@ -139,7 +139,7 @@ impl HasCsrfToken for OnelinerWebForm {
|
||||
}
|
||||
}
|
||||
|
||||
/// `POST /web/cases/{case_id}/oneliner` — session + CSRF authenticated.
|
||||
/// `POST /cases/{case_id}/oneliner` — session + CSRF authenticated.
|
||||
///
|
||||
/// Mirrors [`handle_put_oneliner`] but uses the web auth stack so the
|
||||
/// browser never has to expose an X-API-Key. Closed cases are rejected
|
||||
|
||||
@@ -295,7 +295,7 @@ struct MyCasesTemplate {
|
||||
csrf_token: String,
|
||||
}
|
||||
|
||||
/// Query params for GET /web/cases and /web/cases/{case_id}.
|
||||
/// Query params for GET /cases and /cases/{case_id}.
|
||||
#[derive(Debug, Default, Deserialize)]
|
||||
pub struct CaseListQuery {
|
||||
/// `?show_closed=1` expands the list to include closed cases and
|
||||
@@ -582,7 +582,7 @@ pub async fn handle_my_cases(
|
||||
let retention = user_record.map(|u| u.retention.clone()).unwrap_or_default();
|
||||
let preview_lines = user_record.map(|u| u.preview_lines).unwrap_or(2);
|
||||
|
||||
// Lazy retention sweep: at most one disk scan per /web/cases visit,
|
||||
// Lazy retention sweep: at most one disk scan per /cases visit,
|
||||
// reusing the user's retention policy from users.toml. Runs before
|
||||
// the listing scan so any auto-close/auto-purge actions are
|
||||
// reflected in the same response.
|
||||
@@ -629,7 +629,7 @@ pub async fn handle_my_cases(
|
||||
.map_err(|e| AppError::Internal(format!("Template render failed: {e}")))
|
||||
}
|
||||
|
||||
/// GET /web/cases/{case_id}
|
||||
/// GET /cases/{case_id}
|
||||
///
|
||||
/// Canonical case page. Renders the document inline if present; otherwise
|
||||
/// shows status (analyzing / placeholder for empty / recordings-summary).
|
||||
@@ -771,7 +771,7 @@ pub async fn handle_case_page(
|
||||
.map_err(|e| AppError::Internal(format!("Template render failed: {e}")))
|
||||
}
|
||||
|
||||
/// GET /web/cases/{case_id}/recordings
|
||||
/// GET /cases/{case_id}/recordings
|
||||
///
|
||||
/// Read-only sub-page showing all `.m4a` + transcript pairs for the case.
|
||||
/// Useful for power-users/admins; not part of the day-to-day workflow.
|
||||
|
||||
@@ -13,8 +13,8 @@ pub struct WebSession {
|
||||
pub expires_at: Instant,
|
||||
/// CSRF token — bound to the session for its full lifetime. Minted
|
||||
/// alongside the session cookie via [`generate_token`] and validated
|
||||
/// by the `CsrfForm` extractor on every state-changing POST under
|
||||
/// `/web/`. Not rotated per request (would break multi-tab use).
|
||||
/// by the `CsrfForm` extractor on every state-changing browser POST.
|
||||
/// Not rotated per request (would break multi-tab use).
|
||||
pub csrf_token: String,
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user