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:
2026-07-12 04:34:07 +02:00
parent 9fa120b09e
commit ca10b88bf5
3 changed files with 54 additions and 15 deletions
+32 -13
View File
@@ -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")
+9 -2
View File
@@ -142,10 +142,17 @@ def send_account_suspended(to: str, operator: str | None) -> bool:
def send_admin_new_request(admins: list[str], requester: str) -> bool:
# The link deep-links straight to the admin Access-requests page (the SPA reads ?admin=…). The
# requester address is spelled out (mail clients auto-link both the URL and the address), and the
# reply note is accurate: _send sets Reply-To to the requester, so a reply reaches THEM — not the
# sending mailbox. (The old copy said "Settings → Account", which was the wrong menu.)
approve_url = f"{settings.app_base}/?admin=access-requests"
body = (
f"{requester} just requested access to Siftlode.\n\n"
"Open Siftlode → Settings → Account → Access requests to approve or deny it.\n\n"
"(Reply to this email to reach the requester directly.)\n"
f"Approve or deny it here:\n{approve_url}\n"
f"(or in Siftlode: open Users → Access requests)\n\n"
f"Requester's email: {requester}\n"
f"Replying to this message goes straight to the requester (it's the Reply-To address).\n"
)
return _send(admins, f"Siftlode access request from {requester}", body, reply_to=requester)
+13
View File
@@ -528,6 +528,19 @@ export default function App() {
stripUrlParams();
}, []); // eslint-disable-line react-hooks/exhaustive-deps
// Deep-link from the "new access request" admin email (?admin=access-requests) → jump straight to
// the admin Users → Access requests view. Snapshot the param so the pre-login URL strip doesn't
// drop it; act once `me` is known, and only for an admin (others just land on their default page).
const [adminDeepLink] = useState(() => new URLSearchParams(window.location.search).get("admin"));
useEffect(() => {
if (adminDeepLink !== "access-requests" || !meQuery.data) return;
if (meQuery.data.role === "admin") {
focusAccessRequestsTab();
setPage("users");
}
stripUrlParams();
}, [adminDeepLink, meQuery.data?.id]); // eslint-disable-line react-hooks/exhaustive-deps
// First-login onboarding: prompt the user to connect YouTube (and resume the flow after
// each consent redirect). Derived from granted scopes + storage flags so it's stable
// across the full-page OAuth round-trip; dismissible and reopenable from Settings.