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:
parent
9163a5f68e
commit
4765db89de
6 changed files with 81 additions and 17 deletions
|
|
@ -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,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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 {},
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue