From 746392b5013db72cdad22ca632d4b83cb7a86cd4 Mon Sep 17 00:00:00 2001 From: HoungDev <311347655+HoungDev@users.noreply.github.com> Date: Tue, 11 Aug 2026 20:42:53 +0700 Subject: [PATCH 1/2] fix: enforce active account lifecycle --- API_EXAMPLES.md | 12 +++++ ARCHITECTURE.md | 38 ++++++++------ CHANGELOG.md | 2 + README.md | 1 + src/app/api/v1/admin.py | 44 +++++++++++++++- src/app/auth/current_user.py | 6 +++ src/app/auth/login.py | 2 +- src/app/auth/refresh.py | 15 ++++++ src/app/schemas/user.py | 5 ++ src/app/services/session_issuance.py | 18 ++++++- src/app/services/sessions.py | 13 +++-- tests/test_admin.py | 77 ++++++++++++++++++++++++++++ tests/test_documentation.py | 1 + tests/test_refresh_failures.py | 20 +++++++- 14 files changed, 230 insertions(+), 24 deletions(-) diff --git a/API_EXAMPLES.md b/API_EXAMPLES.md index c8ce736..fc874e7 100644 --- a/API_EXAMPLES.md +++ b/API_EXAMPLES.md @@ -290,6 +290,18 @@ curl --request PATCH http://127.0.0.1:8000/admin/users/1/role \ --data '{"role":"admin"}' ``` +To disable an account and atomically revoke its refresh-token sessions: + +```bash +curl --request PATCH http://127.0.0.1:8000/admin/users/1/status \ + --header "Authorization: Bearer ${ADMIN_ACCESS_TOKEN}" \ + --header "Content-Type: application/json" \ + --data '{"is_active":false}' +``` + +Set `is_active` to `true` to allow new authentication again. Re-enabling an +account does not restore revoked sessions; the user must sign in again. + Non-admin users receive `403`; an unknown user ID returns `404`. ## Inspect health and metrics diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 8f20fb8..0b2afca 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -81,54 +81,60 @@ the response omits internal details. creates an opaque refresh token, and stores only the refresh-token hash. 3. Protected routes decode the JWT and validate algorithm, issuer, audience, expiration, token type, and subject. The current user is then loaded from the - database, so deleted users and role changes take effect without waiting for a - new access token. + database, so deleted users, disabled users, and role changes take effect + without waiting for a new access token. 4. `POST /auth/refresh` validates the stored token, rejects revoked or expired records, locks and revokes the old token, and commits a replacement in the same server-generated family. Replay of a rotated token revokes the family's live descendant. 5. `POST /auth/logout` revokes the submitted refresh-token family. -6. Registration may attach a normalized email identity. Verification requests +6. Disabling a user through the admin status endpoint atomically marks the + account inactive and revokes every refresh-token session. Existing access + tokens fail the current-user lookup, and re-enabling the account does not + restore revoked sessions. +7. Registration may attach a normalized email identity. Verification requests invalidate older outstanding tokens, persist only a SHA-256 token hash, and deliver the raw token through the configured SMTP boundary. -7. Confirmation locks and atomically consumes the scoped token while setting +8. Confirmation locks and atomically consumes the scoped token while setting `email_verified_at`. Expiry, replay, purpose, and current-email checks occur before the transaction commits. -8. Password-reset requests reuse the account-action token table with a distinct +9. Password-reset requests reuse the account-action token table with a distinct purpose and only accept active, verified email identities without revealing eligibility to the caller. -9. Reset confirmation locks both token and user, updates the password hash, +10. Reset confirmation locks both token and user, updates the password hash, consumes outstanding reset tokens, and revokes every refresh token in one transaction. It creates no replacement session. -10. Authenticated session endpoints aggregate active refresh-token families and +11. Authenticated session endpoints aggregate active refresh-token families and allow idempotent revocation of one or all families while filtering every operation by current user ownership. -11. TOTP enrollment encrypts the authenticator seed with a dedicated Fernet +12. TOTP enrollment encrypts the authenticator seed with a dedicated Fernet key. Confirmation stores only hashes of newly generated recovery codes. -12. Login for an MFA-enabled account returns a short-lived, opaque challenge +13. Login for an MFA-enabled account returns a short-lived, opaque challenge instead of session tokens. Successful TOTP or recovery verification consumes the challenge and creates the device session in one transaction. -13. Accepted TOTP counters are recorded to reject replay in the same time step. +14. Accepted TOTP counters are recorded to reject replay in the same time step. Access tokens record authentication methods (`amr`) and time (`auth_time`); refresh-issued access tokens use `amr=["refresh"]` and cannot satisfy recent MFA step-up checks. -14. OIDC authorization creates a short-lived database transaction containing +15. OIDC authorization creates a short-lived database transaction containing hashes of `state`, nonce, and browser binding plus an encrypted PKCE verifier. The authorization request always uses Authorization Code and PKCE S256. -15. The callback validates browser binding, discovery issuer, ID-token signature, +16. The callback validates browser binding, discovery issuer, ID-token signature, algorithm, issuer, audience, authorized party, lifetime, subject, and nonce before consuming the transaction and issuing a local device session. -16. External identities use immutable `(issuer, subject)` keys. Matching email +17. External identities use immutable `(issuer, subject)` keys. Matching email never links an existing account; linking requires a recent authenticated local session. Identity changes revoke refresh sessions. -17. Optional Redis cache-aside stores only validated public OIDC discovery and +18. Optional Redis cache-aside stores only validated public OIDC discovery and JWKS documents under versioned issuer-digest keys. Every cache read is validated again. Misses use a bounded refresh lock; Redis errors bypass to the provider. An unknown cached `kid` forces one provider JWKS refresh. Refresh-token families detect replay and make device-level revocation possible. -Access tokens are stateless and remain valid until expiration, so clients must -discard them on logout and deployments must protect the signing secret. +Access tokens are stateless and logout does not revoke them, so clients must +discard them on logout and deployments must protect the signing secret. A +protected request still reloads the user, allowing deletion or account disable +to reject an otherwise unexpired access token. ## Authorization model diff --git a/CHANGELOG.md b/CHANGELOG.md index aaf3b91..9cc22c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Added +- Administrative account disable/re-enable lifecycle with immediate access + rejection and atomic refresh-session revocation - Optional Redis-backed fixed-window rate limiting shared across API processes - Privacy-preserving HMAC client identifiers, atomic counters, and bounded TTLs - Explicit fail-closed/fail-open outage policies with readiness and metrics diff --git a/README.md b/README.md index 5376c3a..3848f1a 100644 --- a/README.md +++ b/README.md @@ -179,6 +179,7 @@ and safe extension points. | `GET` | `/auth/oidc/identities` | List linked external identity providers | | `GET` | `/auth/me` | Return the authenticated user | | `GET` | `/admin/users` | List users as an admin | +| `PATCH` | `/admin/users/{user_id}/status` | Disable or re-enable a user and revoke sessions on disable | The generated OpenAPI document at `/docs` is the source of truth for the full request and response schemas. diff --git a/src/app/api/v1/admin.py b/src/app/api/v1/admin.py index bad096b..f594920 100644 --- a/src/app/api/v1/admin.py +++ b/src/app/api/v1/admin.py @@ -5,7 +5,11 @@ from app.auth.permissions import require_admin from app.db.dependency import get_db from app.models.user import User -from app.schemas.user import UserAdminResponse, UserRoleUpdate +from app.schemas.user import UserAdminResponse, UserRoleUpdate, UserStatusUpdate +from app.services.sessions import ( + ACCOUNT_DISABLED_REVOCATION_REASON, + revoke_all_user_sessions, +) router = APIRouter( prefix="/admin", @@ -89,6 +93,44 @@ def update_user_role( return user +@router.patch( + "/users/{user_id}/status", + response_model=UserAdminResponse, +) +def update_user_status( + user_id: int, + data: UserStatusUpdate, + current_user: User = Depends(get_current_user), + db: Session = Depends(get_db), +): + require_admin(current_user) + + user = db.query(User).filter(User.id == user_id).first() + + if not user: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="User not found", + ) + + try: + user.is_active = data.is_active + if data.is_active: + db.commit() + else: + revoke_all_user_sessions( + user.id, + db, + reason=ACCOUNT_DISABLED_REVOCATION_REASON, + ) + db.refresh(user) + except Exception: + db.rollback() + raise + + return user + + @router.delete( "/users/{user_id}", ) diff --git a/src/app/auth/current_user.py b/src/app/auth/current_user.py index 6824719..ab33fda 100644 --- a/src/app/auth/current_user.py +++ b/src/app/auth/current_user.py @@ -27,4 +27,10 @@ def get_current_user( detail="User not found", ) + if not user.is_active: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Inactive user", + ) + return user diff --git a/src/app/auth/login.py b/src/app/auth/login.py index fa55f90..cdb9e12 100644 --- a/src/app/auth/login.py +++ b/src/app/auth/login.py @@ -32,7 +32,7 @@ def login( ): user = db.query(User).filter(User.username == form_data.username).first() - if not user: + if not user or not user.is_active: raise HTTPException( status_code=401, detail="Invalid username or password", diff --git a/src/app/auth/refresh.py b/src/app/auth/refresh.py index c61e543..b4a4492 100644 --- a/src/app/auth/refresh.py +++ b/src/app/auth/refresh.py @@ -11,6 +11,10 @@ ) from app.models.refresh_token import RefreshToken from app.models.user import User +from app.services.sessions import ( + ACCOUNT_DISABLED_REVOCATION_REASON, + revoke_all_user_sessions, +) ROTATION_REASON = "rotated" REPLAY_REASON = "reuse_detected" @@ -92,6 +96,17 @@ def refresh_access_token( detail="User not found", ) + if not user.is_active: + revoke_all_user_sessions( + user.id, + db, + reason=ACCOUNT_DISABLED_REVOCATION_REASON, + ) + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid refresh token", + ) + new_refresh_token, expires_at = create_refresh_token() new_db_refresh_token = RefreshToken( diff --git a/src/app/schemas/user.py b/src/app/schemas/user.py index 7374ce4..4909917 100644 --- a/src/app/schemas/user.py +++ b/src/app/schemas/user.py @@ -30,10 +30,15 @@ class UserRoleUpdate(BaseModel): role: str +class UserStatusUpdate(BaseModel): + is_active: bool + + class UserAdminResponse(BaseModel): id: int username: str role: str + is_active: bool model_config = ConfigDict( from_attributes=True, diff --git a/src/app/services/session_issuance.py b/src/app/services/session_issuance.py index e53dba7..35c1e95 100644 --- a/src/app/services/session_issuance.py +++ b/src/app/services/session_issuance.py @@ -1,5 +1,6 @@ from datetime import UTC, datetime +from fastapi import HTTPException, status from sqlalchemy.orm import Session from app.auth.jwt import create_access_token @@ -20,10 +21,23 @@ def prepare_session_tokens( *, authentication_methods: list[str], ) -> Token: + active_user = ( + db.query(User) + .filter(User.id == user.id) + .populate_existing() + .with_for_update() + .first() + ) + if active_user is None or not active_user.is_active: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Authentication is not available", + ) + authenticated_at = datetime.now(UTC) access_token = create_access_token( { - "sub": user.username, + "sub": active_user.username, "amr": authentication_methods, "auth_time": int(authenticated_at.timestamp()), } @@ -31,7 +45,7 @@ def prepare_session_tokens( refresh_token, expires_at = create_refresh_token() db.add( RefreshToken( - user_id=user.id, + user_id=active_user.id, family_id=create_refresh_token_family_id(), token=hash_refresh_token(refresh_token), expires_at=expires_at, diff --git a/src/app/services/sessions.py b/src/app/services/sessions.py index c6ce963..979a163 100644 --- a/src/app/services/sessions.py +++ b/src/app/services/sessions.py @@ -7,6 +7,7 @@ from app.schemas.session import DeviceSession USER_REVOCATION_REASON = "user_revoked" +ACCOUNT_DISABLED_REVOCATION_REASON = "account_disabled" def _as_utc(value: datetime) -> datetime: @@ -52,8 +53,13 @@ def revoke_session_family(user_id: int, family_id: str, db: Session) -> None: _revoke_user_sessions(user_id, db, family_id=family_id) -def revoke_all_user_sessions(user_id: int, db: Session) -> None: - _revoke_user_sessions(user_id, db) +def revoke_all_user_sessions( + user_id: int, + db: Session, + *, + reason: str = USER_REVOCATION_REASON, +) -> None: + _revoke_user_sessions(user_id, db, reason=reason) def _revoke_user_sessions( @@ -61,6 +67,7 @@ def _revoke_user_sessions( db: Session, *, family_id: str | None = None, + reason: str = USER_REVOCATION_REASON, ) -> None: now = datetime.now(UTC) statement = update(RefreshToken).where( @@ -75,7 +82,7 @@ def _revoke_user_sessions( statement.values( revoked=True, revoked_at=now, - revocation_reason=USER_REVOCATION_REASON, + revocation_reason=reason, ) ) db.commit() diff --git a/tests/test_admin.py b/tests/test_admin.py index 7eb8b2c..6373a2d 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -6,6 +6,7 @@ from app.auth.security import hash_password from app.db.session import SessionLocal from app.main import app +from app.models.refresh_token import RefreshToken from app.models.user import User client = TestClient(app) @@ -36,6 +37,7 @@ def create_user(role: str = "user") -> User: def delete_user_if_present(user_id: int) -> None: db = SessionLocal() try: + db.query(RefreshToken).filter(RefreshToken.user_id == user_id).delete() user = db.query(User).filter(User.id == user_id).first() if user: db.delete(user) @@ -96,3 +98,78 @@ def test_admin_can_manage_user_lifecycle(): assert missing_delete_response.json() == {"detail": "User not found"} finally: delete_user_if_present(user.id) + + +def test_admin_disables_account_and_revokes_existing_authentication(): + user = create_user() + admin_headers = auth_headers("houngdev") + + try: + login_response = client.post( + "/login/", + data={"username": user.username, "password": "secret123"}, + ) + assert login_response.status_code == 200 + tokens = login_response.json() + user_headers = { + "Authorization": f"Bearer {tokens['access_token']}", + } + + disable_response = client.patch( + f"/admin/users/{user.id}/status", + headers=admin_headers, + json={"is_active": False}, + ) + + assert disable_response.status_code == 200 + assert disable_response.json()["is_active"] is False + assert client.get("/auth/me", headers=user_headers).status_code == 401 + + disabled_login = client.post( + "/login/", + data={"username": user.username, "password": "secret123"}, + ) + assert disabled_login.status_code == 401 + assert disabled_login.json() == {"detail": "Invalid username or password"} + + refresh_response = client.post( + "/auth/refresh", + json={"refresh_token": tokens["refresh_token"]}, + ) + assert refresh_response.status_code == 401 + + db = SessionLocal() + try: + sessions = ( + db.query(RefreshToken).filter(RefreshToken.user_id == user.id).all() + ) + assert sessions + assert all(session.revoked for session in sessions) + assert all( + session.revocation_reason == "account_disabled" for session in sessions + ) + finally: + db.close() + + enable_response = client.patch( + f"/admin/users/{user.id}/status", + headers=admin_headers, + json={"is_active": True}, + ) + assert enable_response.status_code == 200 + assert enable_response.json()["is_active"] is True + + new_login = client.post( + "/login/", + data={"username": user.username, "password": "secret123"}, + ) + assert new_login.status_code == 200 + + missing_response = client.patch( + "/admin/users/2147483647/status", + headers=admin_headers, + json={"is_active": False}, + ) + assert missing_response.status_code == 404 + finally: + delete_user_if_present(user.id) diff --git a/tests/test_documentation.py b/tests/test_documentation.py index abf76b8..72311cb 100644 --- a/tests/test_documentation.py +++ b/tests/test_documentation.py @@ -31,6 +31,7 @@ DOCUMENTED_API_PATHS = { "/admin/users", "/admin/users/{user_id}/role", + "/admin/users/{user_id}/status", "/auth/logout", "/auth/email-verification/confirm", "/auth/email-verification/request", diff --git a/tests/test_refresh_failures.py b/tests/test_refresh_failures.py index 466cb8a..8f89074 100644 --- a/tests/test_refresh_failures.py +++ b/tests/test_refresh_failures.py @@ -88,6 +88,24 @@ def test_refresh_rejects_token_for_missing_user(): assert_unauthorized(error, "User not found") +def test_refresh_rejects_inactive_user_and_revokes_sessions(): + stored_token = SimpleNamespace( + revoked=False, + expires_at=datetime.now(UTC) + timedelta(days=1), + user_id=123, + family_id="family-1", + ) + user = SimpleNamespace(id=123, username="houngdev", is_active=False) + db = database_with_results(stored_token, user) + + with pytest.raises(HTTPException) as error: + refresh_access_token("inactive-user-token", db) + + assert_unauthorized(error, "Invalid refresh token") + db.execute.assert_called_once() + db.commit.assert_called_once_with() + + def test_refresh_rolls_back_when_rotation_commit_fails(): stored_token = SimpleNamespace( revoked=False, @@ -96,7 +114,7 @@ def test_refresh_rolls_back_when_rotation_commit_fails(): family_id="family-1", device_name="Test device", ) - user = SimpleNamespace(id=123, username="houngdev") + user = SimpleNamespace(id=123, username="houngdev", is_active=True) db = database_with_results(stored_token, user) db.commit.side_effect = RuntimeError("database unavailable") From 9911736e07363a9a992cc2fed33439d6763ecb6b Mon Sep 17 00:00:00 2001 From: HoungDev <311347655+HoungDev@users.noreply.github.com> Date: Tue, 11 Aug 2026 20:52:57 +0700 Subject: [PATCH 2/2] test: preserve pending MFA state during session issuance --- src/app/services/session_issuance.py | 3 +++ tests/test_session_issuance.py | 28 ++++++++++++++++++++++++++++ tests/test_transaction_rollbacks.py | 1 + 3 files changed, 32 insertions(+) create mode 100644 tests/test_session_issuance.py diff --git a/src/app/services/session_issuance.py b/src/app/services/session_issuance.py index 35c1e95..06d98f7 100644 --- a/src/app/services/session_issuance.py +++ b/src/app/services/session_issuance.py @@ -21,6 +21,9 @@ def prepare_session_tokens( *, authentication_methods: list[str], ) -> Token: + # SessionLocal disables autoflush. Preserve pending security state, such as + # an accepted MFA counter, before populate_existing reloads the locked row. + db.flush() active_user = ( db.query(User) .filter(User.id == user.id) diff --git a/tests/test_session_issuance.py b/tests/test_session_issuance.py new file mode 100644 index 0000000..b37f514 --- /dev/null +++ b/tests/test_session_issuance.py @@ -0,0 +1,28 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock + +from app.services.session_issuance import prepare_session_tokens + + +def test_session_issuance_flushes_pending_security_state_before_reload(): + events = [] + db = MagicMock() + user = SimpleNamespace(id=123, username="active-user", is_active=True) + query = MagicMock() + locked_query = query.filter.return_value.populate_existing.return_value + locked_query.with_for_update.return_value.first.return_value = user + + db.flush.side_effect = lambda: events.append("flush") + db.query.side_effect = lambda _: events.append("query") or query + + tokens = prepare_session_tokens( + user, + "Test device", + db, + authentication_methods=["otp"], + ) + + assert events[:2] == ["flush", "query"] + assert tokens.access_token + assert tokens.refresh_token + db.add.assert_called_once() diff --git a/tests/test_transaction_rollbacks.py b/tests/test_transaction_rollbacks.py index 63fb6e2..2ae2d37 100644 --- a/tests/test_transaction_rollbacks.py +++ b/tests/test_transaction_rollbacks.py @@ -59,6 +59,7 @@ def test_login_rolls_back_when_refresh_token_commit_fails(): password="hashed-password", password_login_enabled=True, mfa_enabled_at=None, + is_active=True, ) db.commit.side_effect = RuntimeError("database unavailable") form_data = SimpleNamespace(username="houngdev", password="secret123")