diff --git a/backend/app/auth.py b/backend/app/auth.py index a63afa6..5f814d6 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -73,6 +73,8 @@ READ_SCOPE = f"{WRITE_SCOPE}.readonly" BASE_SCOPES = "openid email profile" READ_SCOPES = f"{BASE_SCOPES} {READ_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") @@ -132,6 +134,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 +298,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") @@ -376,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 reflects read/write upgrades correctly.""" 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"): tok.refresh_token_enc = encrypt(token["refresh_token"]) # Encrypt the access token at rest too (it's a live ~1h bearer credential), matching the @@ -383,7 +408,8 @@ 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 (never narrow): a broader-or-equal exchange may still list only the newly-added scope. + tok.scopes = " ".join(sorted(old | new)) or BASE_SCOPES db.add(tok) @@ -414,7 +440,9 @@ def purge_user(db: Session, user: User, background: BackgroundTasks) -> None: email = user.email.lower() user_id = user.id tok = user.token - google_token = (decrypt(tok.refresh_token_enc) or tok.access_token) if tok else None + # Both are stored encrypted — decrypt for the revoke call (the access-token fallback was passing + # ciphertext, so a token row with no refresh token silently failed to revoke). + google_token = (decrypt(tok.refresh_token_enc) or decrypt(tok.access_token)) if tok else None # Raw delete so Postgres applies ON DELETE CASCADE / SET NULL on every dependent table. db.execute(delete(User).where(User.id == user_id)) db.execute(delete(Invite).where(Invite.email == email)) @@ -495,10 +523,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 +656,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") diff --git a/backend/app/email.py b/backend/app/email.py index da0eea4..4f5c108 100644 --- a/backend/app/email.py +++ b/backend/app/email.py @@ -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) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 6d64895..68d1339 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -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.