fix(auth): a cookie from before session ids signs in once more; a session's age is its renewal
A sign-out sent with a cookie that had no session id revoked nothing, so a request of its still in flight signed the browser back in; its id, drawn from the raw cookie, also changed whenever the cookie was re-signed. Such a cookie is now refused, like one with no epoch: every one would have run out within 14 days anyway. And any route that writes the session re-signs the cookie (the anonymous Google sign-in start too), so a refused one could be kept fresh past the row revoking it: a session is now as old as its renewed stamp, which only a sign-in and a renewal write.
This commit is contained in:
+26
-22
@@ -1,4 +1,3 @@
|
||||
import hashlib
|
||||
import secrets
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
@@ -282,14 +281,24 @@ def stamp_session(request: Request, user: User) -> None:
|
||||
# (`revoke_session`). Landing after "log out everywhere" or a password change re-stamped this
|
||||
# browser's cookie, it carries the old epoch and signs this browser out: that race fails closed (sign
|
||||
# in again; nothing is reached), and is left so.
|
||||
#
|
||||
# The session's age is its `renewed` stamp, not the cookie's signature: any route that writes the
|
||||
# session re-signs the cookie (an anonymous one too — the Google sign-in start stores its state
|
||||
# there), so a refused cookie could be kept fresh forever. `renewed` is written only by a sign-in and
|
||||
# by a renewal, and a refused session gets neither, so it dies `SESSION_MAX_AGE` after its last one.
|
||||
SESSION_MAX_AGE = 14 * 86400 # the cookie's life; `main` hands it to SessionMiddleware
|
||||
_RENEW_AFTER_SECONDS = 86400
|
||||
|
||||
|
||||
def _session_expired(request: Request) -> bool:
|
||||
renewed = request.session.get("renewed")
|
||||
now = int(datetime.now(timezone.utc).timestamp())
|
||||
return not isinstance(renewed, int) or now - renewed > SESSION_MAX_AGE
|
||||
|
||||
|
||||
def _renew_session(request: Request) -> None:
|
||||
now = int(datetime.now(timezone.utc).timestamp())
|
||||
renewed = request.session.get("renewed")
|
||||
if not isinstance(renewed, int) or now - renewed >= _RENEW_AFTER_SECONDS:
|
||||
if now - request.session["renewed"] >= _RENEW_AFTER_SECONDS:
|
||||
request.session["renewed"] = now
|
||||
|
||||
|
||||
@@ -297,23 +306,11 @@ def new_session_id() -> str:
|
||||
return secrets.token_urlsafe(16)
|
||||
|
||||
|
||||
def _session_id(request: Request) -> str:
|
||||
"""This session's id. A cookie from before session ids gets one drawn from the cookie itself, so
|
||||
parallel requests carrying it agree on the id a sign-out revokes, and two browsers do not."""
|
||||
sid = request.session.get("sid")
|
||||
if isinstance(sid, str):
|
||||
return sid
|
||||
raw = request.cookies.get("session", "")
|
||||
sid = hashlib.sha256(raw.encode()).hexdigest()[:22]
|
||||
request.session["sid"] = sid
|
||||
return sid
|
||||
|
||||
|
||||
def revoke_session(db: Session, request: Request) -> None:
|
||||
"""Revoke this browser's session id, so a cookie carrying it that lands later is refused. Not
|
||||
committed — the caller commits. A row is dropped once no cookie with its id can still be valid:
|
||||
a refused cookie is never renewed, so it dies `SESSION_MAX_AGE` after it was signed, at most a
|
||||
request's time after the revoke (a day of margin)."""
|
||||
a refused session is never renewed, so it expires `SESSION_MAX_AGE` after its `renewed` stamp,
|
||||
which is no later than the revoke (a day of margin)."""
|
||||
sid = request.session.get("sid")
|
||||
if not isinstance(sid, str):
|
||||
return
|
||||
@@ -368,11 +365,18 @@ 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. 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:
|
||||
# A session signed out (or replaced by a sign-in as someone else) whose cookie landed late; one
|
||||
# with no id, which only a build before session ids wrote (its holder signs in once more); or one
|
||||
# not renewed for its whole life. 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. Such a session is refused on every request and never renewed, so leaving
|
||||
# its cookie costs nothing.
|
||||
sid = request.session.get("sid")
|
||||
if (
|
||||
not isinstance(sid, str)
|
||||
or _session_expired(request)
|
||||
or db.get(RevokedSession, sid) is not None
|
||||
):
|
||||
raise refused(401, "not_authenticated")
|
||||
_renew_session(request)
|
||||
# Attribute this request's log lines to the account. Here, not in the request middleware:
|
||||
|
||||
Reference in New Issue
Block a user