Brummel/doctate
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(web): make closed cases fully viewable (discovery-gate, not access-gate)

Browse Source 
Follow-up to 6d1ca71: that commit made the recordings sub-page render for a
closed case by threading ?show_closed=1, but the page's own links and the
audio player still 404'd — playback was dead and the back link led nowhere.
Root cause: the `.closed` marker was implemented as an ACCESS gate (every
case-scoped read 404s a closed case unless the opt-in flag is threaded
through), while it is documented as a DISCOVERY gate ("hidden from the
default listing"). Threading the flag through every link/redirect is
whack-a-mole; the next new link forgets it.

Resolved in favour of discovery-gate semantics: the `.closed` marker only
filters the default case LIST (scan_user_cases). Direct/deep-link access to
a known, owned case and all its sub-resources is closed-agnostic. IDOR is
unchanged — it comes from the per-user root + existence check, orthogonal
to closed-ness.

Scope 1 — closed cases are fully viewable:
- handle_audio: drop the is_closed -> 404 gate (web.rs). Audio is a
  capability URL behind the same per-user path + slug/UUID/filename
  validation as an open case.
- handle_case_page / handle_case_recordings: always resolve via
  locate_closed_case_or_404; show_closed is no longer an access gate, so
  back links (recordings -> case, case -> list) reach live pages without
  threading the flag.
- the "back to list" links carry ?show_closed=1 when the case is closed
  (computed server-side from the marker) so the user returns to the
  closed-inclusive list, not the default list that hides the case.

Scope 2 — closed means read-only:
- case_page.html withholds the analyze / reset / re-analyze / retry forms
  on closed cases; only the reopen affordance remains.
- case_recordings.html withholds the per-recording delete form on closed
  cases (CaseRecordingsTemplate gains is_closed).
- the mutating handlers (analyze/reset/delete/oneliner) keep rejecting
  closed cases via locate_case_or_404 as defense-in-depth behind the
  now-hidden buttons.

Tests (RED-first):
- web_test: audio serves a closed case's file (200, not 404).
- case_page_test: closed case page + recordings render without
  ?show_closed=1; closed case hides analyze/reset; open case still shows
  delete; closed case hides delete.
- analyze_test: closed_case_returns_404_on_detail rewritten to
  closed_case_detail_renders_read_only — the old test pinned the
  now-superseded access-gate contract.

cargo test (440 passed), clippy, and fmt all clean.

closes #17
This commit is contained in:
Brummel
2026-06-01 01:42:25 +02:00
parent 6d1ca71d61
commit 5e86cb59b0
7 changed files with 272 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/routes/user_web.rs
+25 -44
View File
@@ -409,6 +409,10 @@ struct CaseRecordingsTemplate {
recordings: Vec<RecordingView>,
transcribe_busy: bool,
is_admin: bool,
/// True when the case carries the `.closed` marker. Drives the read-only
/// view: the per-recording delete form is withheld and the back-to-list
/// link returns to the closed-inclusive list (`?show_closed=1`).
is_closed: bool,
/// Session CSRF token — rendered as a hidden field into the
/// per-recording delete forms and the logout form.
csrf_token: String,
@@ -736,7 +740,6 @@ pub async fn handle_case_page(
State(boot_time): State<crate::BootTime>,
State(in_flight): State<InFlight>,
CaseIdPath(case_id): CaseIdPath,
Query(query): Query<CaseListQuery>,
) -> Result<Html<String>, AppError> {
let user_root = config.data_path.join(&user.slug);
pipeline
@@ -750,24 +753,13 @@ pub async fn handle_case_page(
)
.await;
let show_closed = query.include_closed();
let case_dir = if show_closed {
locate_closed_case_or_404(
&user_root,
&case_id,
&user.slug,
"case page (show_closed=1)",
)
.await?
} else {
locate_case_or_404(
&user_root,
&case_id,
&user.slug,
"case page (possible IDOR probe)",
)
.await?
};
// Direct case access is closed-agnostic: the `.closed` marker only hides
// a case from the default LIST (the discovery gate lives in
// `scan_user_cases`). A deep link or back link to a known, owned case —
// open or closed — must resolve, so the recordings page's back link works
// without threading `?show_closed=1`. IDOR is still guarded by the
// per-user root plus the existence check inside the resolver.
let case_dir = locate_closed_case_or_404(&user_root, &case_id, &user.slug, "case page").await?;
// Auto-analysis trigger for deep-link navigation: same evaluation as
// the list view. Document render below still wins the race if the
// worker happens to finish synchronously.
@@ -892,7 +884,6 @@ pub async fn handle_case_recordings(
State(http_client): State<reqwest::Client>,
State(vocab): State<Arc<Gazetteer>>,
CaseIdPath(case_id): CaseIdPath,
Query(query): Query<CaseListQuery>,
) -> Result<Html<String>, AppError> {
let user_root = config.data_path.join(&user.slug);
pipeline
@@ -906,27 +897,12 @@ pub async fn handle_case_recordings(
)
.await;
// Closing a case only writes a `.closed` marker; the `.m4a` recordings
// stay on disk. A user who opted into viewing closed cases must still
// reach this list, so branch on `show_closed=1` exactly like
// `handle_case_page` does.
let case_dir = if query.include_closed() {
locate_closed_case_or_404(
&user_root,
&case_id,
&user.slug,
"case recordings (show_closed=1)",
)
.await?
} else {
locate_case_or_404(
&user_root,
&case_id,
&user.slug,
"case recordings (possible IDOR probe)",
)
.await?
};
// Closed-agnostic, like `handle_case_page`: closing only writes a
// `.closed` marker and the `.m4a` recordings stay on disk, so the
// recordings sub-page must resolve for a closed case too. The discovery
// gate is the case LIST, not this view.
let case_dir =
locate_closed_case_or_404(&user_root, &case_id, &user.slug, "case recordings").await?;
let case_id_str = case_id.to_string();
info!(slug = %user.slug, case_id = %case_id, "case recordings viewed");
@@ -936,6 +912,7 @@ pub async fn handle_case_recordings(
let t_busy = pipeline.transcribe_busy.0.load(Ordering::Acquire);
let case_id_short = case_id_str.chars().take(8).collect();
let is_admin = user.is_admin();
let is_closed = crate::paths::is_closed(&case_dir).await;
CaseRecordingsTemplate {
csrf_token: user.csrf_token,
@@ -946,6 +923,7 @@ pub async fn handle_case_recordings(
recordings,
transcribe_busy: t_busy,
is_admin,
is_closed,
}
.render()
.map(Html)
@@ -992,9 +970,12 @@ pub(crate) async fn locate_case_or_404(
}
}
/// Like `locate_case_or_404` but accepts closed cases. Only the detail
/// view uses this, and only when the user navigated there with
/// `?show_closed=1`. A non-existent directory still 404s as expected.
/// Like `locate_case_or_404` but accepts closed cases. The read-only VIEW
/// handlers (case page, recordings sub-page) use this: the `.closed` marker
/// is a discovery filter for the case LIST, not an access gate on a known
/// case, so direct/deep-link access resolves regardless of closed-ness. The
/// mutating handlers keep using `locate_case_or_404` (which rejects closed)
/// since a closed case is read-only. A non-existent directory still 404s.
pub(crate) async fn locate_closed_case_or_404(
user_root: &Path,
case_id: &CaseId,
server/src/routes/web.rs
+5 -3
View File
@@ -56,10 +56,12 @@ pub async fn handle_audio(
.map_err(|_| AppError::BadRequest("Invalid case_id (not a UUID)".into()))?;
validate_filename(&filename)?;
// The `.closed` marker is a discovery filter for the case LIST, not an
// access gate on the audio file server: a closed case keeps its `.m4a`
// files on disk and its recordings page must still play them. IDOR is
// guarded by the per-user path plus the slug/UUID/filename validation
// above, independent of whether the case is closed.
let case_path = crate::paths::case_dir(&config.data_path, &user, &case_id);
if crate::paths::is_closed(&case_path).await {
return Err(AppError::NotFound("Audio file not found".into()));
}
let file_path = case_path.join(&filename);
let metadata = match tokio::fs::metadata(&file_path).await {
Ok(m) => m,
server/templates/case_page.html
+9 -5
View File
@@ -377,7 +377,7 @@
</head>
<body>
<header>
<div><a class="back" href="/cases">&larr; Fälle</a></div>
<div><a class="back" href="/cases{% if is_closed %}?show_closed=1{% endif %}">&larr; Fälle</a></div>
<div class="header-right">
{% if is_admin %}<label class="admin-toggle"
><input type="checkbox" id="admin-view-toggle" /> Admin
@@ -481,7 +481,7 @@
{% endif %}
<div class="actions">
{% if can_analyze && !has_document %}
{% if !is_closed && can_analyze && !has_document %}
<form method="post" action="/cases/{{ case_id }}/analyze">
{% call csrf::field(csrf_token) %}
<input type="hidden" name="return_to" value="/cases/{{ case_id }}">
@@ -497,9 +497,10 @@
{% endfor %}
</form>
{% endif %}
{% else if llm_missing && !has_document %}
{% else if !is_closed && llm_missing && !has_document %}
<span class="llm-missing">Kein LLM-Backend verfügbar — IONOS_API_KEY setzen.</span>
{% endif %} {% if is_admin %}
{% if !is_closed %}
<form
class="admin-only"
method="post"
@@ -509,6 +510,7 @@
<input type="hidden" name="return_to" value="/cases/{{ case_id }}">
<button type="submit">Reset</button>
</form>
{% endif %}
<button
type="button"
id="debug-copy-btn"
@@ -526,7 +528,7 @@
<div class="stale-banner">Analyse nicht aktuell — neue Aufnahmen seit letztem Lauf.</div>
{% endif %}
<div id="doc-content" class="doc-content">
{% if can_analyze %}
{% if !is_closed && can_analyze %}
<div class="doc-actions-top">
<form method="post" action="/cases/{{ case_id }}/analyze">
{% call csrf::field(csrf_token) %}
@@ -698,6 +700,7 @@
</dl>
<pre class="failure-reason">{{ failure.reason }}</pre>
</details>
{% if !is_closed %}
<form
method="post"
action="/cases/{{ case_id }}/analyze"
@@ -729,6 +732,7 @@
{% endfor %}
</form>
{% endif %}
{% endif %}
</div>
{% when None %} {% if analyzing %}
<div class="placeholder">Wird analysiert …</div>
@@ -744,7 +748,7 @@
<div>
<a
class="recordings-link"
href="/cases/{{ case_id }}/recordings{% if is_closed %}?show_closed=1{% endif %}"
href="/cases/{{ case_id }}/recordings"
>&rarr; Aufnahmen anzeigen ({{ recordings_count }})</a
>
</div>
server/templates/case_recordings.html
+3 -1
View File
@@ -133,7 +133,7 @@ try {
<body>
<header>
<div class="breadcrumb">
<a href="/cases">&larr; Fälle</a>
<a href="/cases{% if is_closed %}?show_closed=1{% endif %}">&larr; Fälle</a>
{% match oneliner %}
{% when Some with (t) %}
<a href="/cases/{{ case_id }}">&larr; {{ t }}</a>
@@ -158,6 +158,7 @@ try {
<input type="range" class="seek" min="0" max="1000" value="0" step="1" aria-label="Position">
<span class="time">0:00 / 0:00</span>
</div>
{% if !is_closed %}
<form method="post" action="/cases/{{ case_id }}/recordings/delete" class="rec-delete-form" data-ts="{{ rec.recorded_at_iso }}">
{% call csrf::field(csrf_token) %}
<input type="hidden" name="filename" value="{{ rec.filename }}">
@@ -168,6 +169,7 @@ try {
<button type="button" class="cancel-btn" aria-label="Abbrechen">✕</button>
</span>
</form>
{% endif %}
</div>
{% if is_admin %}<div class="filename admin-only">{{ rec.filename }}</div>{% endif %}
{% if rec.failed %}
server/tests/analyze_test.rs
+21 -3
View File
@@ -14,7 +14,7 @@ use wiremock::matchers::{method, path};
use wiremock::{Mock, MockServer, ResponseTemplate};
use common::{
ANALYSIS_INPUT_FILE, CLOSE_MARKER, DOCUMENT_FILE, ONELINER_FILENAME, TestConfig,
ANALYSIS_INPUT_FILE, CLOSE_MARKER, DOCUMENT_FILE, ONELINER_FILENAME, TestConfig, body_string,
csrf_form_post, get_with_cookie, login_with_csrf, paths, seed_case, seed_recording, test_admin,
test_user, unique_tmpdir, write_document_for_test,
};
@@ -737,8 +737,13 @@ async fn close_writes_marker_and_hides_case() {
);
}
/// After closing, the case detail page still renders, read-only. The
/// `.closed` marker is a discovery filter for the case LIST, not an access
/// gate on a known, owned case (issue #17): a deep link or back link must
/// resolve. Closing swaps the mutating actions for a reopen affordance
/// rather than 404ing the page.
#[tokio::test]
async fn closed_case_returns_404_on_detail() {
async fn closed_case_detail_renders_read_only() {
let (cfg, settings) = cfg_llm("close-2");
let case_id = "11111111-1111-1111-1111-111111111111";
let case_dir = seed_case(&cfg.data_path, "dr_a", case_id);
@@ -763,7 +768,20 @@ async fn closed_case_returns_404_on_detail() {
.oneshot(get_with_cookie(paths::case_detail(case_id), &cookie))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
assert_eq!(
resp.status(),
StatusCode::OK,
"closed case detail must render read-only, not 404",
);
let body = body_string(resp).await;
assert!(
body.contains(&format!(r#"action="{}""#, paths::case_reopen(case_id))),
"closed case detail must offer the reopen action"
);
assert!(
!body.contains(&format!(r#"action="{}""#, paths::case_analyze(case_id))),
"closed case detail must not offer the analyze action"
);
}
#[tokio::test]
server/tests/case_page_test.rs
+174
View File
@@ -417,6 +417,180 @@ async fn case_recordings_back_link_targets_case_page() {
);
}
/// Property: a closed case is reachable by direct navigation WITHOUT
/// `?show_closed=1`. The `.closed` marker is a discovery filter for the case
/// list, not an access gate — so deep-linking to a closed case, or following
/// the recordings page's back link (which carries no flag), must render the
/// case page, not 404. Regression for issue #17.
#[tokio::test]
async fn case_page_renders_for_closed_case_without_show_closed() {
let cfg = TestConfig::new()
.with_label("cp-closed-noflag")
.with_user(test_user("dr_a"))
.with_llm("http://unused")
.build();
let case_id = "11111111-1111-1111-1111-111111111111";
let case_dir = seed_case(&cfg.data_path, "dr_a", case_id);
seed_recording(&case_dir, "2026-04-15T10-00-00Z", Some("ein Text"));
write_closed_marker(&case_dir, "2026-04-15T12:00:00Z");
let app = doctate_server::create_router(cfg);
let cookie = login_dr_a(&app).await;
// No ?show_closed=1 on the URL — the deep-link / back-link path.
let resp = app
.oneshot(get_with_cookie(paths::case_detail(case_id), &cookie))
.await
.unwrap();
assert_eq!(
resp.status(),
StatusCode::OK,
"closed case page must render on direct access, not 404",
);
let body = body_string(resp).await;
assert!(
body.contains(&format!(r#"action="{}""#, paths::case_reopen(case_id))),
"closed case page must show the reopen action (proves the closed view rendered)"
);
}
/// Property: a closed case is read-only. Closing means "done", so the
/// mutating affordances (analyze, reset) are withheld even though the case
/// page still renders for viewing. Only reopen stays — it's how the closed
/// state is left.
#[tokio::test]
async fn case_page_hides_mutation_actions_when_closed() {
let cfg = TestConfig::new()
.with_label("cp-closed-ro")
.with_user(test_admin("dr_a"))
.with_llm("http://unused")
.build();
let case_id = "11111111-1111-1111-1111-111111111111";
let case_dir = seed_case(&cfg.data_path, "dr_a", case_id);
seed_recording(&case_dir, "2026-04-15T10-00-00Z", Some("ein Text"));
write_closed_marker(&case_dir, "2026-04-15T12:00:00Z");
let app = doctate_server::create_router(cfg);
let cookie = login_dr_a(&app).await;
let resp = app
.oneshot(get_with_cookie(paths::case_detail(case_id), &cookie))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let body = body_string(resp).await;
assert!(
!body.contains(&format!(r#"action="{}""#, paths::case_analyze(case_id))),
"closed case must not expose the analyze action"
);
assert!(
!body.contains(&format!(r#"action="{}""#, paths::case_reset(case_id))),
"closed case must not expose the reset action"
);
assert!(
body.contains(&format!(r#"action="{}""#, paths::case_reopen(case_id))),
"closed case must still offer reopen"
);
}
/// Property: the recordings sub-page is reachable for a closed case WITHOUT
/// `?show_closed=1` (same discovery-gate reasoning as the case page).
#[tokio::test]
async fn case_recordings_renders_for_closed_case_without_show_closed() {
let cfg = TestConfig::new()
.with_label("rec-closed-noflag")
.with_user(test_user("dr_a"))
.with_llm("http://unused")
.build();
let case_id = "11111111-1111-1111-1111-111111111111";
let case_dir = seed_case(&cfg.data_path, "dr_a", case_id);
seed_recording(&case_dir, "2026-04-15T10-00-00Z", Some("erste Aufnahme"));
write_closed_marker(&case_dir, "2026-04-15T12:00:00Z");
let app = doctate_server::create_router(cfg);
let cookie = login_dr_a(&app).await;
let resp = app
.oneshot(get_with_cookie(paths::case_recordings(case_id), &cookie))
.await
.unwrap();
assert_eq!(
resp.status(),
StatusCode::OK,
"closed case recordings must render on direct access, not 404",
);
let body = body_string(resp).await;
assert!(
body.contains("2026-04-15T10-00-00Z.m4a"),
"recording on disk must be listed for the closed case"
);
}
/// Property: an OPEN case's recordings page exposes the per-recording delete
/// form. Locks the positive side so the closed-case hiding below cannot
/// silently regress into hiding delete everywhere.
#[tokio::test]
async fn case_recordings_shows_delete_when_open() {
let cfg = TestConfig::new()
.with_label("rec-del-open")
.with_user(test_user("dr_a"))
.with_llm("http://unused")
.build();
let case_id = "11111111-1111-1111-1111-111111111111";
let case_dir = seed_case(&cfg.data_path, "dr_a", case_id);
seed_recording(&case_dir, "2026-04-15T10-00-00Z", Some("Text"));
let app = doctate_server::create_router(cfg);
let cookie = login_dr_a(&app).await;
let resp = app
.oneshot(get_with_cookie(paths::case_recordings(case_id), &cookie))
.await
.unwrap();
let body = body_string(resp).await;
assert!(
body.contains(&format!(
r#"action="{}/delete""#,
paths::case_recordings(case_id)
)),
"open recordings page must expose the delete action"
);
}
/// Property: a closed case is read-only — the per-recording delete form is
/// withheld on the recordings sub-page, mirroring the case page hiding its
/// mutating actions.
#[tokio::test]
async fn case_recordings_hides_delete_when_closed() {
let cfg = TestConfig::new()
.with_label("rec-del-closed")
.with_user(test_user("dr_a"))
.with_llm("http://unused")
.build();
let case_id = "11111111-1111-1111-1111-111111111111";
let case_dir = seed_case(&cfg.data_path, "dr_a", case_id);
seed_recording(&case_dir, "2026-04-15T10-00-00Z", Some("Text"));
write_closed_marker(&case_dir, "2026-04-15T12:00:00Z");
let app = doctate_server::create_router(cfg);
let cookie = login_dr_a(&app).await;
let resp = app
.oneshot(get_with_cookie(paths::case_recordings(case_id), &cookie))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let body = body_string(resp).await;
assert!(
!body.contains(&format!(
r#"action="{}/delete""#,
paths::case_recordings(case_id)
)),
"closed recordings page must not expose the delete action"
);
}
#[tokio::test]
async fn case_page_uses_oneliner_as_h1_when_present() {
let cfg = TestConfig::new()
server/tests/web_test.rs
+35
View File
@@ -191,3 +191,38 @@ async fn web_audio_nonexistent_file_returns_404() {
assert_eq!(response.status(), StatusCode::NOT_FOUND);
}
/// Property: a closed case still serves its audio. Closing only writes a
/// `.closed` marker — the `.m4a` files stay on disk — and that marker is a
/// discovery filter for the case LIST, not an access gate on the audio file
/// server. So playback from a closed case's recordings page must work, not
/// 404. Regression for issue #17 (audio playback broken on closed cases).
#[tokio::test]
async fn web_audio_serves_closed_case_file() {
let config = test_config();
let data_path = config.data_path.clone();
let case_id = "770e8400-e29b-41d4-a716-446655440099";
let case_dir = common::seed_case(&data_path, "dr_test", case_id);
let audio_bytes = b"fake audio content for a closed case";
let filename = "2026-04-13T10-30-00Z.m4a";
std::fs::write(case_dir.join(filename), audio_bytes).unwrap();
common::write_closed_marker(&case_dir, "2026-04-13T12:00:00Z");
let app = doctate_server::create_router(config);
let response = app
.oneshot(
Request::builder()
.uri(format!("/audio/dr_test/{case_id}/{filename}"))
.body(Body::empty())
.unwrap(),
)
.await
.unwrap();
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(header_opt(&response, "content-type"), Some("audio/mp4"));
let bytes = common::body_bytes(response).await;
assert_eq!(&bytes[..], audio_bytes);
}