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.
This commit is contained in:
npeter83 2026-06-13 23:56:34 +02:00
parent 01ebdee8bc
commit 580d7f0e0d
6 changed files with 81 additions and 17 deletions

View file

@ -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,
)

View file

@ -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()

View file

@ -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:

View file

@ -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 {},

View file

@ -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)