fix(review-2): address re-review — close two blocker regressions + harden tests
The round-2 re-review (full suite green) found two real defects the passing
tests missed, both now fixed and covered:
B1 — banned users could still edit/delete their OWN content: edit_upload,
delete_upload, delete_comment skipped the is_banned guard that upload/
like/comment got (round-2 removed the global extractor block and missed
these three). Added the guard (using auth.is_banned — no extra query).
The moderation ban test now asserts 403 on DELETE upload, PATCH upload,
and DELETE comment for a banned owner (+ upload survives) — it previously
only exercised POST /upload, which is why the gap shipped green.
B2 — H3 live-eviction was dead code: 'user-hidden' and 'comment-deleted' were
missing from KNOWN_EVENTS, so EventSource never listened and the handlers
never fired. Added both. New browser-driven sse-eviction test asserts a
hidden user's card actually evicts from an open feed with no reload — the
backend-only SseListener checks couldn't catch this.
Minor items folded in:
- admin dashboard bounces a mid-session-demoted admin to /feed via live
/me/context (mirrors the host page) instead of a dead error screen.
- feed "new posts" pill cleared on any full refresh (pull-to-refresh no longer
strands it).
- corrected the stale sse.rs comment (banned users may hold a read-only stream).
Verified: backend 35 unit tests; svelte-check 0 errors; e2e 04-host + comment-ui
+ export-leak 13/13 on chromium (incl. the two new regression tests).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -199,6 +199,10 @@ pub async fn delete_comment(
|
||||
auth: AuthUser,
|
||||
Path(comment_id): Path<Uuid>,
|
||||
) -> Result<StatusCode, AppError> {
|
||||
// Banned users keep read access but cannot mutate (USER_JOURNEYS §10).
|
||||
if auth.is_banned {
|
||||
return Err(AppError::Forbidden("Du bist gesperrt.".into()));
|
||||
}
|
||||
let comment = Comment::find_by_id(&state.pool, comment_id)
|
||||
.await?
|
||||
.ok_or_else(|| AppError::NotFound("Kommentar nicht gefunden.".into()))?;
|
||||
|
||||
@@ -48,10 +48,10 @@ pub async fn stream(
|
||||
.consume(&q.ticket)
|
||||
.ok_or_else(|| AppError::Unauthorized("Ticket ungültig oder abgelaufen.".into()))?;
|
||||
|
||||
// NOTE: this authenticates via ticket→session, not the `AuthUser` extractor, so
|
||||
// it does not re-check `is_banned`. A user banned mid-stream keeps receiving
|
||||
// events until the connection drops; they cannot reconnect (issue_ticket uses
|
||||
// AuthUser, which rejects banned users). Documented in docs/SECURITY-BACKLOG.md.
|
||||
// NOTE: this authenticates via ticket→session, not the `AuthUser` extractor. The
|
||||
// SSE stream is read-only, and under the read-only-ban model (USER_JOURNEYS §10)
|
||||
// banned users retain read access — so both minting a ticket and holding a stream
|
||||
// open are intentionally allowed for banned users; only writes are blocked.
|
||||
Session::find_by_token_hash(&state.pool, &token_hash)
|
||||
.await
|
||||
.map_err(|e| AppError::Internal(e.into()))?
|
||||
|
||||
@@ -272,6 +272,10 @@ pub async fn edit_upload(
|
||||
Path(upload_id): Path<Uuid>,
|
||||
Json(body): Json<EditUploadRequest>,
|
||||
) -> Result<StatusCode, AppError> {
|
||||
// Banned users keep read access but cannot mutate (USER_JOURNEYS §10).
|
||||
if auth.is_banned {
|
||||
return Err(AppError::Forbidden("Du bist gesperrt.".into()));
|
||||
}
|
||||
let upload = Upload::find_by_id_and_event(&state.pool, upload_id, auth.event_id)
|
||||
.await?
|
||||
.ok_or_else(|| AppError::NotFound("Upload nicht gefunden.".into()))?;
|
||||
@@ -303,6 +307,10 @@ pub async fn delete_upload(
|
||||
auth: AuthUser,
|
||||
Path(upload_id): Path<Uuid>,
|
||||
) -> Result<StatusCode, AppError> {
|
||||
// Banned users keep read access but cannot mutate (USER_JOURNEYS §10).
|
||||
if auth.is_banned {
|
||||
return Err(AppError::Forbidden("Du bist gesperrt.".into()));
|
||||
}
|
||||
let upload = Upload::find_by_id_and_event(&state.pool, upload_id, auth.event_id)
|
||||
.await?
|
||||
.ok_or_else(|| AppError::NotFound("Upload nicht gefunden.".into()))?;
|
||||
|
||||
Reference in New Issue
Block a user