From 4765db89de289e59881ec9a4e35f046641445332 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 13 Jun 2026 23:56:34 +0200 Subject: [PATCH] feat(auth): split base sign-in from YouTube scopes for incremental onboarding Base login now requests only openid/email/profile (non-sensitive), so a new user gets a clean Google consent with no "unverified app" warning and no 7-day refresh token expiry. YouTube read (youtube.readonly) and write (youtube) are granted later by the onboarding wizard via a parameterized /auth/upgrade?access=read|write. Security fixes folded in from the baseline audit: - config: refuse to boot in production (https OAUTH_REDIRECT_URL) with the placeholder/short SECRET_KEY or a missing TOKEN_ENCRYPTION_KEY, closing a session-forgery / admin-impersonation hole. - main: mark the session cookie Secure when served over HTTPS. - me: expose can_read; sync/subscriptions returns a friendly 403 (not a 500) until YouTube read access is granted. --- .env.example | 2 ++ backend/app/auth.py | 48 ++++++++++++++++++++++++++++---------- backend/app/config.py | 33 +++++++++++++++++++++++++- backend/app/main.py | 4 ++-- backend/app/routes/me.py | 3 ++- backend/app/routes/sync.py | 8 ++++++- 6 files changed, 81 insertions(+), 17 deletions(-) diff --git a/.env.example b/.env.example index 5dafc3c..5c796e5 100644 --- a/.env.example +++ b/.env.example @@ -9,6 +9,8 @@ POSTGRES_DB=subfeed APP_PORT=8080 # Session signing key. Generate with: python -c "import secrets;print(secrets.token_urlsafe(48))" +# In production (an https OAUTH_REDIRECT_URL) the app refuses to start with this placeholder +# or any key shorter than 32 chars — a known signing key would let anyone forge a session. SECRET_KEY=change-me-session-key # Fernet key for encrypting stored OAuth refresh tokens. Generate with: diff --git a/backend/app/auth.py b/backend/app/auth.py index d2f5e2f..c5b086b 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -16,12 +16,19 @@ from app.security import encrypt _EMAIL_RE = re.compile(r"^[^@\s]+@[^@\s]+\.[^@\s]+$") -# YouTube's full scope (read + write: unsubscribe, playlist export) and its read-only -# counterpart. Default login asks for read-only; write is an explicit opt-in via -# /auth/upgrade (incremental consent), so friends who won't grant write can still browse. +# Base sign-in requests only the non-sensitive OpenID scopes (profile/email). These do +# NOT trigger Google's "unverified app" warning and do NOT expire after 7 days, so a new +# user gets a clean, familiar consent. YouTube access is granted later, one step at a +# time, by the onboarding wizard via /auth/upgrade (incremental authorization): +# - read -> youtube.readonly (read subscriptions, build the feed) +# - write -> youtube (full: unsubscribe / playlist export) +# Both YouTube scopes are "sensitive"; the wizard explains the Google warning before +# sending the user there, so the extra permission never feels like a surprise. WRITE_SCOPE = "https://www.googleapis.com/auth/youtube" -READ_SCOPES = f"openid email profile {WRITE_SCOPE}.readonly" -WRITE_SCOPES = f"openid email profile {WRITE_SCOPE}" +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}" log = logging.getLogger("subfeed.auth") @@ -33,10 +40,20 @@ oauth.register( client_id=settings.google_client_id, client_secret=settings.google_client_secret, server_metadata_url="https://accounts.google.com/.well-known/openid-configuration", - client_kwargs={"scope": READ_SCOPES}, + client_kwargs={"scope": BASE_SCOPES}, ) +def has_read_scope(user: User) -> bool: + """Whether the user's stored grant lets us read their YouTube data (build the feed). + The full write scope is a superset, so it implies read.""" + tok = user.token + if tok is None or not tok.scopes: + return False + granted = tok.scopes.split() + return READ_SCOPE in granted or WRITE_SCOPE in granted + + def has_write_scope(user: User) -> bool: """Whether the user's stored grant includes YouTube write (unsubscribe / export).""" tok = user.token @@ -91,7 +108,7 @@ async def login(request: Request): access_type="offline", prompt="select_account", include_granted_scopes="true", - scope=READ_SCOPES, + scope=BASE_SCOPES, ) @@ -145,7 +162,9 @@ async def callback( tok.expiry = ( datetime.fromtimestamp(expires_at, tz=timezone.utc) if expires_at else None ) - tok.scopes = token.get("scope") or READ_SCOPES + # include_granted_scopes=true means Google returns the union of all scopes the user + # has ever granted this app, so this correctly reflects read/write upgrades too. + tok.scopes = token.get("scope") or BASE_SCOPES db.add(tok) db.commit() @@ -193,16 +212,21 @@ def current_user(request: Request, db: Session = Depends(get_db)) -> User: @router.get("/upgrade") -async def upgrade(request: Request, user: User = Depends(current_user)): - """Incremental consent: re-authorize with the full YouTube (write) scope. The shared - callback stores the new scope set, so `can_write` flips on once Google grants it.""" +async def upgrade( + request: Request, access: str = "write", user: User = Depends(current_user) +): + """Incremental consent for the onboarding wizard. `access=read` grants YouTube + read-only (enough to build the feed); `access=write` additionally grants management + (unsubscribe). The shared callback stores whatever Google returns, so `can_read` / + `can_write` flip on once granted.""" + scope = READ_SCOPES if access == "read" else WRITE_SCOPES return await oauth.google.authorize_redirect( request, settings.oauth_redirect_url, access_type="offline", prompt="consent", include_granted_scopes="true", - scope=WRITE_SCOPES, + scope=scope, ) diff --git a/backend/app/config.py b/backend/app/config.py index b99a66f..91bbb49 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -1,5 +1,9 @@ +from pydantic import model_validator from pydantic_settings import BaseSettings, SettingsConfigDict +# Shipped placeholder; production must override it. Used by the startup guard below. +_DEFAULT_SECRET_KEY = "change-me-session-key" + class Settings(BaseSettings): model_config = SettingsConfigDict(env_file=".env", extra="ignore") @@ -9,7 +13,7 @@ class Settings(BaseSettings): database_url: str = "postgresql+psycopg://subfeed:subfeed@db:5432/subfeed" # Session cookie signing key. - secret_key: str = "change-me-session-key" + secret_key: str = _DEFAULT_SECRET_KEY # Fernet key (urlsafe base64, 32 bytes) for encrypting stored refresh tokens. token_encryption_key: str = "" @@ -77,5 +81,32 @@ class Settings(BaseSettings): def admin_email_set(self) -> set[str]: return {e.strip().lower() for e in self.admin_emails.split(",") if e.strip()} + @property + def session_https_only(self) -> bool: + """Mark the session cookie Secure when we're served over HTTPS. We treat an + https OAUTH_REDIRECT_URL as the signal for 'real deployment' vs local dev.""" + return self.oauth_redirect_url.lower().startswith("https://") + + @model_validator(mode="after") + def _enforce_production_secrets(self) -> "Settings": + """Fail fast rather than boot a public instance with the shipped placeholder + signing key (which would let anyone forge an admin session) or without token + encryption. Local dev (http redirect URL) keeps the convenient defaults.""" + if not self.session_https_only: + return self # local/dev: allow the placeholder defaults + if self.secret_key == _DEFAULT_SECRET_KEY or len(self.secret_key) < 32: + raise ValueError( + "SECRET_KEY must be a unique random value of at least 32 chars in " + "production (an https OAUTH_REDIRECT_URL was detected). Generate one " + 'with: python -c "import secrets; print(secrets.token_urlsafe(48))"' + ) + if not self.token_encryption_key: + raise ValueError( + "TOKEN_ENCRYPTION_KEY must be set in production so refresh tokens are " + "encrypted at rest. Generate one with: " + 'python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())"' + ) + return self + settings = Settings() diff --git a/backend/app/main.py b/backend/app/main.py index 9c4b801..969a909 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -46,8 +46,8 @@ app = FastAPI(title=settings.app_name, lifespan=lifespan) app.add_middleware( SessionMiddleware, secret_key=settings.secret_key, - same_site="lax", - https_only=False, + same_site="lax", # required so the cookie rides the OAuth redirect back from Google + https_only=settings.session_https_only, # Secure flag when served over HTTPS (prod) ) if settings.frontend_origin: diff --git a/backend/app/routes/me.py b/backend/app/routes/me.py index 1da878d..28312e2 100644 --- a/backend/app/routes/me.py +++ b/backend/app/routes/me.py @@ -2,7 +2,7 @@ from fastapi import APIRouter, Depends from sqlalchemy import func, select from sqlalchemy.orm import Session -from app.auth import current_user, has_write_scope +from app.auth import current_user, has_read_scope, has_write_scope from app.db import get_db from app.models import Invite, User @@ -29,6 +29,7 @@ def get_me( "display_name": user.display_name, "avatar_url": user.avatar_url, "role": user.role, + "can_read": has_read_scope(user), "can_write": has_write_scope(user), "pending_invites": pending_invites, "preferences": user.preferences or {}, diff --git a/backend/app/routes/sync.py b/backend/app/routes/sync.py index 55dadfb..ce2d155 100644 --- a/backend/app/routes/sync.py +++ b/backend/app/routes/sync.py @@ -3,7 +3,7 @@ from sqlalchemy import and_, case, func, select, update from sqlalchemy.orm import Session from app import quota, state -from app.auth import current_user +from app.auth import current_user, has_read_scope from app.db import get_db from app.models import Channel, Subscription, User, Video from app.sync.runner import ( @@ -33,6 +33,12 @@ def _user_channels(db: Session, user: User) -> list[Channel]: def sync_subscriptions( user: User = Depends(current_user), db: Session = Depends(get_db) ) -> dict: + if not has_read_scope(user): + # No YouTube read grant yet — the onboarding wizard hasn't been completed. + raise HTTPException( + status_code=403, + detail="Connect your YouTube account (read access) to import subscriptions.", + ) before = quota.units_used_today(db) with quota.attribute(user.id, "sync_subscriptions"): result = import_subscriptions(db, user)