fix(auth): don't let a re-sign-in clobber the YouTube grant (root cause)
Correcting the earlier scope-union-only fix. The real damage from a logout→login isn't just the stored `scope` field — verified via a live token refresh that the stored refresh token itself had been REPLACED with a base-only one: a plain sign-in requests only BASE_SCOPES and Google handed back a fresh base-only access+refresh token, which _store_token adopted verbatim, destroying the read/write grant (the old refresh token stays valid on Google's side, so overwriting it is what loses access). _store_token now detects when an exchange would NARROW our YouTube scopes and, in that case, keeps the WHOLE existing grant (refresh token, access token, scopes) untouched. Broader-or-equal exchanges (first grant, read→write upgrade) adopt + union as before. Unit-verified across base-relogin / upgrade / new-user / base-user cases.
This commit is contained in:
parent
fa0d0bceaf
commit
9e6c90bcaf
1 changed files with 16 additions and 9 deletions
|
|
@ -73,6 +73,8 @@ READ_SCOPE = f"{WRITE_SCOPE}.readonly"
|
||||||
BASE_SCOPES = "openid email profile"
|
BASE_SCOPES = "openid email profile"
|
||||||
READ_SCOPES = f"{BASE_SCOPES} {READ_SCOPE}"
|
READ_SCOPES = f"{BASE_SCOPES} {READ_SCOPE}"
|
||||||
WRITE_SCOPES = f"{BASE_SCOPES} {READ_SCOPE} {WRITE_SCOPE}"
|
WRITE_SCOPES = f"{BASE_SCOPES} {READ_SCOPE} {WRITE_SCOPE}"
|
||||||
|
# The YouTube grants (read ⊂ write). A sign-in must never silently drop one of these — see _store_token.
|
||||||
|
_YOUTUBE_SCOPES = frozenset({READ_SCOPE, WRITE_SCOPE})
|
||||||
|
|
||||||
log = logging.getLogger("siftlode.auth")
|
log = logging.getLogger("siftlode.auth")
|
||||||
|
|
||||||
|
|
@ -387,6 +389,18 @@ def _store_token(db: Session, user: User, token: dict) -> None:
|
||||||
include_granted_scopes=true means `scope` is the union of everything granted, so this
|
include_granted_scopes=true means `scope` is the union of everything granted, so this
|
||||||
reflects read/write upgrades correctly."""
|
reflects read/write upgrades correctly."""
|
||||||
tok = user.token or OAuthToken(user=user)
|
tok = user.token or OAuthToken(user=user)
|
||||||
|
old = set((tok.scopes or "").split())
|
||||||
|
new = set((token.get("scope") or "").split())
|
||||||
|
# A plain re-sign-in requests only BASE_SCOPES, and Google can hand back a FRESH base-only
|
||||||
|
# access+refresh token. Adopting it would DESTROY a previously-granted YouTube read/write grant —
|
||||||
|
# can_read/can_write flip to false and the feed demands a reconnect (the exact bug users hit on a
|
||||||
|
# logout→login). The OLD refresh token stays valid on Google's side, so when this exchange would
|
||||||
|
# NARROW our YouTube scopes, keep the WHOLE existing grant (refresh token, access token, scopes)
|
||||||
|
# untouched. The grant only shrinks on a full disconnect (purge_user deletes the token row); an
|
||||||
|
# externally-revoked scope surfaces later as a YouTube API 403 → reconnect, not through this write.
|
||||||
|
if (old & _YOUTUBE_SCOPES) - new:
|
||||||
|
db.add(tok)
|
||||||
|
return
|
||||||
if token.get("refresh_token"):
|
if token.get("refresh_token"):
|
||||||
tok.refresh_token_enc = encrypt(token["refresh_token"])
|
tok.refresh_token_enc = encrypt(token["refresh_token"])
|
||||||
# Encrypt the access token at rest too (it's a live ~1h bearer credential), matching the
|
# Encrypt the access token at rest too (it's a live ~1h bearer credential), matching the
|
||||||
|
|
@ -394,15 +408,8 @@ def _store_token(db: Session, user: User, token: dict) -> None:
|
||||||
tok.access_token = encrypt(token.get("access_token"))
|
tok.access_token = encrypt(token.get("access_token"))
|
||||||
expires_at = token.get("expires_at")
|
expires_at = token.get("expires_at")
|
||||||
tok.expiry = datetime.fromtimestamp(expires_at, tz=timezone.utc) if expires_at else None
|
tok.expiry = datetime.fromtimestamp(expires_at, tz=timezone.utc) if expires_at else None
|
||||||
# UNION the scopes, never narrow them. A plain sign-in requests only BASE_SCOPES, and Google's
|
# Union (never narrow): a broader-or-equal exchange may still list only the newly-added scope.
|
||||||
# returned `scope` can list just those rather than the union of everything granted — so writing it
|
tok.scopes = " ".join(sorted(old | new)) or BASE_SCOPES
|
||||||
# verbatim would DROP a previously-granted YouTube read/write scope on every re-login, wrongly
|
|
||||||
# flipping can_read to false (the feed asks to reconnect). The underlying grant (refresh token)
|
|
||||||
# survives such a login, so we keep the broadest set we've seen. Scopes only shrink when the whole
|
|
||||||
# grant is torn down (purge_user deletes the token row); an externally-revoked scope surfaces as a
|
|
||||||
# YouTube API 403 → the reconnect prompt, not via this field.
|
|
||||||
returned = (token.get("scope") or BASE_SCOPES).split()
|
|
||||||
tok.scopes = " ".join(sorted(set((tok.scopes or "").split()) | set(returned)))
|
|
||||||
db.add(tok)
|
db.add(tok)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue