fix(auth): preserve OAuth scopes across re-login + accurate multi-admin request emails
Two user-reported signin bugs:
1) A plain re-login (or logout→login) wiped the user's YouTube grant: login requests
only BASE_SCOPES and Google's returned `scope` can list just those, but _store_token
wrote it verbatim — dropping a previously-granted youtube.readonly/youtube scope, so
can_read flipped to false and the feed demanded a reconnect. The underlying grant
(refresh token) survives such a login, so UNION the scopes instead of narrowing; they
only shrink on full disconnect (purge deletes the token row).
2) Admin 'new access request' email: (a) it named the wrong menu ('Settings → Account'
instead of Users → Access requests); (b) the requester address was invisible so the
'reply reaches them' note looked wrong (Reply-To is in fact set to the requester) —
now spelled out + a deep-link straight to the approve view (?admin=access-requests,
handled in App); (c) it emailed only the static env ADMIN_EMAILS — now notifies the
ACTUAL admins (active role=admin users) unioned with env, so a UI-promoted admin (who
CAN approve — the approve UI is role-gated) is notified too.
This commit is contained in:
parent
3f58ad0d84
commit
fa0d0bceaf
3 changed files with 54 additions and 15 deletions
|
|
@ -132,6 +132,20 @@ def has_write_scope(user: User) -> bool:
|
|||
return WRITE_SCOPE in tok.scopes.split()
|
||||
|
||||
|
||||
def admin_notify_emails(db: Session) -> list[str]:
|
||||
"""Who to email about admin events (e.g. a new access request). The ACTUAL admins — every active,
|
||||
non-suspended `role=admin` user — UNIONED with the env ADMIN_EMAILS bootstrap list. Using the DB
|
||||
roles means an admin promoted through the UI is notified too (env alone would miss them, yet they
|
||||
CAN approve requests — the approve UI is role-gated, so the notify set must match); the env union
|
||||
keeps a freshly-seeded instance working before any DB admin exists."""
|
||||
db_admins = db.execute(
|
||||
select(User.email).where(
|
||||
User.role == "admin", User.is_active.is_(True), User.is_suspended.is_(False)
|
||||
)
|
||||
).scalars()
|
||||
return sorted({e.lower() for e in db_admins if e} | settings.admin_email_set)
|
||||
|
||||
|
||||
def is_allowed(db: Session, email: str) -> bool:
|
||||
"""May this email sign in? An approved Invite is the source of truth; the env
|
||||
ALLOWED_EMAILS/ADMIN_EMAILS remain a bootstrap fallback (e.g. a fresh DB)."""
|
||||
|
|
@ -282,12 +296,9 @@ async def callback(
|
|||
# land the user on a friendly "request received" screen instead of a raw 403.
|
||||
if email:
|
||||
inv = upsert_pending_invite(db, email)
|
||||
if inv is not None and settings.admin_email_set:
|
||||
background.add_task(
|
||||
email_mod.send_admin_new_request,
|
||||
sorted(settings.admin_email_set),
|
||||
email,
|
||||
)
|
||||
admins = admin_notify_emails(db)
|
||||
if inv is not None and admins:
|
||||
background.add_task(email_mod.send_admin_new_request, admins, email)
|
||||
return RedirectResponse(url="/?access=requested")
|
||||
return RedirectResponse(url="/?access=denied")
|
||||
|
||||
|
|
@ -383,7 +394,15 @@ def _store_token(db: Session, user: User, token: dict) -> None:
|
|||
tok.access_token = encrypt(token.get("access_token"))
|
||||
expires_at = token.get("expires_at")
|
||||
tok.expiry = datetime.fromtimestamp(expires_at, tz=timezone.utc) if expires_at else None
|
||||
tok.scopes = token.get("scope") or BASE_SCOPES
|
||||
# UNION the scopes, never narrow them. A plain sign-in requests only BASE_SCOPES, and Google's
|
||||
# returned `scope` can list just those rather than the union of everything granted — so writing it
|
||||
# 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)
|
||||
|
||||
|
||||
|
|
@ -495,10 +514,9 @@ async def request_access(
|
|||
if is_allowed(db, email):
|
||||
return {"status": "approved"} # already allowed — just sign in
|
||||
inv = upsert_pending_invite(db, email)
|
||||
if inv is not None and settings.admin_email_set:
|
||||
background.add_task(
|
||||
email_mod.send_admin_new_request, sorted(settings.admin_email_set), email
|
||||
)
|
||||
admins = admin_notify_emails(db)
|
||||
if inv is not None and admins:
|
||||
background.add_task(email_mod.send_admin_new_request, admins, email)
|
||||
return {"status": "pending"}
|
||||
|
||||
|
||||
|
|
@ -629,8 +647,9 @@ def _register_account(email: str, password: str) -> None:
|
|||
raw = _issue_token(db, user, "verify", VERIFY_TTL)
|
||||
# Fragment (#), not a query token: keeps the token out of proxy/access logs + Referer (SB3).
|
||||
email_mod.send_verify_email(email, f"{_app_base()}/#verify={raw}")
|
||||
if settings.admin_email_set:
|
||||
email_mod.send_admin_new_request(sorted(settings.admin_email_set), email)
|
||||
admins = admin_notify_emails(db)
|
||||
if admins:
|
||||
email_mod.send_admin_new_request(admins, email)
|
||||
|
||||
|
||||
@router.post("/verify")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue