fix(auth): address SA4 review findings (WS epoch check, commit ordering, verify guard)
Adversarial re-review of the session-epoch work surfaced: - WS auth (messages_ws) skipped the epoch check, so a revoked-but-unexpired cookie could still open the live push channel after a reset/logout-others. Now mirrors current_user: loads the user once, rejects a stale-epoch cookie before connecting. - set_password + logout_others re-stamped the cookie BEFORE db.commit(); a failed commit would strand the current session at a newer epoch than the DB and wrongly 401 it. Commit first, then re-stamp. - Welcome verify effect could double-POST the single-use token (StrictMode/remount) and flip the banner to a false 'invalid'. Fire-once useRef guard. Left as-is (low value, documented): the Plex image proxy authenticates without a DB load / epoch check (poster/art fetches only); adding one would cost a DB hit per image.
This commit is contained in:
parent
95d1549570
commit
07dba4da9e
3 changed files with 20 additions and 7 deletions
|
|
@ -809,9 +809,11 @@ def logout_others(
|
||||||
in somewhere" / a suspected session compromise. The demo account (shared) is excluded."""
|
in somewhere" / a suspected session compromise. The demo account (shared) is excluded."""
|
||||||
if user.is_demo:
|
if user.is_demo:
|
||||||
raise HTTPException(status_code=403, detail="Not available in the demo account.")
|
raise HTTPException(status_code=403, detail="Not available in the demo account.")
|
||||||
|
# Commit the bump BEFORE re-stamping this cookie, so a failed commit can't strand the current
|
||||||
|
# session at a newer epoch than the DB (see set_password).
|
||||||
bump_session_epoch(user)
|
bump_session_epoch(user)
|
||||||
_remember_epoch(request, user)
|
|
||||||
db.commit()
|
db.commit()
|
||||||
|
_remember_epoch(request, user)
|
||||||
log.info("Logout-others: uid=%s (epoch=%s)", user.id, user.session_epoch)
|
log.info("Logout-others: uid=%s (epoch=%s)", user.id, user.session_epoch)
|
||||||
return {"ok": True}
|
return {"ok": True}
|
||||||
|
|
||||||
|
|
@ -892,10 +894,11 @@ def set_password(
|
||||||
raise HTTPException(status_code=403, detail="Current password is incorrect.")
|
raise HTTPException(status_code=403, detail="Current password is incorrect.")
|
||||||
user.password_hash = hash_password(new_password)
|
user.password_hash = hash_password(new_password)
|
||||||
# SA4: a password change logs out every OTHER session; re-stamp this cookie at the new epoch so
|
# SA4: a password change logs out every OTHER session; re-stamp this cookie at the new epoch so
|
||||||
# the person making the change stays signed in here.
|
# the person making the change stays signed in here. Commit BEFORE re-stamping so a failed commit
|
||||||
|
# can't leave the cookie at a newer epoch than the DB (which would wrongly 401 THIS session).
|
||||||
bump_session_epoch(user)
|
bump_session_epoch(user)
|
||||||
_remember_epoch(request, user)
|
|
||||||
db.commit()
|
db.commit()
|
||||||
|
_remember_epoch(request, user)
|
||||||
log.info("Password set/changed: uid=%s", user.id)
|
log.info("Password set/changed: uid=%s", user.id)
|
||||||
return {"ok": True}
|
return {"ok": True}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -381,7 +381,13 @@ async def messages_ws(ws: WebSocket) -> None:
|
||||||
return
|
return
|
||||||
db = SessionLocal()
|
db = SessionLocal()
|
||||||
try:
|
try:
|
||||||
ok = is_messageable_user(db.get(User, uid))
|
user = db.get(User, uid)
|
||||||
|
# SA4: honour server-side session revocation on the live channel too — reject a cookie whose
|
||||||
|
# recorded epoch is behind the account's current one (mirrors current_user). Without this a
|
||||||
|
# copied cookie could keep receiving pushes after a password reset / "log out everywhere".
|
||||||
|
epochs = sess.get("epochs") or {}
|
||||||
|
epoch_ok = user is not None and int(epochs.get(str(uid), 0)) == (user.session_epoch or 0)
|
||||||
|
ok = epoch_ok and is_messageable_user(user)
|
||||||
finally:
|
finally:
|
||||||
db.close()
|
db.close()
|
||||||
if not ok:
|
if not ok:
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
import { useEffect, useState } from "react";
|
import { useEffect, useRef, useState } from "react";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
import {
|
import {
|
||||||
ArrowRight,
|
ArrowRight,
|
||||||
|
|
@ -282,9 +282,13 @@ function AuthCard() {
|
||||||
}
|
}
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
// Confirm a fragment verification token by POSTing it (new SB3 flow).
|
// Confirm a fragment verification token by POSTing it (new SB3 flow). Fire ONCE: the token is
|
||||||
|
// single-use, so a double POST (StrictMode remount / any re-render) would 400 on the second call
|
||||||
|
// and wrongly flip the banner to "invalid".
|
||||||
|
const verifyFired = useRef(false);
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!verifyToken) return;
|
if (!verifyToken || verifyFired.current) return;
|
||||||
|
verifyFired.current = true;
|
||||||
api
|
api
|
||||||
.verifyEmail(verifyToken)
|
.verifyEmail(verifyToken)
|
||||||
.then(() => setVerify("ok"))
|
.then(() => setVerify("ok"))
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue