refactor(auth): centralize token hashing, email validation & admin gating
- app/utils.py: shared valid_email() + now_utc() (one email regex, was duplicated in auth.py and admin.py with a looser "@"-in check elsewhere). - security.hash_token(): one SHA-256 token hasher (was duplicated in auth.py + state.py). - auth.admin_user dependency + count_admins() helper, replacing the inline role checks and the last-admin count query repeated across admin.py/me.py/tags.py.
This commit is contained in:
parent
8ef748c7b8
commit
63701f5344
7 changed files with 84 additions and 65 deletions
|
|
@ -1,6 +1,4 @@
|
|||
import hashlib
|
||||
import logging
|
||||
import re
|
||||
import secrets
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
|
|
@ -8,7 +6,7 @@ import httpx
|
|||
from authlib.integrations.starlette_client import OAuth, OAuthError
|
||||
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request
|
||||
from fastapi.responses import JSONResponse, RedirectResponse
|
||||
from sqlalchemy import delete, select
|
||||
from sqlalchemy import delete, func, select
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app import email as email_mod
|
||||
|
|
@ -17,7 +15,8 @@ from app.config import settings
|
|||
from app.db import get_db
|
||||
from app.models import AuthToken, DemoWhitelist, Invite, OAuthToken, User
|
||||
from app.ratelimit import RateLimiter
|
||||
from app.security import decrypt, encrypt, hash_password, verify_password
|
||||
from app.security import decrypt, encrypt, hash_password, hash_token, verify_password
|
||||
from app.utils import valid_email
|
||||
|
||||
# Email+password auth tuning.
|
||||
PASSWORD_MIN_LEN = 10
|
||||
|
|
@ -43,8 +42,6 @@ def _notify_suspended(background: BackgroundTasks, email: str) -> None:
|
|||
if _suspend_email_limiter.allow(email):
|
||||
background.add_task(email_mod.send_account_suspended, email, operator_contact())
|
||||
|
||||
_EMAIL_RE = re.compile(r"^[^@\s]+@[^@\s]+\.[^@\s]+$")
|
||||
|
||||
# The single shared demo account's stable identity. The row is created lazily on first
|
||||
# demo login (see get_or_create_demo_user); these are just its sentinel keys.
|
||||
DEMO_GOOGLE_SUB = "__demo__"
|
||||
|
|
@ -184,7 +181,7 @@ def upsert_pending_invite(db: Session, email: str) -> Invite | None:
|
|||
"""Idempotently record an access request. Returns the Invite if a *new* pending row
|
||||
was created (so the caller can notify admins), else None for already-known emails."""
|
||||
email = (email or "").lower()
|
||||
if not _EMAIL_RE.match(email):
|
||||
if not valid_email(email):
|
||||
return None
|
||||
inv = db.execute(select(Invite).where(Invite.email == email)).scalar_one_or_none()
|
||||
if inv is not None:
|
||||
|
|
@ -443,7 +440,7 @@ async def request_access(
|
|||
"""Public: ask for access. Idempotent — repeat requests for the same email don't
|
||||
duplicate or re-spam admins (upsert returns None for an already-pending row)."""
|
||||
email = (payload.get("email") or "").strip().lower()
|
||||
if not _EMAIL_RE.match(email):
|
||||
if not valid_email(email):
|
||||
raise HTTPException(status_code=400, detail="Enter a valid email address.")
|
||||
if is_allowed(db, email):
|
||||
return {"status": "approved"} # already allowed — just sign in
|
||||
|
|
@ -465,7 +462,7 @@ def demo_login(payload: dict, request: Request, db: Session = Depends(get_db)) -
|
|||
if not _demo_limiter.allow(_client_ip(request)):
|
||||
return {"authenticated": False}
|
||||
email = (payload.get("email") or "").strip().lower()
|
||||
if _EMAIL_RE.match(email) and is_demo_allowed(db, email):
|
||||
if valid_email(email) and is_demo_allowed(db, email):
|
||||
demo = get_or_create_demo_user(db)
|
||||
request.session["user_id"] = demo.id
|
||||
log.info("Demo login (key=%s, demo_id=%s)", email, demo.id)
|
||||
|
|
@ -487,10 +484,6 @@ def _app_base() -> str:
|
|||
return settings.app_base
|
||||
|
||||
|
||||
def _hash_token(raw: str) -> str:
|
||||
return hashlib.sha256(raw.encode()).hexdigest()
|
||||
|
||||
|
||||
def _issue_token(db: Session, user: User, kind: str, ttl: timedelta) -> str:
|
||||
"""Create a single-use token of `kind`; only its hash is stored. Returns the raw token
|
||||
(goes only into the emailed link)."""
|
||||
|
|
@ -499,7 +492,7 @@ def _issue_token(db: Session, user: User, kind: str, ttl: timedelta) -> str:
|
|||
AuthToken(
|
||||
user_id=user.id,
|
||||
kind=kind,
|
||||
token_hash=_hash_token(raw),
|
||||
token_hash=hash_token(raw),
|
||||
expires_at=datetime.now(timezone.utc) + ttl,
|
||||
)
|
||||
)
|
||||
|
|
@ -513,7 +506,7 @@ def _consume_token(db: Session, raw: str | None, kind: str) -> User | None:
|
|||
return None
|
||||
row = db.execute(
|
||||
select(AuthToken).where(
|
||||
AuthToken.token_hash == _hash_token(raw), AuthToken.kind == kind
|
||||
AuthToken.token_hash == hash_token(raw), AuthToken.kind == kind
|
||||
)
|
||||
).scalar_one_or_none()
|
||||
if row is None or row.used_at is not None or row.expires_at < datetime.now(timezone.utc):
|
||||
|
|
@ -537,7 +530,7 @@ def register(
|
|||
email = (payload.get("email") or "").strip().lower()
|
||||
password = payload.get("password") or ""
|
||||
# Input validation is safe to reveal (independent of whether the email exists).
|
||||
if not _EMAIL_RE.match(email):
|
||||
if not valid_email(email):
|
||||
raise HTTPException(status_code=400, detail="Enter a valid email address.")
|
||||
if len(password) < PASSWORD_MIN_LEN:
|
||||
raise HTTPException(
|
||||
|
|
@ -633,7 +626,7 @@ def password_reset_request(
|
|||
if not _reset_limiter.allow(_client_ip(request)):
|
||||
return {"status": "ok"}
|
||||
email = (payload.get("email") or "").strip().lower()
|
||||
if _EMAIL_RE.match(email):
|
||||
if valid_email(email):
|
||||
user = db.execute(select(User).where(User.email == email)).scalar_one_or_none()
|
||||
if user is not None and user.password_hash and not user.is_demo:
|
||||
raw = _issue_token(db, user, "reset", RESET_TTL)
|
||||
|
|
@ -694,6 +687,23 @@ def require_human(user: User = Depends(current_user)) -> User:
|
|||
return user
|
||||
|
||||
|
||||
def admin_user(user: User = Depends(current_user)) -> User:
|
||||
"""Dependency: require the signed-in user to be an admin. Single source of truth for the
|
||||
admin gate — every admin route/action depends on this rather than re-checking the role."""
|
||||
if user.role != "admin":
|
||||
raise HTTPException(status_code=403, detail="Admin only")
|
||||
return user
|
||||
|
||||
|
||||
def count_admins(db: Session, *, active_only: bool = False) -> int:
|
||||
"""How many admins exist (optionally only un-suspended ones). Used by the 'can't remove /
|
||||
suspend / delete the last admin' guards so they don't each re-spell the count query."""
|
||||
q = select(func.count()).select_from(User).where(User.role == "admin")
|
||||
if active_only:
|
||||
q = q.where(User.is_suspended.is_(False))
|
||||
return db.scalar(q) or 0
|
||||
|
||||
|
||||
@router.get("/link")
|
||||
async def link_google(
|
||||
request: Request,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
"""Admin-only onboarding: review access requests (Invites), approve/deny, manual add.
|
||||
Also the demo-account admin surface: the demo email whitelist + a manual state reset."""
|
||||
import re
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException
|
||||
|
|
@ -8,7 +7,13 @@ from sqlalchemy import delete, func, select
|
|||
from sqlalchemy.orm import Session
|
||||
|
||||
from app import email as email_mod
|
||||
from app.auth import current_user, get_or_create_demo_user, operator_contact, purge_user
|
||||
from app.auth import (
|
||||
admin_user,
|
||||
count_admins,
|
||||
get_or_create_demo_user,
|
||||
operator_contact,
|
||||
purge_user,
|
||||
)
|
||||
from app.db import get_db
|
||||
from app.models import (
|
||||
DemoWhitelist,
|
||||
|
|
@ -20,17 +25,10 @@ from app.models import (
|
|||
Video,
|
||||
VideoState,
|
||||
)
|
||||
from app.utils import valid_email
|
||||
|
||||
router = APIRouter(prefix="/api/admin", tags=["admin"])
|
||||
|
||||
_EMAIL_RE = re.compile(r"^[^@\s]+@[^@\s]+\.[^@\s]+$")
|
||||
|
||||
|
||||
def admin_user(user: User = Depends(current_user)) -> User:
|
||||
if user.role != "admin":
|
||||
raise HTTPException(status_code=403, detail="Admin only")
|
||||
return user
|
||||
|
||||
|
||||
def _serialize(inv: Invite) -> dict:
|
||||
return {
|
||||
|
|
@ -110,7 +108,7 @@ def add_invite(
|
|||
) -> dict:
|
||||
"""Manually whitelist an email (approved straight away). Convenience for the admin."""
|
||||
email = (payload.get("email") or "").strip().lower()
|
||||
if "@" not in email:
|
||||
if not valid_email(email):
|
||||
raise HTTPException(status_code=400, detail="Enter a valid email address.")
|
||||
inv = db.execute(select(Invite).where(Invite.email == email)).scalar_one_or_none()
|
||||
if inv is None:
|
||||
|
|
@ -168,9 +166,7 @@ def set_user_role(
|
|||
raise HTTPException(status_code=400, detail="The demo account's role can't be changed.")
|
||||
if target.id == admin.id:
|
||||
raise HTTPException(status_code=400, detail="You can't change your own role.")
|
||||
if target.role == "admin" and role == "user":
|
||||
admin_count = db.scalar(select(func.count()).select_from(User).where(User.role == "admin"))
|
||||
if (admin_count or 0) <= 1:
|
||||
if target.role == "admin" and role == "user" and count_admins(db) <= 1:
|
||||
raise HTTPException(status_code=400, detail="Can't remove the last admin.")
|
||||
changed = target.role != role
|
||||
target.role = role
|
||||
|
|
@ -201,13 +197,12 @@ def set_user_suspended(
|
|||
raise HTTPException(status_code=400, detail="The demo account can't be suspended.")
|
||||
if target.id == admin.id:
|
||||
raise HTTPException(status_code=400, detail="You can't suspend your own account.")
|
||||
if suspended and target.role == "admin" and not target.is_suspended:
|
||||
active_admins = db.scalar(
|
||||
select(func.count())
|
||||
.select_from(User)
|
||||
.where(User.role == "admin", User.is_suspended.is_(False))
|
||||
)
|
||||
if (active_admins or 0) <= 1:
|
||||
if (
|
||||
suspended
|
||||
and target.role == "admin"
|
||||
and not target.is_suspended
|
||||
and count_admins(db, active_only=True) <= 1
|
||||
):
|
||||
raise HTTPException(status_code=400, detail="Can't suspend the last active admin.")
|
||||
was_suspended = target.is_suspended
|
||||
target.is_suspended = suspended
|
||||
|
|
@ -238,11 +233,7 @@ def admin_delete_user(
|
|||
raise HTTPException(
|
||||
status_code=400, detail="Delete your own account from Settings → Account."
|
||||
)
|
||||
if target.role == "admin":
|
||||
admin_count = db.scalar(
|
||||
select(func.count()).select_from(User).where(User.role == "admin")
|
||||
)
|
||||
if (admin_count or 0) <= 1:
|
||||
if target.role == "admin" and count_admins(db) <= 1:
|
||||
raise HTTPException(status_code=400, detail="Can't delete the last admin.")
|
||||
purge_user(db, target, background)
|
||||
return {"deleted": user_id}
|
||||
|
|
@ -281,7 +272,7 @@ def add_demo_whitelist(
|
|||
"""Add an email that may enter the shared demo account from the login page (no Google
|
||||
sign-in). Idempotent — re-adding an existing email just returns it."""
|
||||
email = (payload.get("email") or "").strip().lower()
|
||||
if not _EMAIL_RE.match(email):
|
||||
if not valid_email(email):
|
||||
raise HTTPException(status_code=400, detail="Enter a valid email address.")
|
||||
row = db.execute(
|
||||
select(DemoWhitelist).where(DemoWhitelist.email == email)
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ from sqlalchemy import func, select
|
|||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.auth import (
|
||||
count_admins,
|
||||
current_user,
|
||||
google_enabled,
|
||||
has_read_scope,
|
||||
|
|
@ -111,9 +112,7 @@ def delete_my_account(
|
|||
(that would lock everyone out of the admin surfaces)."""
|
||||
if user.is_demo:
|
||||
raise HTTPException(status_code=403, detail="The shared demo account can't be deleted.")
|
||||
if user.role == "admin":
|
||||
admins = db.scalar(select(func.count()).select_from(User).where(User.role == "admin")) or 0
|
||||
if admins <= 1:
|
||||
if user.role == "admin" and count_admins(db) <= 1:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="You're the only admin — promote another admin before deleting your account.",
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ from sqlalchemy import func, or_, select
|
|||
from sqlalchemy.exc import IntegrityError
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.auth import current_user
|
||||
from app.auth import admin_user, current_user
|
||||
from app.db import get_db
|
||||
from app.models import ChannelTag, Tag, User
|
||||
from app.sync.autotag import run_autotag_all
|
||||
|
|
@ -103,10 +103,8 @@ def delete_tag(
|
|||
|
||||
@router.post("/recompute")
|
||||
def recompute(
|
||||
user: User = Depends(current_user),
|
||||
_: User = Depends(admin_user),
|
||||
db: Session = Depends(get_db),
|
||||
only_missing: bool = False,
|
||||
) -> dict:
|
||||
if user.role != "admin":
|
||||
raise HTTPException(status_code=403, detail="Admin only")
|
||||
return run_autotag_all(db, only_missing=only_missing)
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
import hashlib
|
||||
|
||||
from argon2 import PasswordHasher
|
||||
from cryptography.fernet import Fernet
|
||||
|
||||
|
|
@ -7,6 +9,12 @@ from app.config import settings
|
|||
_ph = PasswordHasher()
|
||||
|
||||
|
||||
def hash_token(raw: str) -> str:
|
||||
"""SHA-256 hex digest for storing single-use tokens (email-link tokens, the setup token)
|
||||
by hash rather than in cleartext. Not a password hash — these are high-entropy randoms."""
|
||||
return hashlib.sha256(raw.encode()).hexdigest()
|
||||
|
||||
|
||||
def hash_password(password: str) -> str:
|
||||
return _ph.hash(password)
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,4 @@
|
|||
"""Global admin-controlled app state (e.g. pausing background sync, first-run setup)."""
|
||||
import hashlib
|
||||
import logging
|
||||
import secrets
|
||||
|
||||
|
|
@ -7,6 +6,7 @@ from sqlalchemy.orm import Session
|
|||
|
||||
from app.config import settings
|
||||
from app.models import AppState
|
||||
from app.security import hash_token
|
||||
|
||||
log = logging.getLogger("siftlode.state")
|
||||
|
||||
|
|
@ -73,16 +73,12 @@ def mark_configured(db: Session) -> None:
|
|||
log.info("Instance marked configured; setup wizard disabled.")
|
||||
|
||||
|
||||
def _hash_token(raw: str) -> str:
|
||||
return hashlib.sha256(raw.encode()).hexdigest()
|
||||
|
||||
|
||||
def rotate_setup_token(db: Session) -> str:
|
||||
"""Generate a fresh one-time setup token, persist only its hash, and return the raw token
|
||||
(logged at startup so the admin can open the wizard)."""
|
||||
raw = secrets.token_urlsafe(24)
|
||||
row = _row(db)
|
||||
row.setup_token_hash = _hash_token(raw)
|
||||
row.setup_token_hash = hash_token(raw)
|
||||
db.add(row)
|
||||
db.commit()
|
||||
return raw
|
||||
|
|
@ -93,7 +89,7 @@ def check_setup_token(db: Session, raw: str | None) -> bool:
|
|||
if not raw:
|
||||
return False
|
||||
stored = _row(db).setup_token_hash
|
||||
return stored is not None and secrets.compare_digest(stored, _hash_token(raw))
|
||||
return stored is not None and secrets.compare_digest(stored, hash_token(raw))
|
||||
|
||||
|
||||
def get_maintenance_batch(db: Session) -> int:
|
||||
|
|
|
|||
17
backend/app/utils.py
Normal file
17
backend/app/utils.py
Normal file
|
|
@ -0,0 +1,17 @@
|
|||
"""Small cross-cutting helpers shared across modules (kept dependency-light on purpose)."""
|
||||
import re
|
||||
from datetime import datetime, timezone
|
||||
|
||||
# Single source of truth for "looks like an email" — used by every endpoint that accepts an
|
||||
# address (auth register/reset/demo, admin invites + demo whitelist, the setup wizard). Keep
|
||||
# the strictness in one place so routes don't drift between this and a looser "@"-in check.
|
||||
_EMAIL_RE = re.compile(r"^[^@\s]+@[^@\s]+\.[^@\s]+$")
|
||||
|
||||
|
||||
def valid_email(value: str | None) -> bool:
|
||||
return bool(value and _EMAIL_RE.match(value))
|
||||
|
||||
|
||||
def now_utc() -> datetime:
|
||||
"""Current time as a timezone-aware UTC datetime."""
|
||||
return datetime.now(timezone.utc)
|
||||
Loading…
Add table
Add a link
Reference in a new issue