fix(auth): refusing a revoked session no longer deletes a newer sign-in's cookie
The refusal cleared the session, so a request in flight with the previous account's cookie answered with a cookie deletion that could land after a sign-in as someone else. A revoked id is refused on every request anyway. Also a test for the pruning of old revocations.
This commit is contained in:
+4
-2
@@ -368,9 +368,11 @@ def _session_user(request: Request, db: Session, user_id: int | None) -> User:
|
||||
if epoch is None or int(epoch) != (user.session_epoch or 0):
|
||||
request.session.clear()
|
||||
raise refused(401, "not_authenticated")
|
||||
# A session signed out (or replaced by a sign-in as someone else) whose cookie landed late.
|
||||
# A session signed out (or replaced by a sign-in as someone else) whose cookie landed late. Not
|
||||
# cleared: this request may have left before a sign-in as someone else answered, and a cookie
|
||||
# deletion landing after that sign-in's cookie would sign the browser out of it. A revoked id is
|
||||
# refused on every request and never renewed, so leaving its cookie costs nothing.
|
||||
if db.get(RevokedSession, _session_id(request)) is not None:
|
||||
request.session.clear()
|
||||
raise refused(401, "not_authenticated")
|
||||
_renew_session(request)
|
||||
# Attribute this request's log lines to the account. Here, not in the request middleware:
|
||||
|
||||
@@ -205,7 +205,10 @@ def test_a_cookie_from_before_a_sign_in_as_another_account_is_refused(client, ma
|
||||
assert _login(client, "late2b@example.com", _PW).status_code == 200
|
||||
bob = client.cookies.get("session")
|
||||
_hold(client, in_flight)
|
||||
assert client.get(_PROTECTED).status_code == 401
|
||||
r = client.get(_PROTECTED)
|
||||
assert r.status_code == 401
|
||||
# The refusal deletes no cookie: landing after Bob's, a deletion would sign Bob out.
|
||||
assert "set-cookie" not in r.headers
|
||||
_hold(client, bob) # Bob's own session is untouched
|
||||
assert client.get("/api/me").json()["email"] == "late2b@example.com"
|
||||
|
||||
@@ -225,6 +228,28 @@ def test_a_session_from_before_session_ids_gets_one_and_its_sign_out_holds(clien
|
||||
assert client.get(_PROTECTED).status_code == 401
|
||||
|
||||
|
||||
def test_a_revoke_drops_only_the_revocations_no_cookie_can_outlive(db):
|
||||
# Each revoke prunes the table: a row goes once no cookie carrying its id can still be valid, and
|
||||
# never sooner — a later sign-out must not undo an earlier one.
|
||||
from datetime import datetime, timezone
|
||||
from types import SimpleNamespace
|
||||
|
||||
from sqlalchemy import select
|
||||
|
||||
from app.auth import SESSION_MAX_AGE, revoke_session
|
||||
from app.models import RevokedSession
|
||||
|
||||
now = datetime.now(timezone.utc)
|
||||
life = timedelta(seconds=SESSION_MAX_AGE)
|
||||
db.add(RevokedSession(sid="outlived", revoked_at=now - life - timedelta(days=2)))
|
||||
db.add(RevokedSession(sid="still-live", revoked_at=now - life + timedelta(hours=1)))
|
||||
db.commit()
|
||||
revoke_session(db, SimpleNamespace(session={"user_id": 1, "sid": "this-one"}, cookies={}))
|
||||
db.commit()
|
||||
left = set(db.scalars(select(RevokedSession.sid)).all())
|
||||
assert left == {"still-live", "this-one"}
|
||||
|
||||
|
||||
def _session(client) -> dict:
|
||||
"""The decoded payload of the client's session cookie."""
|
||||
return _decode(client.cookies.get("session"))
|
||||
|
||||
Reference in New Issue
Block a user