From 9fc489258b7adab651b84c2525863e5d32b8ae45 Mon Sep 17 00:00:00 2001 From: anti Date: Sat, 30 May 2026 18:27:53 -0400 Subject: [PATCH] fix(auth): bulk-revoke sessions on password and role change A stolen JWT used to survive a password reset for its full 24h. Now every session-invalidating change moves the user's tokens_valid_from cutoff to 'now', so all of that user's prior tokens 401 on next use: - self change-password, admin reset-password, role change all bump the cutoff (delete needs no bump: the row is gone, so the user lookup 401s). - Cutoff is compared against the token's iat floored to whole seconds, so a re-login in the same second as the change isn't caught by its own revocation (the cost is a <=1s grey zone on same-second-old tokens). - Per-user: changing one user never revokes another. --- decnet/web/dependencies.py | 7 +- decnet/web/router/auth/api_change_pass.py | 4 + decnet/web/router/config/api_manage_users.py | 9 ++ tests/api/auth/test_bulk_revocation.py | 138 +++++++++++++++++++ 4 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 tests/api/auth/test_bulk_revocation.py diff --git a/decnet/web/dependencies.py b/decnet/web/dependencies.py index d11fdb01..28b3e0b5 100644 --- a/decnet/web/dependencies.py +++ b/decnet/web/dependencies.py @@ -195,8 +195,13 @@ async def _resolve_token(token: str) -> tuple[str, dict[str, Any]]: if not jti: raise _CREDENTIALS_EXCEPTION # 2. Bulk cutoff: password/role change moves tokens_valid_from forward. + # JWT iat is whole-seconds, so floor the cutoff to whole seconds too — + # otherwise a re-login landing in the SAME second as the change gets an + # iat that truncates below a sub-second cutoff and is wrongly rejected. + # Cost: tokens issued earlier in that same second survive (≤1s), which is + # negligible against a 24h lifetime. cutoff = user.get("tokens_valid_from") - if cutoff is not None and _epoch(payload.get("iat", 0)) < _epoch(cutoff): + if cutoff is not None and _epoch(payload.get("iat", 0)) < int(_epoch(cutoff)): raise _CREDENTIALS_EXCEPTION # 3. Single-token denylist (logout). if await _is_revoked_cached(jti): diff --git a/decnet/web/router/auth/api_change_pass.py b/decnet/web/router/auth/api_change_pass.py index e4c2c606..47c84d37 100644 --- a/decnet/web/router/auth/api_change_pass.py +++ b/decnet/web/router/auth/api_change_pass.py @@ -1,4 +1,5 @@ # SPDX-License-Identifier: AGPL-3.0-or-later +from datetime import datetime, timezone from typing import Any, Optional from fastapi import APIRouter, Depends, HTTPException, status @@ -32,5 +33,8 @@ async def change_password(request: ChangePasswordRequest, current_user: str = De _new_hash: str = await ahash_password(request.new_password) await repo.update_user_password(current_user, _new_hash, must_change_password=False) + # Changing a password revokes every existing session for this user (incl. + # the current one): the caller's next request 401s and re-authenticates. + await repo.set_tokens_valid_from(current_user, datetime.now(timezone.utc)) invalidate_user_cache(current_user) return {"message": "Password updated successfully"} diff --git a/decnet/web/router/config/api_manage_users.py b/decnet/web/router/config/api_manage_users.py index e0e350b2..2a100df7 100644 --- a/decnet/web/router/config/api_manage_users.py +++ b/decnet/web/router/config/api_manage_users.py @@ -1,5 +1,6 @@ # SPDX-License-Identifier: AGPL-3.0-or-later import uuid as _uuid +from datetime import datetime, timezone from fastapi import APIRouter, Depends, HTTPException @@ -77,6 +78,8 @@ async def api_delete_user( deleted = await repo.delete_user(user_uuid) if not deleted: raise HTTPException(status_code=404, detail="User not found") + # No token bump needed: the user row is gone, so _resolve_token already + # 401s any of their tokens (user lookup returns None). invalidate_user_cache(user_uuid) invalidate_list_users_cache() return {"message": "User deleted"} @@ -108,6 +111,9 @@ async def api_update_user_role( raise HTTPException(status_code=404, detail="User not found") await repo.update_user_role(user_uuid, req.role) + # Revoke the target's sessions so a privilege change can't be outrun by an + # in-flight token carrying the old role. + await repo.set_tokens_valid_from(user_uuid, datetime.now(timezone.utc)) invalidate_user_cache(user_uuid) invalidate_list_users_cache() return {"message": "User role updated"} @@ -140,6 +146,9 @@ async def api_reset_user_password( await ahash_password(req.new_password), must_change_password=True, ) + # Admin reset implies the old credential is burned — revoke the target's + # existing sessions so a leaked token can't survive the reset. + await repo.set_tokens_valid_from(user_uuid, datetime.now(timezone.utc)) invalidate_user_cache(user_uuid) invalidate_list_users_cache() return {"message": "Password reset successfully"} diff --git a/tests/api/auth/test_bulk_revocation.py b/tests/api/auth/test_bulk_revocation.py new file mode 100644 index 00000000..240bdc57 --- /dev/null +++ b/tests/api/auth/test_bulk_revocation.py @@ -0,0 +1,138 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +"""Bulk session revocation (WI3): password/role changes move tokens_valid_from +forward, killing every prior token for that user. + +End-to-end revocation is proven with deterministically *aged* tokens (iat well +in the past) so the assertions don't race the floored-cutoff grey zone — a +token minted in the very same wall-clock second as the change intentionally +survives (see dependencies._resolve_token). The same-second re-login path is +covered separately. +""" +from __future__ import annotations + +import time +import uuid as _uuid + +import jwt +import pytest +from sqlalchemy import select + +from decnet.env import DECNET_ADMIN_USER, DECNET_ADMIN_PASSWORD +from decnet.web.auth import ALGORITHM, SECRET_KEY, get_password_hash +from decnet.web.db.models import User +from decnet.web.dependencies import repo + +PROTECTED = "/api/v1/attackers?limit=1" + + +async def _seed_user(username: str, password: str, role: str = "viewer") -> str: + async with repo.session_factory() as session: + existing = (await session.execute( + select(User).where(User.username == username) + )).scalar_one_or_none() + if existing: + return existing.uuid + u = str(_uuid.uuid4()) + session.add(User( + uuid=u, username=username, password_hash=get_password_hash(password), + role=role, must_change_password=False, + )) + await session.commit() + return u + + +def _auth(token: str) -> dict[str, str]: + return {"Authorization": f"Bearer {token}"} + + +def _uuid_of(token: str) -> str: + return jwt.decode(token, options={"verify_signature": False})["uuid"] + + +def _aged_token(uuid: str, *, seconds_old: int = 30) -> str: + """A well-formed token issued ``seconds_old`` ago — older than any cutoff a + change sets to 'now', so it is deterministically revoked once bumped.""" + now = int(time.time()) + return jwt.encode( + {"uuid": uuid, "jti": f"aged-{uuid}", "iat": now - seconds_old, "exp": now + 3600}, + SECRET_KEY, algorithm=ALGORITHM, + ) + + +async def _login(client, username: str, password: str) -> str: + r = await client.post( + "/api/v1/auth/login", json={"username": username, "password": password}, + ) + return r.json()["access_token"] + + +@pytest.mark.asyncio +async def test_self_password_change_revokes_prior_tokens(client): + # Dedicated user — the admin fixture already bumped admin's cutoff. The aged + # token is the "old session"; a fresh login drives the change. + uuid = await _seed_user("selfchange-user", "selfchange-pass-1") + aged = _aged_token(uuid) + assert (await client.get(PROTECTED, headers=_auth(aged))).status_code == 200 + current = await _login(client, "selfchange-user", "selfchange-pass-1") + r = await client.post( + "/api/v1/auth/change-password", + json={"old_password": "selfchange-pass-1", "new_password": "selfchange-pass-2"}, + headers=_auth(current), + ) + assert r.status_code == 200, r.text + # Every token issued before the change is dead. + assert (await client.get(PROTECTED, headers=_auth(aged))).status_code == 401 + + +@pytest.mark.asyncio +async def test_relogin_after_password_change_works_immediately(client, auth_token): + # Guards the same-second iat/cutoff race: a re-login right after the change + # must succeed (floored cutoff), not get caught by its own revocation. + await client.post( + "/api/v1/auth/change-password", + json={"old_password": DECNET_ADMIN_PASSWORD, "new_password": "fresh-pass-77"}, + headers=_auth(auth_token), + ) + fresh = await _login(client, DECNET_ADMIN_USER, "fresh-pass-77") + assert (await client.get(PROTECTED, headers=_auth(fresh))).status_code == 200 + + +@pytest.mark.asyncio +async def test_admin_password_reset_revokes_target_sessions(client, auth_token, viewer_token): + viewer_uuid = _uuid_of(viewer_token) + aged = _aged_token(viewer_uuid) + assert (await client.get(PROTECTED, headers=_auth(aged))).status_code == 200 + r = await client.put( + f"/api/v1/config/users/{viewer_uuid}/reset-password", + json={"new_password": "reset-by-admin-1"}, + headers=_auth(auth_token), + ) + assert r.status_code == 200, r.text + assert (await client.get(PROTECTED, headers=_auth(aged))).status_code == 401 + + +@pytest.mark.asyncio +async def test_role_change_revokes_target_sessions(client, auth_token, viewer_token): + viewer_uuid = _uuid_of(viewer_token) + aged = _aged_token(viewer_uuid) + assert (await client.get(PROTECTED, headers=_auth(aged))).status_code == 200 + r = await client.put( + f"/api/v1/config/users/{viewer_uuid}/role", + json={"role": "admin"}, + headers=_auth(auth_token), + ) + assert r.status_code == 200, r.text + assert (await client.get(PROTECTED, headers=_auth(aged))).status_code == 401 + + +@pytest.mark.asyncio +async def test_revocation_is_per_user(client, auth_token, viewer_token): + # Resetting the viewer must not revoke the admin's own (valid) token. + assert (await client.get(PROTECTED, headers=_auth(auth_token))).status_code == 200 + viewer_uuid = _uuid_of(viewer_token) + await client.put( + f"/api/v1/config/users/{viewer_uuid}/reset-password", + json={"new_password": "reset-by-admin-2"}, + headers=_auth(auth_token), + ) + assert (await client.get(PROTECTED, headers=_auth(auth_token))).status_code == 200