fix: Inline recording-delete confirm + stable redirects
- Replace native confirm() in the recording list with an inline
two-step confirm row; hide the player via visibility:hidden
while open so the row height stays stable.
- Hard-redirect recording delete back to /web/cases/{id}/recordings
instead of relying on the Referer header (which silently fell
back to /web/cases when stripped).
- Switch analyze/reset to a hidden return_to form field so the
source page (case detail vs. case list) is preserved reliably,
with same-origin /web/ prefix validation.
- Tighten delete_recording_test on the concrete Location header;
adjust analyze/reset redirect expectations to the new
case-detail fallback.
This commit is contained in:
@@ -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<String>,
|
||||
}
|
||||
|
||||
impl HasCsrfToken for CsrfOnlyForm {
|
||||
|
||||
@@ -43,8 +43,7 @@ pub async fn handle_analyze_case(
|
||||
State(analyze_tx): State<AnalyzeSender>,
|
||||
State(events_tx): State<EventSender>,
|
||||
CaseIdPath(case_id): CaseIdPath,
|
||||
headers: HeaderMap,
|
||||
_csrf: CsrfForm<CsrfOnlyForm>,
|
||||
CsrfForm(form): CsrfForm<CsrfOnlyForm>,
|
||||
) -> Result<Redirect, AppError> {
|
||||
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<String>, 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<Arc<Config>>,
|
||||
State(events_tx): State<EventSender>,
|
||||
CaseIdPath(case_id): CaseIdPath,
|
||||
headers: HeaderMap,
|
||||
_csrf: CsrfForm<CsrfOnlyForm>,
|
||||
CsrfForm(form): CsrfForm<CsrfOnlyForm>,
|
||||
) -> Result<Redirect, AppError> {
|
||||
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<Arc<Config>>,
|
||||
State(events_tx): State<EventSender>,
|
||||
CaseIdPath(case_id): CaseIdPath,
|
||||
headers: HeaderMap,
|
||||
CsrfForm(form): CsrfForm<DeleteRecordingForm>,
|
||||
) -> Result<Redirect, AppError> {
|
||||
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> {
|
||||
|
||||
@@ -385,6 +385,7 @@
|
||||
{% if can_analyze && !has_document %}
|
||||
<form method="post" action="/web/cases/{{ case_id }}/analyze">
|
||||
{% call csrf::field(csrf_token) %}
|
||||
<input type="hidden" name="return_to" value="/web/cases/{{ case_id }}">
|
||||
<button type="submit">Analysieren</button>
|
||||
</form>
|
||||
{% else if llm_missing && !has_document %}
|
||||
@@ -396,6 +397,7 @@
|
||||
action="/web/cases/{{ case_id }}/reset"
|
||||
>
|
||||
{% call csrf::field(csrf_token) %}
|
||||
<input type="hidden" name="return_to" value="/web/cases/{{ case_id }}">
|
||||
<button type="submit">Reset</button>
|
||||
</form>
|
||||
{% endif %}
|
||||
@@ -407,6 +409,7 @@
|
||||
<div class="doc-actions-top">
|
||||
<form method="post" action="/web/cases/{{ case_id }}/analyze">
|
||||
{% call csrf::field(csrf_token) %}
|
||||
<input type="hidden" name="return_to" value="/web/cases/{{ case_id }}">
|
||||
<button
|
||||
type="submit"
|
||||
class="copy-btn"
|
||||
|
||||
@@ -63,7 +63,10 @@ header form { margin: 0; }
|
||||
.admin-toggle { font-size: 0.85em; color: #666; display: inline-flex; align-items: center; gap: 0.3em; cursor: pointer; user-select: none; }
|
||||
html.admin-view-off .admin-only { display: none !important; }
|
||||
|
||||
.rec-delete-form { margin: 0; }
|
||||
/* `margin-left: auto` pins the delete control to the row's right edge
|
||||
even when the player (the usual `flex: 1` spacer) is hidden during
|
||||
the confirm state — prevents the confirm row from collapsing left. */
|
||||
.rec-delete-form { margin: 0 0 0 auto; display: inline-flex; align-items: center; }
|
||||
.rec-delete-form .delete-btn {
|
||||
padding: 0.3em; background: transparent; border: none; color: #888;
|
||||
cursor: pointer; display: inline-flex; align-items: center;
|
||||
@@ -71,6 +74,43 @@ html.admin-view-off .admin-only { display: none !important; }
|
||||
transition: color 0.15s, background 0.15s;
|
||||
}
|
||||
.rec-delete-form .delete-btn:hover { color: #e24a4a; background: #fff0f0; }
|
||||
/* Inline confirmation. Hidden by default; revealed via .is-confirming
|
||||
which the JS below toggles on the form. Replaces native confirm(). */
|
||||
.rec-delete-form .confirm-row {
|
||||
display: none;
|
||||
align-items: center;
|
||||
gap: 0.4em;
|
||||
font-size: 0.9em;
|
||||
}
|
||||
.rec-delete-form .confirm-row .confirm-label { color: #b00; }
|
||||
.rec-delete-form .confirm-row .confirm-btn {
|
||||
padding: 0.3em 0.7em;
|
||||
background: #e24a4a;
|
||||
color: #fff;
|
||||
border: 1px solid #c93c3c;
|
||||
border-radius: 4px;
|
||||
cursor: pointer;
|
||||
font: inherit;
|
||||
font-size: 0.85em;
|
||||
}
|
||||
.rec-delete-form .confirm-row .confirm-btn:hover { background: #c93c3c; }
|
||||
.rec-delete-form .confirm-row .cancel-btn {
|
||||
padding: 0.2em 0.4em;
|
||||
background: transparent;
|
||||
border: none;
|
||||
color: #888;
|
||||
cursor: pointer;
|
||||
font-size: 1em;
|
||||
line-height: 1;
|
||||
}
|
||||
.rec-delete-form .confirm-row .cancel-btn:hover { color: #333; }
|
||||
.rec-delete-form.is-confirming .delete-btn { display: none; }
|
||||
.rec-delete-form.is-confirming .confirm-row { display: inline-flex; }
|
||||
/* Hide the audio controls while a delete confirmation is open.
|
||||
`visibility: hidden` (not `display: none`) keeps the player's
|
||||
layout slot — and its height — reserved, so the row doesn't
|
||||
shrink. `:has()` reacts to descendant state from the parent. */
|
||||
.recording-head:has(.rec-delete-form.is-confirming) .player { visibility: hidden; }
|
||||
</style>
|
||||
<script>
|
||||
try {
|
||||
@@ -112,6 +152,11 @@ try {
|
||||
{% call csrf::field(csrf_token) %}
|
||||
<input type="hidden" name="filename" value="{{ rec.filename }}">
|
||||
<button type="submit" class="delete-btn" title="Aufnahme entfernen" aria-label="Aufnahme entfernen"><svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="16" height="16" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><polyline points="3 6 5 6 21 6"></polyline><path d="M19 6l-1 14a2 2 0 0 1-2 2H8a2 2 0 0 1-2-2L5 6"></path><path d="M10 11v6"></path><path d="M14 11v6"></path><path d="M9 6V4a2 2 0 0 1 2-2h2a2 2 0 0 1 2 2v2"></path></svg></button>
|
||||
<span class="confirm-row" role="group" aria-label="Löschen bestätigen">
|
||||
<span class="confirm-label">Wirklich löschen?</span>
|
||||
<button type="submit" class="confirm-btn">Löschen</button>
|
||||
<button type="button" class="cancel-btn" aria-label="Abbrechen">✕</button>
|
||||
</span>
|
||||
</form>
|
||||
</div>
|
||||
{% if is_admin %}<div class="filename admin-only">{{ rec.filename }}</div>{% endif %}
|
||||
@@ -160,18 +205,39 @@ try {
|
||||
el.textContent = formatRec(d);
|
||||
});
|
||||
|
||||
// Native confirm() before destructive delete. If JS is off, the form
|
||||
// submits unconfirmed — acceptable for an internal tool.
|
||||
// Inline two-step confirm before destructive delete: first submit
|
||||
// attempt is intercepted and toggles `.is-confirming` (which reveals
|
||||
// the inline "Wirklich löschen? [Löschen] [✕]" row); the second
|
||||
// submit (now from the explicit confirm button) goes through.
|
||||
// Without JS the form submits unconfirmed on first click — same
|
||||
// behavior as before; acceptable for an internal tool.
|
||||
const closeOtherConfirms = (except) => {
|
||||
document.querySelectorAll('.rec-delete-form.is-confirming').forEach(f => {
|
||||
if (f !== except) f.classList.remove('is-confirming');
|
||||
});
|
||||
};
|
||||
document.querySelectorAll('.rec-delete-form').forEach(form => {
|
||||
form.addEventListener('submit', e => {
|
||||
const iso = form.dataset.ts;
|
||||
let label = 'diese Aufnahme';
|
||||
if (iso) {
|
||||
const d = new Date(iso);
|
||||
if (!isNaN(d.getTime())) label = 'die Aufnahme vom ' + formatRec(d);
|
||||
if (!form.classList.contains('is-confirming')) {
|
||||
e.preventDefault();
|
||||
closeOtherConfirms(form);
|
||||
form.classList.add('is-confirming');
|
||||
}
|
||||
if (!confirm(`Soll ${label} wirklich gelöscht werden?`)) e.preventDefault();
|
||||
});
|
||||
const cancelBtn = form.querySelector('.cancel-btn');
|
||||
if (cancelBtn) {
|
||||
cancelBtn.addEventListener('click', () => {
|
||||
form.classList.remove('is-confirming');
|
||||
});
|
||||
}
|
||||
});
|
||||
// Single document-level listener: clicking outside the active confirm
|
||||
// closes it. `closest('.rec-delete-form.is-confirming')` returns the
|
||||
// form only if the click landed inside the currently-open confirm
|
||||
// row, which is the one case we want to keep open.
|
||||
document.addEventListener('click', e => {
|
||||
const insideConfirming = e.target.closest('.rec-delete-form.is-confirming');
|
||||
closeOtherConfirms(insideConfirming);
|
||||
});
|
||||
|
||||
const fmtDur = s => {
|
||||
|
||||
@@ -103,6 +103,8 @@ try {
|
||||
{% else %}
|
||||
<form method="post" action="/web/cases/bulk">
|
||||
{% call csrf::field(csrf_token) %}
|
||||
<input type="hidden" name="return_to" value="/web/cases{% if show_closed %}?show_closed=1{% endif %}">
|
||||
|
||||
{% for g in groups %}
|
||||
<section>
|
||||
<h2 class="date-group" data-iso="{{ g.label }}"><span class="date-label">{{ g.label }}</span><span class="count">{{ g.open_count }}/{{ g.total_count }} Fälle</span></h2>
|
||||
|
||||
@@ -216,7 +216,7 @@ async fn analyze_case_happy_path_writes_input_and_redirects() {
|
||||
.unwrap()
|
||||
.to_str()
|
||||
.unwrap(),
|
||||
"/web/cases"
|
||||
format!("/web/cases/{case_id}")
|
||||
);
|
||||
|
||||
let input_path = case_dir.join(ANALYSIS_INPUT_FILE);
|
||||
@@ -1190,7 +1190,10 @@ async fn reset_case_clears_derived_and_unfails_audio() {
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::SEE_OTHER);
|
||||
assert_eq!(resp.headers().get(header::LOCATION).unwrap(), "/web/cases");
|
||||
assert_eq!(
|
||||
resp.headers().get(header::LOCATION).unwrap(),
|
||||
&format!("/web/cases/{case_id}")
|
||||
);
|
||||
|
||||
// Audio preserved, .failed renamed to .m4a.
|
||||
assert!(dir.join("2026-04-15T09-00-00Z.m4a").exists());
|
||||
|
||||
@@ -54,6 +54,14 @@ async fn delete_recording_removes_file_and_sidecars_and_invalidates_derived() {
|
||||
"delete should redirect (3xx), got {}",
|
||||
resp.status()
|
||||
);
|
||||
let expected_location = format!("/web/cases/{case_id}/recordings");
|
||||
assert_eq!(
|
||||
resp.headers()
|
||||
.get(axum::http::header::LOCATION)
|
||||
.and_then(|v| v.to_str().ok()),
|
||||
Some(expected_location.as_str()),
|
||||
"delete should redirect back to the recording list, not the case list"
|
||||
);
|
||||
|
||||
// Victim + its sidecars are gone.
|
||||
assert!(!case_dir.join(&victim).exists(), "m4a should be deleted");
|
||||
@@ -212,6 +220,14 @@ async fn delete_last_recording_clears_case() {
|
||||
"last-recording delete should still redirect, got {}",
|
||||
resp.status()
|
||||
);
|
||||
let expected_location = format!("/web/cases/{case_id}/recordings");
|
||||
assert_eq!(
|
||||
resp.headers()
|
||||
.get(axum::http::header::LOCATION)
|
||||
.and_then(|v| v.to_str().ok()),
|
||||
Some(expected_location.as_str()),
|
||||
"last-recording delete should still redirect to the recording list"
|
||||
);
|
||||
|
||||
assert!(!case_dir.join(&only).exists());
|
||||
assert!(!case_dir.join(ONELINER_FILENAME).exists());
|
||||
|
||||
Reference in New Issue
Block a user