Security hardening (maziggy/bambuddy-security #14)

backend/app/api/routes/auth.py

@@ -38,6 +38,7 @@ from backend.app.core.auth import (
     resolve_session_max_minutes,
     revoke_jti,
     security,
+    token_outlived_its_session,
 )
 from backend.app.core.database import async_session, get_db
 from backend.app.core.oidc_env import env_bool
@@ -1238,14 +1239,16 @@ async def forgot_password_confirm(request: ForgotPasswordConfirmRequest, db: Asy
             AuthEphemeralToken.token == request.token,
             AuthEphemeralToken.token_type == TokenType.PASSWORD_RESET,
         )
-        .returning(AuthEphemeralToken.username, AuthEphemeralToken.expires_at)
+        .returning(AuthEphemeralToken.username, AuthEphemeralToken.expires_at, AuthEphemeralToken.created_at)
     )
     row = result.one_or_none()
     await db.commit()
     if row is None:
         raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid or expired password reset token")
 
-    username, expires_at = row
+    username, expires_at, minted_at = row
+    if await token_outlived_its_session(db, username, minted_at):
+        raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid or expired password reset token")
     # Stored as naive UTC on both databases; treat it as UTC.
     if expires_at.tzinfo is None:
         expires_at = expires_at.replace(tzinfo=timezone.utc)

backend/app/api/routes/connected_apps.py

@@ -44,6 +44,7 @@ from backend.app.core.auth import (
     get_user_by_username,
     is_auth_enabled,
     security,
+    token_outlived_its_session,
     verify_password,
 )
 from backend.app.core.database import get_db
@@ -370,13 +371,15 @@ async def exchange_code(
             AuthEphemeralToken.provider_id == app.id,
             AuthEphemeralToken.expires_at > now,
         )
-        .returning(AuthEphemeralToken.username, AuthEphemeralToken.nonce)
+        .returning(AuthEphemeralToken.username, AuthEphemeralToken.nonce, AuthEphemeralToken.created_at)
     )
     row = consumed.one_or_none()
     await db.commit()
     if row is None:
         raise await fail(400, "invalid_grant", "unknown, expired, reused or foreign code")
-    username, code_challenge = row
+    username, code_challenge, minted_at = row
+    if await token_outlived_its_session(db, username, minted_at):
+        raise await fail(400, "invalid_grant", "code from before the user's current session")
 
     if not hmac.compare_digest(app.redirect_uri, data.redirect_uri):
         raise await fail(400, "invalid_grant", "callback URL mismatch")

backend/app/api/routes/mfa.py

@@ -48,6 +48,7 @@ from backend.app.core.auth import (
     get_user_by_username,
     is_auth_enabled,
     resolve_session_max_minutes,
+    token_outlived_its_session,
     verify_password,
 )
 from backend.app.core.database import get_db
@@ -283,12 +284,15 @@ async def consume_pre_auth_token(db: AsyncSession, token: str, challenge_id: str
             AuthEphemeralToken.token_type == TokenType.PRE_AUTH,
             AuthEphemeralToken.expires_at > now,
         )
-        .returning(AuthEphemeralToken.username, AuthEphemeralToken.challenge_id)
+        .returning(AuthEphemeralToken.username, AuthEphemeralToken.challenge_id, AuthEphemeralToken.created_at)
     )
     row = result.one_or_none()
     if row is None:
         return None
-    username, stored_challenge_id = row
+    username, stored_challenge_id, minted_at = row
+    if await token_outlived_its_session(db, username, minted_at):
+        await db.commit()  # spent all the same
+        return None
     # Enforce client binding: if the token was issued with a challenge_id,
     # the caller must supply the matching value.
     if stored_challenge_id is not None and stored_challenge_id != challenge_id:
@@ -314,7 +318,7 @@ async def peek_pre_auth_token(db: AsyncSession, token: str, challenge_id: str |
         )
     )
     eph = result.scalar_one_or_none()
-    if eph is None:
+    if eph is None or await token_outlived_its_session(db, eph.username, eph.created_at):
         return None
     # Enforce client binding: if the token was issued with a challenge_id the
     # cookie must match.  Treat a mismatch as if the token doesn't exist.
@@ -982,7 +986,7 @@ async def confirm_enable_email_otp(
         )
     )
     eph = peek_result.scalar_one_or_none()
-    if eph is None:
+    if eph is None or await token_outlived_its_session(db, eph.username, eph.created_at):
         raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid or expired setup token")
 
     code_hash = eph.nonce  # code hash stored in the nonce field
@@ -2204,14 +2208,16 @@ async def oidc_exchange(
             AuthEphemeralToken.token_type == TokenType.OIDC_EXCHANGE,
             AuthEphemeralToken.expires_at > now,  # reject expired tokens atomically
         )
-        .returning(AuthEphemeralToken.username)
+        .returning(AuthEphemeralToken.username, AuthEphemeralToken.created_at)
     )
     row = consume_result.one_or_none()
     if row is None:
         raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid or expired OIDC exchange token")
 
-    (username,) = row
+    username, minted_at = row
     await db.commit()
+    if await token_outlived_its_session(db, username, minted_at):
+        raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid or expired OIDC exchange token")
 
     user = await get_user_by_username(db, username)
     if not user or not user.is_active:

backend/app/core/auth.py

@@ -841,6 +841,26 @@ async def verify_slicer_download_token(
         return True
 
 
+async def token_outlived_its_session(db: AsyncSession, username: str | None, minted_at: datetime | None) -> bool:
+    """Whether a token named after a user was minted before that user's session began.
+
+    password_changed_at marks it: set for a new user, and on a rename, a
+    password change or reset, a 2FA removal or a restore. A token from before
+    belongs to an earlier holder of the name or an ended session; deletes and
+    renames drop those, but one minted while they commit would survive.
+    Truncated to the second as for JWTs (_is_token_fresh): a new user and the
+    first token it is given can be written in the same flush.
+    """
+    if not username or minted_at is None:
+        return False
+    changed = (
+        await db.execute(select(User.password_changed_at).where(func.lower(User.username) == func.lower(username)))
+    ).scalar_one_or_none()
+    if changed is None:
+        return False
+    return minted_at.replace(tzinfo=None) < changed.replace(tzinfo=None, microsecond=0)
+
+
 # --- Camera stream tokens ---
 # Reusable tokens for camera stream/snapshot endpoints loaded via <img>/<video>
 # tags (these cannot send Authorization headers).  Unlike slicer tokens they are
@@ -947,7 +967,7 @@ async def verify_websocket_token_principal(token: str) -> tuple[str, int | None]
             )
         )
         row = result.scalar_one_or_none()
-        if row is None:
+        if row is None or await token_outlived_its_session(db, row.username, row.created_at):
             return None
         return row.username or "", row.api_key_id
 
@@ -1091,6 +1111,8 @@ async def camera_stream_access(token: str) -> tuple[PrinterScope, StreamOpener]
             )
         )
         row = result.scalar_one_or_none()
+        if row is not None and await token_outlived_its_session(db, row.username, row.created_at):
+            return None
         if row is not None:
             # Minting it needed camera:view; a group or key change can take it away.
             if not await principal_holds(db, row.username, row.api_key_id, Permission.CAMERA_VIEW):
@@ -1310,7 +1332,7 @@ async def verify_media_token(token: str) -> str | None:
             )
         )
         row = result.scalar_one_or_none()
-        if row is None:
+        if row is None or await token_outlived_its_session(db, row.username, row.created_at):
             return None
         return row.username or ""
 

backend/tests/integration/test_token_outlived_session.py

@@ -0,0 +1,176 @@
+"""A one-time token minted before its user's session began is refused.
+
+Deleting or renaming a user drops the tokens pending under the name in its
+transaction; one minted in the milliseconds around it (a WebSocket token from
+the old user's still-valid JWT, a forgot-password link) kept the name and
+worked for whoever was given it next. Every token named after a user is now
+refused if it was minted before that user's password_changed_at, which a new
+user, a rename, a password change or reset, a 2FA removal and a restore set.
+"""
+
+from datetime import datetime, timedelta, timezone
+
+import pytest
+from httpx import AsyncClient
+from sqlalchemy import text
+
+from backend.app.core import auth
+from backend.app.models.auth_ephemeral import AuthEphemeralToken, TokenType
+
+pytestmark = [pytest.mark.integration, pytest.mark.asyncio]
+
+
+@pytest.fixture
+async def carol(async_client: AsyncClient, db_session) -> int:
+    await async_client.post(
+        "/api/v1/auth/setup",
+        json={"auth_enabled": True, "admin_username": "tokadmin", "admin_password": "AdminPass1!"},
+    )
+    login = await async_client.post("/api/v1/auth/login", json={"username": "tokadmin", "password": "AdminPass1!"})
+    admin = {"Authorization": f"Bearer {login.json()['access_token']}"}
+    groups = (await async_client.get("/api/v1/groups/", headers=admin)).json()
+    viewers = next(g["id"] for g in groups if g["name"] == "Viewers")
+    user = await async_client.post(
+        "/api/v1/users/",
+        headers=admin,
+        json={"username": "carol", "password": "Userpass1!", "role": "user", "group_ids": [viewers]},
+    )
+    carol_id = user.json()["id"]
+    # Carol's current session began 30 minutes ago (a rename, a new account...).
+    await db_session.execute(
+        text("UPDATE users SET password_changed_at = :t WHERE id = :id"),
+        {"t": _ago(minutes=30), "id": carol_id},
+    )
+    await db_session.commit()
+    return carol_id
+
+
+def _ago(**delta) -> datetime:
+    return datetime.now(timezone.utc).replace(tzinfo=None) - timedelta(**delta)
+
+
+async def _token(db, kind: str, *, minted_ago_minutes: int, **fields) -> str:
+    value = f"{kind}-{minted_ago_minutes}"
+    db.add(
+        AuthEphemeralToken(
+            token=value,
+            token_type=kind,
+            username=fields.pop("username", "carol"),
+            created_at=_ago(minutes=minted_ago_minutes),
+            expires_at=datetime.now(timezone.utc) + timedelta(hours=1),
+            **fields,
+        )
+    )
+    await db.commit()
+    return value
+
+
+@pytest.mark.parametrize(
+    ("kind", "verify"),
+    [
+        ("websocket", lambda t: auth.verify_websocket_token_principal(t)),
+        ("camera_stream", lambda t: auth.camera_stream_access(t)),
+        ("media", lambda t: auth.verify_media_token(t)),
+    ],
+)
+async def test_a_reusable_token_from_before_the_session_is_refused(carol, db_session, kind, verify):
+    stale = await _token(db_session, kind, minted_ago_minutes=60)
+    fresh = await _token(db_session, kind, minted_ago_minutes=10)
+
+    assert await verify(stale) is None
+    assert await verify(fresh) is not None
+
+
+async def test_a_pre_auth_token_from_before_the_session_is_refused(carol, db_session):
+    from backend.app.api.routes.mfa import consume_pre_auth_token, peek_pre_auth_token
+
+    stale = await _token(db_session, TokenType.PRE_AUTH, minted_ago_minutes=60)
+    fresh = await _token(db_session, TokenType.PRE_AUTH, minted_ago_minutes=10)
+
+    assert await peek_pre_auth_token(db_session, stale) is None
+    assert await consume_pre_auth_token(db_session, stale) is None
+    assert await consume_pre_auth_token(db_session, fresh) == "carol"
+
+
+async def test_a_reset_link_from_before_the_session_is_refused(async_client: AsyncClient, carol, db_session):
+    stale = await _token(db_session, TokenType.PASSWORD_RESET, minted_ago_minutes=60)
+
+    response = await async_client.post(
+        "/api/v1/auth/forgot-password/confirm", json={"token": stale, "new_password": "Newpass12!"}
+    )
+
+    assert response.status_code == 400, response.text
+
+
+async def test_an_oidc_exchange_token_from_before_the_session_is_refused(async_client: AsyncClient, carol, db_session):
+    stale = await _token(db_session, TokenType.OIDC_EXCHANGE, minted_ago_minutes=60)
+
+    response = await async_client.post("/api/v1/auth/oidc/exchange", json={"oidc_token": stale})
+
+    assert response.status_code == 401, response.text
+
+
+async def test_an_email_otp_setup_token_from_before_the_session_is_refused(
+    async_client: AsyncClient, carol, db_session
+):
+    from backend.app.api.routes.mfa import pwd_context
+
+    stale = await _token(db_session, TokenType.EMAIL_OTP_SETUP, minted_ago_minutes=60, nonce=pwd_context.hash("123456"))
+    login = await async_client.post("/api/v1/auth/login", json={"username": "carol", "password": "Userpass1!"})
+
+    response = await async_client.post(
+        "/api/v1/auth/2fa/email/enable/confirm",
+        headers={"Authorization": f"Bearer {login.json()['access_token']}"},
+        json={"setup_token": stale, "code": "123456"},
+    )
+
+    assert response.status_code == 400, response.text
+
+
+async def test_a_connect_code_from_before_the_session_is_refused(async_client: AsyncClient, carol, db_session):
+    from backend.tests.integration.test_connected_apps import _exchange, _pkce, _register
+
+    login = await async_client.post("/api/v1/auth/login", json={"username": "tokadmin", "password": "AdminPass1!"})
+    app = await _register(async_client, login.json()["access_token"])
+    verifier, challenge = _pkce()
+    code = "connect-code-stale"
+    from backend.app.api.routes.connected_apps import _hash_code
+
+    db_session.add(
+        AuthEphemeralToken.new_connect_code(
+            code_hash=_hash_code(code),
+            username="carol",
+            app_id=app["id"],
+            code_challenge=challenge,
+            expires_at=datetime.now(timezone.utc) + timedelta(minutes=1),
+        )
+    )
+    await db_session.commit()
+    await db_session.execute(
+        text("UPDATE auth_ephemeral_tokens SET created_at = :t WHERE token = :h"),
+        {"t": _ago(minutes=60), "h": _hash_code(code)},
+    )
+    await db_session.commit()
+
+    response = await _exchange(async_client, app, code, verifier)
+
+    assert response.status_code == 400, response.text
+
+
+async def test_every_token_kind_named_after_a_user_is_covered():
+    """A new kind must get the check (and a test above)."""
+    covered = {
+        "websocket",
+        "camera_stream",
+        "media",
+        TokenType.PRE_AUTH.value,
+        TokenType.PASSWORD_RESET.value,
+        TokenType.OIDC_EXCHANGE.value,
+        TokenType.EMAIL_OTP_SETUP.value,
+        TokenType.CONNECT_CODE.value,
+    }
+    named_by_nobody = {
+        TokenType.OIDC_STATE.value,  # before anyone is signed in
+        TokenType.SLICER_DOWNLOAD.value,  # bound to a resource, not a user
+    }
+    assert {kind.value for kind in TokenType} - named_by_nobody <= covered