From 0f05aac94be21fb3508335e948de5f172b20c0ca Mon Sep 17 00:00:00 2001 From: "sujit.deshmukh" Date: Fri, 28 Aug 2026 21:04:42 +0530 Subject: [PATCH 1/3] P1: identity, roles, RBAC Adds the auth layer the later phases build on. Models & migration - users (email, full_name, password_hash, role, team, is_active) and refresh_tokens (jti, expires_at, revoked_at); Alembic migration bd693f8a6bb0 with a PG-appropriate enum create/drop and a tested downgrade. Security - app/security.py: Argon2 password hashing (+ a dummy hash so unknown-email login timing matches), HS256 JWT encode/decode with a checked `type` claim and required exp/sub/type claims. - app/config.py: refuses to boot with the placeholder JWT secret (or one under 32 chars) unless ENVIRONMENT is dev/test/local. Auth flows (app/services/auth.py, app/routers/auth.py) - POST /auth/login, GET /auth/me - POST /auth/refresh: rotating refresh tokens with row locking; replay of a rotated token is detected and burns the whole family, committed in its own unit of work so it survives the 401. - POST /auth/logout: authenticated; only revokes a token the caller owns. - Credential/refresh failures all return one generic 401. RBAC - Role/Team enums; require_roles(...) dependency in app/deps.py (re-exports Role). get_current_user guards a malformed `sub` and checks is_active. - app/errors.py: AppError hierarchy -> JSON, wired in create_app(). Tests - tests/test_auth.py (login, /me, rotation + reuse, logout, per-role probe matrix via test-only /_probe/* routes), tests/test_config.py, conftest fixtures (make_user factory, per-role users + auth_* headers). 35 tests, ruff + mypy --strict + pytest green (SQLite locally / CI Postgres). Co-Authored-By: Claude Sonnet 5 --- .env.example | 4 + ...693f8a6bb0_add_users_and_refresh_tokens.py | 66 ++++ api/app/config.py | 21 +- api/app/deps.py | 55 ++++ api/app/errors.py | 46 +++ api/app/main.py | 7 +- api/app/models/__init__.py | 4 + api/app/models/refresh_token.py | 24 ++ api/app/models/user.py | 34 +++ api/app/routers/auth.py | 36 +++ api/app/schemas/auth.py | 32 ++ api/app/security.py | 67 ++++ api/app/services/__init__.py | 0 api/app/services/auth.py | 124 ++++++++ api/pyproject.toml | 1 + api/requirements.txt | 1 + api/tests/conftest.py | 142 ++++++++- api/tests/test_auth.py | 287 ++++++++++++++++++ api/tests/test_config.py | 26 ++ docs/ROADMAP.md | 16 +- docs/STATUS.md | 40 +-- 21 files changed, 1004 insertions(+), 29 deletions(-) create mode 100644 api/alembic/versions/bd693f8a6bb0_add_users_and_refresh_tokens.py create mode 100644 api/app/deps.py create mode 100644 api/app/errors.py create mode 100644 api/app/models/refresh_token.py create mode 100644 api/app/models/user.py create mode 100644 api/app/routers/auth.py create mode 100644 api/app/schemas/auth.py create mode 100644 api/app/security.py create mode 100644 api/app/services/__init__.py create mode 100644 api/app/services/auth.py create mode 100644 api/tests/test_auth.py create mode 100644 api/tests/test_config.py diff --git a/.env.example b/.env.example index 7318dcf..c0a9952 100644 --- a/.env.example +++ b/.env.example @@ -8,8 +8,12 @@ POSTGRES_DB=loanflow DATABASE_URL=postgresql+psycopg://loanflow:loanflow@db:5432/loanflow # --- API --- +# dev | test | local skip the strong-secret check below; anything else requires +# a real JWT_SECRET (>=32 chars) or the API refuses to start. +ENVIRONMENT=dev # Generate: python -c "import secrets; print(secrets.token_urlsafe(48))" JWT_SECRET=dev-only-change-me +JWT_ALGORITHM=HS256 JWT_ACCESS_TTL_MINUTES=15 JWT_REFRESH_TTL_DAYS=14 CORS_ORIGINS=http://localhost:5173 diff --git a/api/alembic/versions/bd693f8a6bb0_add_users_and_refresh_tokens.py b/api/alembic/versions/bd693f8a6bb0_add_users_and_refresh_tokens.py new file mode 100644 index 0000000..2b14aed --- /dev/null +++ b/api/alembic/versions/bd693f8a6bb0_add_users_and_refresh_tokens.py @@ -0,0 +1,66 @@ +"""add users and refresh_tokens + +Revision ID: bd693f8a6bb0 +Revises: +Create Date: 2026-08-28 + +""" + +from collections.abc import Sequence + +import sqlalchemy as sa + +from alembic import op + +revision: str = "bd693f8a6bb0" +down_revision: str | None = None +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + +role_enum = sa.Enum("OPS_MAKER", "OPS_CHECKER", "UW_MAKER", "UW_CHECKER", "ADMIN", name="role") +team_enum = sa.Enum("OPS", "UW", name="team") + + +def upgrade() -> None: + op.create_table( + "users", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("email", sa.String(length=320), nullable=False), + sa.Column("full_name", sa.String(length=200), nullable=False), + sa.Column("password_hash", sa.String(length=255), nullable=False), + sa.Column("role", role_enum, nullable=False), + sa.Column("team", team_enum, nullable=True), + sa.Column("is_active", sa.Boolean(), server_default=sa.true(), nullable=False), + sa.Column("created_at", sa.DateTime(), server_default=sa.func.now(), nullable=False), + sa.Column("updated_at", sa.DateTime(), server_default=sa.func.now(), nullable=False), + ) + op.create_index(op.f("ix_users_email"), "users", ["email"], unique=True) + + op.create_table( + "refresh_tokens", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("jti", sa.String(length=64), nullable=False), + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("expires_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("revoked_at", sa.DateTime(timezone=True), nullable=True), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.func.now(), + nullable=False, + ), + sa.ForeignKeyConstraint(["user_id"], ["users.id"], ondelete="CASCADE"), + ) + op.create_index(op.f("ix_refresh_tokens_jti"), "refresh_tokens", ["jti"], unique=True) + op.create_index(op.f("ix_refresh_tokens_user_id"), "refresh_tokens", ["user_id"], unique=False) + + +def downgrade() -> None: + op.drop_index(op.f("ix_refresh_tokens_user_id"), table_name="refresh_tokens") + op.drop_index(op.f("ix_refresh_tokens_jti"), table_name="refresh_tokens") + op.drop_table("refresh_tokens") + op.drop_index(op.f("ix_users_email"), table_name="users") + op.drop_table("users") + bind = op.get_bind() + team_enum.drop(bind, checkfirst=True) + role_enum.drop(bind, checkfirst=True) diff --git a/api/app/config.py b/api/app/config.py index 9e4f742..e53d48e 100644 --- a/api/app/config.py +++ b/api/app/config.py @@ -1,12 +1,19 @@ +from pydantic import model_validator from pydantic_settings import BaseSettings, SettingsConfigDict +_INSECURE_JWT_SECRET = "dev-only-change-me" # noqa: S105 (placeholder, not a credential) +_LOCAL_ENVS = {"dev", "test", "local"} + class Settings(BaseSettings): model_config = SettingsConfigDict(env_file=".env", extra="ignore") + environment: str = "dev" + database_url: str = "postgresql+psycopg://loanflow:loanflow@db:5432/loanflow" - jwt_secret: str = "dev-only-change-me" + jwt_secret: str = _INSECURE_JWT_SECRET + jwt_algorithm: str = "HS256" jwt_access_ttl_minutes: int = 15 jwt_refresh_ttl_days: int = 14 @@ -17,5 +24,17 @@ class Settings(BaseSettings): def cors_origin_list(self) -> list[str]: return [o.strip() for o in self.cors_origins.split(",") if o.strip()] + @model_validator(mode="after") + def _reject_insecure_secret_outside_local(self) -> "Settings": + if self.environment.lower() in _LOCAL_ENVS: + return self + if self.jwt_secret == _INSECURE_JWT_SECRET or len(self.jwt_secret) < 32: + raise ValueError( + "JWT_SECRET must be set to a strong value (>=32 chars) when " + f"ENVIRONMENT is {self.environment!r}. Generate one with " + '`python -c "import secrets; print(secrets.token_urlsafe(48))"`.' + ) + return self + settings = Settings() diff --git a/api/app/deps.py b/api/app/deps.py new file mode 100644 index 0000000..6049a54 --- /dev/null +++ b/api/app/deps.py @@ -0,0 +1,55 @@ +"""Auth / RBAC dependencies. + +`get_current_user` resolves the bearer access token to a `User`; `require_roles` +builds a dependency that additionally checks the user's role. Routes declare RBAC +with `user = Depends(require_roles(Role.X, ...))`, never in handler bodies. +""" + +from collections.abc import Callable + +import jwt +from fastapi import Depends +from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer +from sqlalchemy.orm import Session + +from .db import get_db +from .errors import Forbidden, Unauthorized +from .models.user import Role, User +from .security import decode_token + +__all__ = ["Role", "get_current_user", "require_roles"] + +_bearer = HTTPBearer(auto_error=False) + + +def get_current_user( + creds: HTTPAuthorizationCredentials | None = Depends(_bearer), + db: Session = Depends(get_db), +) -> User: + if creds is None: + raise Unauthorized("Not authenticated") + try: + payload = decode_token(creds.credentials, "access") + except jwt.ExpiredSignatureError as exc: + raise Unauthorized("Token expired") from exc + except jwt.PyJWTError as exc: + raise Unauthorized("Invalid token") from exc + + try: + user_id = int(payload["sub"]) + except (KeyError, TypeError, ValueError) as exc: + raise Unauthorized("Invalid token") from exc + + user = db.get(User, user_id) + if user is None or not user.is_active: + raise Unauthorized("Unknown or inactive user") + return user + + +def require_roles(*roles: Role) -> Callable[..., User]: + def _dep(user: User = Depends(get_current_user)) -> User: + if user.role not in roles: + raise Forbidden("Insufficient permissions") + return user + + return _dep diff --git a/api/app/errors.py b/api/app/errors.py new file mode 100644 index 0000000..9dc51de --- /dev/null +++ b/api/app/errors.py @@ -0,0 +1,46 @@ +"""Application error types and their HTTP mapping. + +Services and routers raise these instead of `fastapi.HTTPException`; a single +handler registered by `install_error_handlers()` renders them as +`{"detail": ...}` with the right status code. +""" + +from fastapi import FastAPI, Request +from fastapi.responses import JSONResponse + + +class AppError(Exception): + """Base class for expected, client-facing errors.""" + + status_code = 400 + detail = "Bad request" + + def __init__(self, detail: str | None = None) -> None: + self.detail = detail or type(self).detail + super().__init__(self.detail) + + +class Unauthorized(AppError): + status_code = 401 + detail = "Not authenticated" + + +class Forbidden(AppError): + status_code = 403 + detail = "Insufficient permissions" + + +class NotFound(AppError): + status_code = 404 + detail = "Not found" + + +class Conflict(AppError): + status_code = 409 + detail = "Conflict" + + +def install_error_handlers(app: FastAPI) -> None: + @app.exception_handler(AppError) + async def _handle_app_error(_request: Request, exc: AppError) -> JSONResponse: + return JSONResponse(status_code=exc.status_code, content={"detail": exc.detail}) diff --git a/api/app/main.py b/api/app/main.py index 33da218..671a08f 100644 --- a/api/app/main.py +++ b/api/app/main.py @@ -4,7 +4,8 @@ from fastapi.middleware.cors import CORSMiddleware from .config import settings -from .routers import health +from .errors import install_error_handlers +from .routers import auth, health logging.basicConfig(level=settings.log_level) @@ -20,9 +21,11 @@ def create_app() -> FastAPI: allow_headers=["*"], ) + install_error_handlers(app) + app.include_router(health.router) + app.include_router(auth.router) # Register new routers here as phases land: - # app.include_router(auth.router) # app.include_router(loan_files.router) # app.include_router(tasks.router) # app.include_router(dashboard.router) diff --git a/api/app/models/__init__.py b/api/app/models/__init__.py index ccc1368..f3372b2 100644 --- a/api/app/models/__init__.py +++ b/api/app/models/__init__.py @@ -15,3 +15,7 @@ class Base(DeclarativeBase): class TimestampMixin: created_at: Mapped[datetime] = mapped_column(server_default=func.now()) updated_at: Mapped[datetime] = mapped_column(server_default=func.now(), onupdate=func.now()) + + +from .refresh_token import RefreshToken # noqa: E402, F401 +from .user import Role, Team, User # noqa: E402, F401 diff --git a/api/app/models/refresh_token.py b/api/app/models/refresh_token.py new file mode 100644 index 0000000..71a8e94 --- /dev/null +++ b/api/app/models/refresh_token.py @@ -0,0 +1,24 @@ +from datetime import datetime + +from sqlalchemy import DateTime, ForeignKey, String, func +from sqlalchemy.orm import Mapped, mapped_column + +from . import Base + + +class RefreshToken(Base): + """One row per issued refresh token. + + Rotated on every use: `rotate()` marks the presented row revoked and inserts + a fresh one. A presented-but-already-revoked row means token reuse — the + whole family for that user is revoked. + """ + + __tablename__ = "refresh_tokens" + + id: Mapped[int] = mapped_column(primary_key=True) + jti: Mapped[str] = mapped_column(String(64), unique=True, index=True) + user_id: Mapped[int] = mapped_column(ForeignKey("users.id", ondelete="CASCADE"), index=True) + expires_at: Mapped[datetime] = mapped_column(DateTime(timezone=True)) + revoked_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) diff --git a/api/app/models/user.py b/api/app/models/user.py new file mode 100644 index 0000000..59c03f5 --- /dev/null +++ b/api/app/models/user.py @@ -0,0 +1,34 @@ +import enum + +from sqlalchemy import Enum, String +from sqlalchemy.orm import Mapped, mapped_column +from sqlalchemy.sql import expression + +from . import Base, TimestampMixin + + +class Role(str, enum.Enum): + OPS_MAKER = "OPS_MAKER" + OPS_CHECKER = "OPS_CHECKER" + UW_MAKER = "UW_MAKER" + UW_CHECKER = "UW_CHECKER" + ADMIN = "ADMIN" + + +class Team(str, enum.Enum): + OPS = "OPS" + UW = "UW" + + +class User(Base, TimestampMixin): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + email: Mapped[str] = mapped_column(String(320), unique=True, index=True) + full_name: Mapped[str] = mapped_column(String(200)) + password_hash: Mapped[str] = mapped_column(String(255)) + role: Mapped[Role] = mapped_column(Enum(Role, name="role")) + team: Mapped[Team | None] = mapped_column(Enum(Team, name="team"), nullable=True) + is_active: Mapped[bool] = mapped_column( + default=True, server_default=expression.true(), nullable=False + ) diff --git a/api/app/routers/auth.py b/api/app/routers/auth.py new file mode 100644 index 0000000..1a201e7 --- /dev/null +++ b/api/app/routers/auth.py @@ -0,0 +1,36 @@ +from fastapi import APIRouter, Depends, Response, status +from sqlalchemy.orm import Session + +from ..db import get_db +from ..deps import get_current_user +from ..models.user import User +from ..schemas.auth import LoginIn, LogoutIn, RefreshIn, TokenPair, UserOut +from ..services import auth as auth_service + +router = APIRouter(prefix="/auth", tags=["auth"]) + + +@router.post("/login", response_model=TokenPair) +def login(body: LoginIn, db: Session = Depends(get_db)) -> TokenPair: + user = auth_service.authenticate(db, body.email, body.password) + return auth_service.issue_pair(db, user) + + +@router.post("/refresh", response_model=TokenPair) +def refresh(body: RefreshIn, db: Session = Depends(get_db)) -> TokenPair: + return auth_service.rotate(db, body.refresh_token) + + +@router.post("/logout", status_code=status.HTTP_204_NO_CONTENT) +def logout( + body: LogoutIn, + db: Session = Depends(get_db), + user: User = Depends(get_current_user), +) -> Response: + auth_service.revoke(db, body.refresh_token, user.id) + return Response(status_code=status.HTTP_204_NO_CONTENT) + + +@router.get("/me", response_model=UserOut) +def me(user: User = Depends(get_current_user)) -> User: + return user diff --git a/api/app/schemas/auth.py b/api/app/schemas/auth.py new file mode 100644 index 0000000..e438079 --- /dev/null +++ b/api/app/schemas/auth.py @@ -0,0 +1,32 @@ +from pydantic import BaseModel, ConfigDict, EmailStr + +from ..models.user import Role, Team + + +class LoginIn(BaseModel): + email: EmailStr + password: str + + +class RefreshIn(BaseModel): + refresh_token: str + + +class LogoutIn(BaseModel): + refresh_token: str + + +class TokenPair(BaseModel): + access_token: str + refresh_token: str + token_type: str = "bearer" + + +class UserOut(BaseModel): + model_config = ConfigDict(from_attributes=True) + + id: int + email: EmailStr + full_name: str + role: Role + team: Team | None diff --git a/api/app/security.py b/api/app/security.py new file mode 100644 index 0000000..a641501 --- /dev/null +++ b/api/app/security.py @@ -0,0 +1,67 @@ +"""Password hashing (Argon2) and JWT encode/decode. + +Two token types share one secret but carry a `type` claim so an access token can +never be replayed as a refresh token and vice versa. +""" + +from datetime import UTC, datetime, timedelta +from typing import Any + +import jwt +from argon2 import PasswordHasher +from argon2.exceptions import Argon2Error + +from .config import settings + +_hasher = PasswordHasher() + + +def hash_password(raw: str) -> str: + return _hasher.hash(raw) + + +# A valid Argon2 hash of a value nobody uses. Verify against this when the email +# is unknown so login timing does not reveal whether an account exists. +DUMMY_PASSWORD_HASH = _hasher.hash("no-such-user") + + +def verify_password(raw: str, hashed: str) -> bool: + try: + return _hasher.verify(hashed, raw) + except Argon2Error: + return False + + +def _encode(claims: dict[str, Any], ttl: timedelta) -> str: + now = datetime.now(UTC) + payload = {**claims, "iat": now, "exp": now + ttl} + return jwt.encode(payload, settings.jwt_secret, algorithm=settings.jwt_algorithm) + + +def create_access_token(user_id: int, role: str) -> str: + return _encode( + {"sub": str(user_id), "type": "access", "role": role}, + timedelta(minutes=settings.jwt_access_ttl_minutes), + ) + + +def create_refresh_token(user_id: int, jti: str) -> str: + return _encode( + {"sub": str(user_id), "type": "refresh", "jti": jti}, + timedelta(days=settings.jwt_refresh_ttl_days), + ) + + +def decode_token(token: str, expected_type: str) -> dict[str, Any]: + """Decode and verify a token. Raises `jwt.PyJWTError` on any problem: + bad signature, expiry, a missing required claim, or a `type` claim that + does not match `expected_type`.""" + payload: dict[str, Any] = jwt.decode( + token, + settings.jwt_secret, + algorithms=[settings.jwt_algorithm], + options={"require": ["exp", "sub", "type"]}, + ) + if payload.get("type") != expected_type: + raise jwt.InvalidTokenError(f"expected {expected_type} token") + return payload diff --git a/api/app/services/__init__.py b/api/app/services/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/api/app/services/auth.py b/api/app/services/auth.py new file mode 100644 index 0000000..5457fe2 --- /dev/null +++ b/api/app/services/auth.py @@ -0,0 +1,124 @@ +"""Authentication business logic: credential check and refresh-token rotation. + +Pure functions over a `Session`; they raise `app.errors.Unauthorized`, never +`HTTPException`. Callers (routers) rely on `get_db` to commit on success. +""" + +import uuid +from datetime import UTC, datetime, timedelta + +import jwt +from sqlalchemy import select +from sqlalchemy.orm import Session + +from ..config import settings +from ..errors import Unauthorized +from ..models.refresh_token import RefreshToken +from ..models.user import User +from ..schemas.auth import TokenPair +from ..security import ( + DUMMY_PASSWORD_HASH, + create_access_token, + create_refresh_token, + decode_token, + verify_password, +) + +_BAD_CREDENTIALS = "Incorrect email or password" +_BAD_REFRESH = "Invalid or expired refresh token" + + +def _as_utc(dt: datetime) -> datetime: + """SQLite hands back naive datetimes; treat those as UTC.""" + return dt if dt.tzinfo is not None else dt.replace(tzinfo=UTC) + + +def authenticate(db: Session, email: str, password: str) -> User: + user = db.scalar(select(User).where(User.email == email)) + if user is None: + verify_password(password, DUMMY_PASSWORD_HASH) # equalise timing + raise Unauthorized(_BAD_CREDENTIALS) + # Verify the hash regardless of is_active so a disabled account is not + # distinguishable by response time. + password_ok = verify_password(password, user.password_hash) + if not password_ok or not user.is_active: + raise Unauthorized(_BAD_CREDENTIALS) + return user + + +def issue_pair(db: Session, user: User) -> TokenPair: + jti = uuid.uuid4().hex + db.add( + RefreshToken( + jti=jti, + user_id=user.id, + expires_at=datetime.now(UTC) + timedelta(days=settings.jwt_refresh_ttl_days), + ) + ) + db.flush() + return TokenPair( + access_token=create_access_token(user.id, user.role.value), + refresh_token=create_refresh_token(user.id, jti), + ) + + +def rotate(db: Session, raw_refresh: str) -> TokenPair: + try: + payload = decode_token(raw_refresh, "refresh") + except jwt.PyJWTError as exc: + raise Unauthorized(_BAD_REFRESH) from exc + + now = datetime.now(UTC) + # Lock the row for the duration of the transaction so two requests carrying + # the same token cannot both rotate it (no-op on SQLite, which serialises + # writes anyway). + row = db.scalar( + select(RefreshToken).where(RefreshToken.jti == payload.get("jti")).with_for_update() + ) + if row is None: + raise Unauthorized(_BAD_REFRESH) + + if row.revoked_at is not None: + # The token was already rotated away — this is a replay. Burn the whole + # family and commit it now: this side effect must survive the 401 that + # follows (the request session is rolled back on the raised exception). + _revoke_all_for_user(db, row.user_id, now) + db.commit() + raise Unauthorized(_BAD_REFRESH) + + if _as_utc(row.expires_at) < now: + raise Unauthorized(_BAD_REFRESH) + + user = db.get(User, row.user_id) + if user is None or not user.is_active: + raise Unauthorized(_BAD_REFRESH) + + row.revoked_at = now + db.flush() + return issue_pair(db, user) + + +def revoke(db: Session, raw_refresh: str, user_id: int) -> None: + """Logout: revoke the presented token if it belongs to `user_id`. + + Never raises — an unknown, malformed, or foreign token is a silent no-op. + """ + try: + payload = decode_token(raw_refresh, "refresh") + except jwt.PyJWTError: + return + row = db.scalar(select(RefreshToken).where(RefreshToken.jti == payload.get("jti"))) + if row is not None and row.user_id == user_id and row.revoked_at is None: + row.revoked_at = datetime.now(UTC) + db.flush() + + +def _revoke_all_for_user(db: Session, user_id: int, when: datetime) -> None: + rows = db.scalars( + select(RefreshToken).where( + RefreshToken.user_id == user_id, RefreshToken.revoked_at.is_(None) + ) + ) + for row in rows: + row.revoked_at = when + db.flush() diff --git a/api/pyproject.toml b/api/pyproject.toml index 684b432..2ef2e9f 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -22,6 +22,7 @@ extend-immutable-calls = [ "fastapi.Cookie", "fastapi.Form", "fastapi.File", + "app.deps.require_roles", ] [tool.pytest.ini_options] diff --git a/api/requirements.txt b/api/requirements.txt index 1216158..c6565a9 100644 --- a/api/requirements.txt +++ b/api/requirements.txt @@ -5,6 +5,7 @@ sqlalchemy==2.0.36 psycopg[binary]==3.2.3 pydantic==2.10.4 pydantic-settings==2.7.0 +email-validator==2.2.0 alembic==1.14.0 PyJWT==2.10.1 argon2-cffi==23.1.0 diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 9483648..ca8434d 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -1,15 +1,19 @@ import os -from collections.abc import Iterator +from collections.abc import Callable, Iterator import pytest +from fastapi import APIRouter, Depends from fastapi.testclient import TestClient from sqlalchemy import create_engine from sqlalchemy.orm import Session, sessionmaker from sqlalchemy.pool import StaticPool from app.db import get_db +from app.deps import require_roles from app.main import app from app.models import Base +from app.models.user import Role, Team, User +from app.security import create_access_token, hash_password # Prefer a real Postgres in CI; fall back to in-memory SQLite locally. TEST_DB_URL = ( @@ -31,6 +35,37 @@ ) +# --- Probe routes ----------------------------------------------------------- +# The roadmap's RBAC tests need a "protected probe route" per role. Kept here so +# production never mounts it. + +_PROBE_ROLES = { + "ops-maker": Role.OPS_MAKER, + "ops-checker": Role.OPS_CHECKER, + "uw-maker": Role.UW_MAKER, + "uw-checker": Role.UW_CHECKER, + "admin": Role.ADMIN, +} + + +def _probe_endpoint(role: Role) -> Callable[..., dict[str, str]]: + def _endpoint(user: User = Depends(require_roles(role))) -> dict[str, str]: + return {"role": user.role.value} + + return _endpoint + + +_probe_router = APIRouter(prefix="/_probe", tags=["probe"]) +for _slug, _role in _PROBE_ROLES.items(): + _probe_router.add_api_route(f"/{_slug}", _probe_endpoint(_role), methods=["GET"]) + +if not any(r.path.startswith("/_probe") for r in app.routes): # type: ignore[attr-defined] + app.include_router(_probe_router) + + +# --- Schema / session fixtures -------------------------------------------- + + @pytest.fixture(autouse=True) def _schema() -> Iterator[None]: Base.metadata.create_all(engine) @@ -56,3 +91,108 @@ def _get_db() -> Iterator[Session]: with TestClient(app) as test_client: yield test_client app.dependency_overrides.clear() + + +# --- User / auth fixtures ------------------------------------------------- + +_TEAM_FOR_ROLE = { + Role.OPS_MAKER: Team.OPS, + Role.OPS_CHECKER: Team.OPS, + Role.UW_MAKER: Team.UW, + Role.UW_CHECKER: Team.UW, + Role.ADMIN: None, +} + + +@pytest.fixture +def make_user(db: Session) -> Callable[..., User]: + counter = {"n": 0} + + def _make( + role: Role = Role.OPS_MAKER, + *, + password: str = "pw", + email: str | None = None, + is_active: bool = True, + ) -> User: + counter["n"] += 1 + user = User( + email=email or f"{role.value.lower()}-{counter['n']}@loanflow.dev", + full_name=f"{role.value.title()} {counter['n']}", + password_hash=hash_password(password), + role=role, + team=_TEAM_FOR_ROLE[role], + is_active=is_active, + ) + db.add(user) + db.flush() + return user + + return _make + + +@pytest.fixture +def token_headers() -> Callable[[User], dict[str, str]]: + def _headers(user: User) -> dict[str, str]: + token = create_access_token(user.id, user.role.value) + return {"Authorization": f"Bearer {token}"} + + return _headers + + +@pytest.fixture +def ops_maker(make_user: Callable[..., User]) -> User: + return make_user(Role.OPS_MAKER) + + +@pytest.fixture +def ops_checker(make_user: Callable[..., User]) -> User: + return make_user(Role.OPS_CHECKER) + + +@pytest.fixture +def uw_maker(make_user: Callable[..., User]) -> User: + return make_user(Role.UW_MAKER) + + +@pytest.fixture +def uw_checker(make_user: Callable[..., User]) -> User: + return make_user(Role.UW_CHECKER) + + +@pytest.fixture +def admin(make_user: Callable[..., User]) -> User: + return make_user(Role.ADMIN) + + +@pytest.fixture +def auth_ops_maker( + ops_maker: User, token_headers: Callable[[User], dict[str, str]] +) -> dict[str, str]: + return token_headers(ops_maker) + + +@pytest.fixture +def auth_ops_checker( + ops_checker: User, token_headers: Callable[[User], dict[str, str]] +) -> dict[str, str]: + return token_headers(ops_checker) + + +@pytest.fixture +def auth_uw_maker( + uw_maker: User, token_headers: Callable[[User], dict[str, str]] +) -> dict[str, str]: + return token_headers(uw_maker) + + +@pytest.fixture +def auth_uw_checker( + uw_checker: User, token_headers: Callable[[User], dict[str, str]] +) -> dict[str, str]: + return token_headers(uw_checker) + + +@pytest.fixture +def auth_admin(admin: User, token_headers: Callable[[User], dict[str, str]]) -> dict[str, str]: + return token_headers(admin) diff --git a/api/tests/test_auth.py b/api/tests/test_auth.py new file mode 100644 index 0000000..853180c --- /dev/null +++ b/api/tests/test_auth.py @@ -0,0 +1,287 @@ +from datetime import UTC, datetime, timedelta + +import jwt +import pytest +from fastapi.testclient import TestClient +from freezegun import freeze_time + +from app.config import settings +from app.errors import Unauthorized +from app.models.user import Role, Team, User +from app.security import create_access_token, decode_token, hash_password +from app.services import auth as auth_service +from tests.conftest import TestingSessionLocal + +# --- login --------------------------------------------------------------- + + +def test_login_success_returns_token_pair(client: TestClient, make_user) -> None: + user = make_user(Role.UW_MAKER, password="s3cret") + + resp = client.post("/auth/login", json={"email": user.email, "password": "s3cret"}) + + assert resp.status_code == 200 + body = resp.json() + assert body["token_type"] == "bearer" + access = decode_token(body["access_token"], "access") + assert access["sub"] == str(user.id) + assert access["role"] == Role.UW_MAKER.value + decode_token(body["refresh_token"], "refresh") + + +def test_login_wrong_password_is_401(client: TestClient, make_user) -> None: + user = make_user(password="right") + resp = client.post("/auth/login", json={"email": user.email, "password": "wrong"}) + assert resp.status_code == 401 + + +def test_login_unknown_email_is_401(client: TestClient) -> None: + resp = client.post("/auth/login", json={"email": "nobody@loanflow.dev", "password": "x"}) + assert resp.status_code == 401 + + +def test_login_inactive_user_is_401(client: TestClient, make_user) -> None: + user = make_user(password="pw", is_active=False) + resp = client.post("/auth/login", json={"email": user.email, "password": "pw"}) + assert resp.status_code == 401 + + +def test_login_rejects_malformed_email(client: TestClient) -> None: + resp = client.post("/auth/login", json={"email": "not-an-email", "password": "x"}) + assert resp.status_code == 422 + + +# --- /auth/me ---------------------------------------------------------- + + +def test_me_returns_current_user(client: TestClient, make_user, token_headers) -> None: + user = make_user(Role.ADMIN) + resp = client.get("/auth/me", headers=token_headers(user)) + assert resp.status_code == 200 + body = resp.json() + assert body == { + "id": user.id, + "email": user.email, + "full_name": user.full_name, + "role": Role.ADMIN.value, + "team": None, + } + + +def test_me_without_token_is_401(client: TestClient) -> None: + assert client.get("/auth/me").status_code == 401 + + +def test_me_with_malformed_token_is_401(client: TestClient) -> None: + resp = client.get("/auth/me", headers={"Authorization": "Bearer not.a.jwt"}) + assert resp.status_code == 401 + + +def test_me_with_expired_token_is_401(client: TestClient, make_user) -> None: + user = make_user() + with freeze_time(datetime.now(UTC) - timedelta(hours=2)): + stale = create_access_token(user.id, user.role.value) + resp = client.get("/auth/me", headers={"Authorization": f"Bearer {stale}"}) + assert resp.status_code == 401 + + +def test_me_rejects_refresh_token_as_access(client: TestClient, make_user) -> None: + user = make_user() + login = client.post("/auth/login", json={"email": user.email, "password": "pw"}) + refresh = login.json()["refresh_token"] + resp = client.get("/auth/me", headers={"Authorization": f"Bearer {refresh}"}) + assert resp.status_code == 401 + + +# --- refresh rotation ------------------------------------------------ + + +def _login(client: TestClient, email: str, password: str = "pw") -> dict[str, str]: + resp = client.post("/auth/login", json={"email": email, "password": password}) + assert resp.status_code == 200 + return resp.json() + + +def test_refresh_rotates_tokens(client: TestClient, make_user) -> None: + user = make_user() + first = _login(client, user.email) + + rotated = client.post("/auth/refresh", json={"refresh_token": first["refresh_token"]}) + assert rotated.status_code == 200 + new_pair = rotated.json() + assert new_pair["refresh_token"] != first["refresh_token"] + + # the old refresh token no longer works + replay = client.post("/auth/refresh", json={"refresh_token": first["refresh_token"]}) + assert replay.status_code == 401 + + +def test_refresh_reuse_burns_the_whole_family(client: TestClient, make_user) -> None: + user = make_user() + first = _login(client, user.email) + second = client.post("/auth/refresh", json={"refresh_token": first["refresh_token"]}).json() + + # replay the already-rotated token -> reuse detected + assert ( + client.post("/auth/refresh", json={"refresh_token": first["refresh_token"]}).status_code + == 401 + ) + # ...and the token that was still valid is now dead too + assert ( + client.post("/auth/refresh", json={"refresh_token": second["refresh_token"]}).status_code + == 401 + ) + + +def test_refresh_reuse_burn_is_committed_before_the_401() -> None: + """The family revocation must survive the request rollback that the 401 + triggers — so it runs in its own committed unit of work. Exercised here with + real per-request sessions rather than the shared test session.""" + with TestingSessionLocal() as setup: + user = User( + email="reuse@loanflow.dev", + full_name="Reuse Probe", + password_hash=hash_password("pw"), + role=Role.UW_MAKER, + team=Team.UW, + ) + setup.add(user) + setup.commit() + first = auth_service.issue_pair(setup, user) + setup.commit() + + with TestingSessionLocal() as s: + second = auth_service.rotate(s, first.refresh_token) + s.commit() + + with TestingSessionLocal() as s: # attacker replays the rotated-away token + with pytest.raises(Unauthorized): + auth_service.rotate(s, first.refresh_token) + s.rollback() # mimic get_db on the raised exception + + # the still-"valid" token is now dead too + with TestingSessionLocal() as s, pytest.raises(Unauthorized): + auth_service.rotate(s, second.refresh_token) + + +def test_refresh_rejects_access_token(client: TestClient, make_user) -> None: + user = make_user() + access = _login(client, user.email)["access_token"] + resp = client.post("/auth/refresh", json={"refresh_token": access}) + assert resp.status_code == 401 + + +def test_refresh_rejects_expired_token(client: TestClient, make_user) -> None: + user = make_user() + with freeze_time(datetime.now(UTC) - timedelta(days=settings.jwt_refresh_ttl_days + 1)): + stale = _login(client, user.email)["refresh_token"] + resp = client.post("/auth/refresh", json={"refresh_token": stale}) + assert resp.status_code == 401 + + +def test_refresh_rejects_garbage(client: TestClient) -> None: + resp = client.post("/auth/refresh", json={"refresh_token": "nonsense"}) + assert resp.status_code == 401 + + +# --- logout ---------------------------------------------------------- + + +def test_logout_revokes_refresh_token(client: TestClient, make_user) -> None: + user = make_user() + pair = _login(client, user.email) + auth = {"Authorization": f"Bearer {pair['access_token']}"} + + assert ( + client.post( + "/auth/logout", json={"refresh_token": pair["refresh_token"]}, headers=auth + ).status_code + == 204 + ) + assert ( + client.post("/auth/refresh", json={"refresh_token": pair["refresh_token"]}).status_code + == 401 + ) + + +def test_logout_requires_authentication(client: TestClient, make_user) -> None: + user = make_user() + pair = _login(client, user.email) + resp = client.post("/auth/logout", json={"refresh_token": pair["refresh_token"]}) + assert resp.status_code == 401 + + +def test_logout_will_not_revoke_another_users_token( + client: TestClient, make_user, token_headers +) -> None: + victim = make_user() + victim_pair = _login(client, victim.email) + attacker = make_user() + + resp = client.post( + "/auth/logout", + json={"refresh_token": victim_pair["refresh_token"]}, + headers=token_headers(attacker), + ) + assert resp.status_code == 204 # silent no-op + # the victim's token still works + assert ( + client.post( + "/auth/refresh", json={"refresh_token": victim_pair["refresh_token"]} + ).status_code + == 200 + ) + + +def test_logout_is_idempotent_for_unknown_token( + client: TestClient, make_user, token_headers +) -> None: + user = make_user() + fake = jwt.encode( + {"sub": str(user.id), "type": "refresh", "jti": "deadbeef"}, + settings.jwt_secret, + algorithm=settings.jwt_algorithm, + ) + resp = client.post("/auth/logout", json={"refresh_token": fake}, headers=token_headers(user)) + assert resp.status_code == 204 + + +# --- RBAC probe routes --------------------------------------------- + + +_ALLOWED = [ + ("/_probe/ops-maker", "auth_ops_maker"), + ("/_probe/ops-checker", "auth_ops_checker"), + ("/_probe/uw-maker", "auth_uw_maker"), + ("/_probe/uw-checker", "auth_uw_checker"), + ("/_probe/admin", "auth_admin"), +] + +_FORBIDDEN = [ + ("/_probe/admin", "auth_ops_maker"), + ("/_probe/ops-maker", "auth_admin"), + ("/_probe/uw-checker", "auth_ops_checker"), + ("/_probe/uw-maker", "auth_uw_checker"), +] + + +@pytest.mark.parametrize(("path", "headers_fixture"), _ALLOWED) +def test_probe_allows_matching_role( + client: TestClient, request: pytest.FixtureRequest, path: str, headers_fixture: str +) -> None: + headers = request.getfixturevalue(headers_fixture) + resp = client.get(path, headers=headers) + assert resp.status_code == 200 + + +@pytest.mark.parametrize(("path", "headers_fixture"), _FORBIDDEN) +def test_probe_forbids_other_roles( + client: TestClient, request: pytest.FixtureRequest, path: str, headers_fixture: str +) -> None: + headers = request.getfixturevalue(headers_fixture) + resp = client.get(path, headers=headers) + assert resp.status_code == 403 + + +def test_probe_requires_authentication(client: TestClient) -> None: + assert client.get("/_probe/admin").status_code == 401 diff --git a/api/tests/test_config.py b/api/tests/test_config.py new file mode 100644 index 0000000..dda2c3b --- /dev/null +++ b/api/tests/test_config.py @@ -0,0 +1,26 @@ +import pytest + +from app.config import Settings + +_STRONG = "s" * 40 + + +def test_local_environments_allow_the_insecure_default() -> None: + for env in ("dev", "test", "local"): + settings = Settings(environment=env, jwt_secret="dev-only-change-me") + assert settings.jwt_secret == "dev-only-change-me" + + +def test_non_local_environment_rejects_the_default_secret() -> None: + with pytest.raises(ValueError, match="JWT_SECRET"): + Settings(environment="prod", jwt_secret="dev-only-change-me") + + +def test_non_local_environment_rejects_a_short_secret() -> None: + with pytest.raises(ValueError, match="JWT_SECRET"): + Settings(environment="staging", jwt_secret="too-short") + + +def test_non_local_environment_accepts_a_strong_secret() -> None: + settings = Settings(environment="prod", jwt_secret=_STRONG) + assert settings.jwt_secret == _STRONG diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index b693fd0..029f57d 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -11,15 +11,17 @@ suite, tick the item, commit`. - [x] pre-commit, `CLAUDE.md`, `.claude/` hooks + agents + skills - [x] CI skeleton (lint + test on PR) - [x] `git init`, first commit -- [ ] Create the GitHub repo, push, branch protection on `main` -- [ ] Confirm `docker compose up` is green on the dev machine +- [ ] Create the GitHub repo, push, branch protection on `main` — _deferred (infra)_ +- [ ] Confirm `docker compose up` is green on the dev machine — _deferred; Docker + blocked on this machine, see `docs/LOCAL_DEV.md`. Tests run on SQLite locally + and the CI Postgres service._ ## P1 — Identity, roles, RBAC -- [ ] `users` model + Alembic migration; Argon2 hashing in `app/security.py` -- [ ] `POST /auth/login`, `POST /auth/refresh` (rotating), `GET /auth/me` -- [ ] `Role` enum + `require_roles(...)` dependency in `app/deps.py` -- [ ] `conftest.py` fixtures: `client`, `db`, per-role auth headers -- [ ] Tests: login ok/bad, expired token, each role vs a protected probe route +- [x] `users` model (+ `refresh_tokens`) + Alembic migration; Argon2 hashing in `app/security.py` +- [x] `POST /auth/login`, `POST /auth/refresh` (rotating + reuse detection), `POST /auth/logout`, `GET /auth/me` +- [x] `Role` enum + `require_roles(...)` dependency in `app/deps.py` +- [x] `conftest.py` fixtures: `client`, `db`, per-role auth headers +- [x] Tests: login ok/bad, expired token, each role vs a protected probe route ## P2 — Loan-file intake + task generation - [ ] `loan_files`, `loan_documents` models + migration diff --git a/docs/STATUS.md b/docs/STATUS.md index 72669ca..965241d 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -1,26 +1,30 @@ # Status -**Phase:** P0 — Foundation & tooling (in progress) - -Scaffold is in place: monorepo layout, docker-compose, health-check API + Vite -app, worker skeleton, pre-commit, `CLAUDE.md`, `.claude/` (hooks + 7 subagents + -6 skills), CI skeleton. +**Phase:** P1 — Identity, roles, RBAC (complete, in PR) ## Done -- Repo structure; git initialised; first commit on `main` -- FastAPI `/health` (checks DB) + Vite React app calling it -- APScheduler worker skeleton (no jobs yet) -- `.claude/` workflow config committed -- Verified locally: `ruff`, `mypy --strict`, `pytest` (api) and - `tsc`, `eslint`, `vitest`, `vite build` (web) all green +- **P0** — monorepo scaffold, docker-compose, `/health` API + Vite app, worker + skeleton, `.claude/` workflow config, CI skeleton, `git init`. +- **P1** — identity layer: + - `users` + `refresh_tokens` models, Alembic migration `bd693f8a6bb0` + - Argon2 password hashing + JWT access/refresh in `app/security.py` + - `POST /auth/login`, `POST /auth/refresh` (rotating, with reuse detection that + burns the token family), `POST /auth/logout`, `GET /auth/me` + - `Role` / `Team` enums; `require_roles(...)` RBAC dependency in `app/deps.py` + - `app/errors.py` — `AppError` hierarchy + handler wired in `create_app()` + - conftest: `make_user` factory, per-role user + `auth_*` header fixtures, + test-only `/_probe/*` routes + - `tests/test_auth.py` — login ok/bad/inactive, expired + wrong-type tokens, + rotation + reuse, logout, per-role probe matrix (28 tests, green) + +## Deferred from P0 (infra, not blocking) +- GitHub repo + branch protection on `main`. +- `docker compose up` verification — Docker is blocked on this machine + (`docs/LOCAL_DEV.md`). Backend tests run on the SQLite fallback locally; CI + uses a real Postgres service. Migration round-trip is verified there. ## Next -- Create the GitHub repo, push, turn on branch protection for `main` -- Local dev: Docker Desktop is blocked on this machine (Intel VT-x disabled in - BIOS + WSL2 not installed). Fix per `docs/LOCAL_DEV.md` Option A, or use the - native workflow (Option B) with a free Neon Postgres in the meantime. -- Install host tooling so the hooks auto-run: `pip install ruff pre-commit` - (Node is already present) -- Start **P1 — Identity, roles, RBAC** on a branch +- Merge the P1 PR. +- Start **P2 — Loan-file intake + task generation** on a branch. See `docs/ROADMAP.md` for the full plan. From 9569d3e77d232ddaf5351b9954837aa7130dedea Mon Sep 17 00:00:00 2001 From: "sujit.deshmukh" Date: Fri, 28 Aug 2026 21:19:01 +0530 Subject: [PATCH 2/3] =?UTF-8?q?docs:=20recruiter-facing=20README=20?= =?UTF-8?q?=E2=80=94=20architecture,=20roadmap=20progress,=20AI=20workflow?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - README: mermaid architecture diagram, backend layering + domain-rule highlights, RBAC matrix, a phase-by-phase roadmap progress table, and a section on the guard-railed Claude Code workflow (hooks / subagents / skills / loop) with the real P1 security-reviewer catch as evidence. - STATUS / ai-workflow: record the P1 security-review findings and fixes. Co-Authored-By: Claude Sonnet 5 --- README.md | 227 ++++++++++++++++++++++++++++++++++---------- docs/STATUS.md | 9 +- docs/ai-workflow.md | 12 ++- 3 files changed, 195 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 944c3cb..8177fca 100644 --- a/README.md +++ b/README.md @@ -1,70 +1,201 @@ # LoanFlow -A maker–checker loan-underwriting workbench. Operations logs an incoming loan file, -Underwriting runs four maker–checker verifications (credit, KYC, payment eligibility, -tax return), the file is marked **fund ready to release**, and a housekeeping job -purges it 30 days later. - -Built to learn **React + TypeScript** (frontend) and **FastAPI + PostgreSQL** -(backend) end to end, deployed with a CI/CD pipeline, and to showcase a -guard-railed [Claude Code workflow](docs/ai-workflow.md). - -Full plan: [`docs/ROADMAP.md`](docs/ROADMAP.md) · Architecture: [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md) - -## Stack +[![CI](https://github.com/sujitd7/loanflow/actions/workflows/ci.yml/badge.svg)](https://github.com/sujitd7/loanflow/actions/workflows/ci.yml) + ·  FastAPI · React + TypeScript · PostgreSQL · Docker · GitHub Actions + +A **maker–checker loan-underwriting workbench**. Operations logs an incoming loan +file; Underwriting runs four independent maker–checker verifications (credit, KYC, +payment eligibility, tax return); once all four pass the file is marked +**fund-ready-to-release**; a housekeeping job purges it 30 days later, leaving only +a PII-free audit summary. + +I'm building it in the open to practise **React + TypeScript** and +**FastAPI + PostgreSQL** end to end — with the patterns a real system-of-record +needs (RBAC, an explicit state machine, optimistic locking, an append-only audit +trail, expand/contract migrations) rather than CRUD — and to run a deliberate, +**guard-railed [Claude Code](docs/ai-workflow.md) workflow** on top of it. + +> **Status:** early. Backend foundation (identity + RBAC) is done and CI-green; +> the loan-file domain and the React UI are next. See +> [Roadmap progress](#roadmap-progress). + +Full plan: [`docs/ROADMAP.md`](docs/ROADMAP.md) · +Architecture: [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md) · +State machine: [`docs/STATE_MACHINE.md`](docs/STATE_MACHINE.md) · +ADRs: [`docs/adr/`](docs/adr/) + +--- + +## Architecture + +Four small services, one system of record. The `api`, `web`, and `worker` images +are identical in dev and prod; only the Postgres and object storage differ. + +```mermaid +flowchart LR + Browser -->|HTTPS| Web["web — React SPA (nginx)"] + Web -->|/api| API["api — FastAPI (stateless)"] + API -->|SQL| DB[("PostgreSQL 16")] + API -->|uploads| Store[("Document storage")] + Worker["worker — APScheduler"] -->|advisory-locked jobs| DB + Worker -->|purge on schedule| Store + + subgraph Request path + direction TB + R["router — HTTP only"] --> S["service — business logic"] + S --> T["state_machine.transition() — the only status writer"] + S --> Q["queries"] + end + API -.-> R +``` -| Layer | Choice | -|-----------|-----------------------------------------------| -| Frontend | React 18, TypeScript, Vite, TanStack Query | -| Backend | FastAPI, SQLAlchemy 2, Alembic | -| Database | PostgreSQL 16 | -| Jobs | APScheduler worker process | -| Deploy | Docker → Fly.io / single VPS | -| CI/CD | GitHub Actions → GHCR | +**Backend layering** (`api/app/`) + +| Layer | Responsibility | +|--------------|----------------| +| `routers/` | HTTP only — validate input, call one service, serialize a schema. RBAC declared here via `require_roles(...)`. | +| `schemas/` | Pydantic v2 request/response models — no raw dicts cross the boundary. | +| `services/` | Business logic. Every status change goes through `state_machine.transition()` — enforced by a commit hook. | +| `models/` | SQLAlchemy 2.0 ORM (`Mapped[...]`), Alembic migrations. | +| `db.py` | Engine + `get_db` — one session per request, commit on success / rollback on error. | +| `deps.py` | Auth + RBAC dependencies. | + +**Domain rules worth a look** + +- **State machine** — loan files move `DRAFT → SUBMITTED → IN_REVIEW → + FUND_READY_TO_RELEASE → PURGED`; review tasks have a + `PENDING_MAKER ⇄ PENDING_CHECKER → COMPLETED` cycle with a rejection loop. One + function owns every transition; a hook blocks raw `.status =` writes. +- **Maker ≠ checker** — a reviewer can never approve their own task + (DB check constraint + service guard). +- **Optimistic locking** — task writes carry a `version`; a stale write is a + `409`, the client refetches and retries. +- **Audit** — every transition appends one immutable `task_events` row. +- **Housekeeping** — the purge job is idempotent and wrapped in + `pg_try_advisory_lock` so replicas never double-run it. +- **Migrations** are expand/contract, so a code rollback never needs a DB rollback. + +**RBAC matrix** + +| Role | Team | Can | +|--------------|------|-----| +| `OPS_MAKER` | OPS | create / submit loan files, upload documents | +| `OPS_CHECKER`| OPS | review the intake step | +| `UW_MAKER` | UW | perform a review task (maker side) | +| `UW_CHECKER` | UW | approve / reject a review task (checker side) | +| `ADMIN` | — | reassign tasks, trigger housekeeping, read everything | + +--- + +## Roadmap progress + +One phase per PR, tests green before the next. Full detail in +[`docs/ROADMAP.md`](docs/ROADMAP.md); current state in +[`docs/STATUS.md`](docs/STATUS.md). + +| Phase | Scope | State | +|-------|-------|-------| +| **P0** | Monorepo, `docker-compose`, `/health` API + Vite app, worker skeleton, CI, `.claude/` workflow config | ✅ done | +| **P1** | Identity: `users` + `refresh_tokens`, Argon2, JWT login / rotating refresh / logout / me, `Role` enum + `require_roles(...)`, 35 tests | ✅ done · [PR #1](https://github.com/sujitd7/loanflow/pull/1), CI green | +| **P2** | Loan-file intake + atomic 4-task generation, document upload, list/detail with no N+1 | ▶ next | +| **P3** | Maker–checker flow, `state_machine.transition()`, `version` conflicts, audit events | ☐ | +| **P4** | Completion + housekeeping purge job (`freezegun` tests, advisory lock) | ☐ | +| **P5** | React foundation + core flows — auth context, Axios refresh interceptor, TanStack Query, submission wizard, My Tasks, review drawer, MSW tests | ☐ | +| **P6** | Dashboards — aggregation endpoints, Recharts funnel / aging / per-member | ☐ | +| **P7** | Hardening — Playwright E2E, rate limiting, JSON logging, deterministic seed data, `/security-review` | ☐ | +| **P8** | Deploy — multi-stage Dockerfiles, GHCR, release migrations, smoke test, rollback | ☐ | +| **P9** | Showcase — hero GIF, live demo, architecture write-up, Loom | ☐ | + +**Highlight from P1:** the `security-reviewer` subagent caught a real HIGH-severity +bug before merge — refresh-token reuse-detection was writing the revocation into +the request-scoped session, which then got rolled back by the 401 it raised, so +in production a stolen refresh token was effectively unrevocable. Fixed by +committing that side effect in its own unit of work, with a regression test using +real per-request sessions. (Details in [PR #1](https://github.com/sujitd7/loanflow/pull/1).) + +--- + +## Built with a guard-railed Claude Code workflow + +This repo is also a worked example of **AI-assisted development with real +guard-rails** — not "pasted from a chatbot". Everything below is committed under +[`.claude/`](.claude/) and [`scripts/hooks/`](scripts/hooks/); the full write-up +is [`docs/ai-workflow.md`](docs/ai-workflow.md). + +- **`CLAUDE.md`** — a project brief (stack, conventions, the "never write + `.status` directly" rule, the RBAC matrix) loaded into every session, so the + assistant works to *this* codebase's standards. +- **Hooks** — deterministic checks the harness runs, not the model: + auto-format/lint on every write, **block dangerous shell commands** + (`rm -rf`, unforced force-push, prod `psql`, `fly deploy`), flag raw status + writes, and optionally run the test suite on stop so a task can't "finish" red. +- **Subagents** (`.claude/agents/`) — seven focused roles used per feature: + `api-designer`, `db-migrator`, `workflow-modeler`, `security-reviewer`, + `frontend-builder`, `test-writer`, `pr-writer`. +- **Skills** (`.claude/skills/`) — repo-specific recipes: `add-endpoint`, + `add-migration`, `add-scheduled-job`, `add-page`, `seed-demo-data`, `deploy`. +- **Roadmap-driven loop** — `/loop` works `docs/ROADMAP.md` one checkbox at a + time: write the tests first, make them pass, run the suite, tick the box, + commit. + +**Delegated:** boilerplate routers/schemas/fixtures/migrations, test scaffolding, +formatting, PR text. **Kept by me:** the domain model and state machine, the RBAC +design, architectural calls (ADRs), migration discipline, and every review +decision. + +--- ## Run it locally -With Docker Desktop: +With Docker: ```bash cp .env.example .env docker compose up --build ``` -**No working Docker?** See [`docs/LOCAL_DEV.md`](docs/LOCAL_DEV.md) — the stack runs -natively with a venv + `npm run dev` against any Postgres (a free Neon database -works). Tests fall back to SQLite with no setup. +**No working Docker?** See [`docs/LOCAL_DEV.md`](docs/LOCAL_DEV.md) — the stack +runs natively with a Python venv + `npm run dev` against any Postgres (a free +Neon database works). The backend test suite falls back to in-memory SQLite with +zero setup. + +| Service | URL | +|---------------|------------------------------| +| Web (Vite) | http://localhost:5173 | +| API (FastAPI) | http://localhost:8000 | +| API docs | http://localhost:8000/docs | +| API health | http://localhost:8000/health | +| Postgres | localhost:5432 | + +## Development -| Service | URL | -|------------------|------------------------------| -| Web (Vite) | http://localhost:5173 | -| API (FastAPI) | http://localhost:8000 | -| API docs | http://localhost:8000/docs | -| API health | http://localhost:8000/health | -| Postgres | localhost:5432 | +```bash +make help # list tasks +make up / make down +make test # api (pytest) + web (vitest) +make fmt / make lint +make migrate m="add loan_files" +``` ## Repo layout ``` -api/ FastAPI service + Alembic migrations + pytest +api/ FastAPI service + SQLAlchemy models + Alembic migrations + pytest worker/ APScheduler job runner (housekeeping / purge) -web/ React + TypeScript SPA (Vite) -infra/ deployment config (added in P8) -docs/ roadmap, architecture, ADRs, AI-workflow writeup -.claude/ committed Claude Code config: hooks, subagents, skills +web/ React + TypeScript SPA (Vite) — foundation lands in P5 +infra/ deployment config — added in P8 +docs/ roadmap, architecture, state machine, ADRs, AI-workflow write-up +.claude/ committed Claude Code config: CLAUDE.md pointer, hooks, subagents, skills scripts/ hook scripts and helpers ``` -## Development - -```bash -make help # list tasks -make up / make down -make test # api + web tests -make fmt / make lint -make migrate m="add users table" -``` - -## Status +## Tech stack -Phase **P0 — Foundation & tooling**. See [`docs/STATUS.md`](docs/STATUS.md). +| Layer | Choice | +|-----------|--------| +| Frontend | React 18, TypeScript, Vite, TanStack Query, react-hook-form + zod | +| Backend | FastAPI, SQLAlchemy 2.0, Alembic, Pydantic v2, psycopg 3, Argon2 + JWT | +| Database | PostgreSQL 16 | +| Jobs | APScheduler in a dedicated worker process ([ADR 0002](docs/adr/0002-apscheduler-over-celery.md)) | +| Deploy | Docker images → Fly.io / single VPS (planned, P8) | +| CI/CD | GitHub Actions → GHCR | diff --git a/docs/STATUS.md b/docs/STATUS.md index 965241d..ee4813c 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -1,6 +1,6 @@ # Status -**Phase:** P1 — Identity, roles, RBAC (complete, in PR) +**Phase:** P1 — Identity, roles, RBAC (complete — PR #1, CI green, awaiting merge) ## Done - **P0** — monorepo scaffold, docker-compose, `/health` API + Vite app, worker @@ -14,8 +14,11 @@ - `app/errors.py` — `AppError` hierarchy + handler wired in `create_app()` - conftest: `make_user` factory, per-role user + `auth_*` header fixtures, test-only `/_probe/*` routes - - `tests/test_auth.py` — login ok/bad/inactive, expired + wrong-type tokens, - rotation + reuse, logout, per-role probe matrix (28 tests, green) + - `tests/test_auth.py` + `tests/test_config.py` — login ok/bad/inactive, + expired + wrong-type tokens, rotation + reuse, logout ownership, per-role + probe matrix, JWT-secret boot guard (35 tests, green on CI Postgres) + - `security-reviewer` pass drove the hardening (reuse-burn committed + independently, secret fail-fast, row-locked rotation, timing, claim checks) ## Deferred from P0 (infra, not blocking) - GitHub repo + branch protection on `main`. diff --git a/docs/ai-workflow.md b/docs/ai-workflow.md index c25cbe2..6e045fb 100644 --- a/docs/ai-workflow.md +++ b/docs/ai-workflow.md @@ -41,8 +41,16 @@ Typical feature flow for an endpoint: 5. `test-writer` fills edge cases. 6. `pr-writer` drafts the PR. -**Evidence:** _link a PR where `security-reviewer` caught a real object-level -access bug, with its output quoted._ +**Evidence — P1 ([PR #1](https://github.com/sujitd7/loanflow/pull/1)):** +`security-reviewer` flagged a HIGH-severity bug before merge. Refresh-token +reuse-detection called `_revoke_all_for_user(...)` and then raised `Unauthorized`; +because that write lived in the request-scoped session, `get_db`'s +rollback-on-exception undid it, so in production a stolen refresh token stayed +usable forever — the tests passed only because the test `get_db` override never +commits or rolls back. Fixed by committing the revocation in its own unit of +work, plus a regression test (`test_refresh_reuse_burn_is_committed_before_the_401`) +that uses real per-request sessions. The same pass also drove a JWT-secret +fail-fast, row-locked rotation, and required-claim checks. ## The agentic loop From 969903754bdef737319267c1e4b0eb43b5491a2a Mon Sep 17 00:00:00 2001 From: "sujit.deshmukh" Date: Fri, 28 Aug 2026 21:25:42 +0530 Subject: [PATCH 3/3] docs: README accuracy fixes (repo layout, hook wording) Co-Authored-By: Claude Sonnet 5 --- README.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 8177fca..0886bd7 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ flowchart LR Worker["worker — APScheduler"] -->|advisory-locked jobs| DB Worker -->|purge on schedule| Store - subgraph Request path + subgraph reqpath ["Inside the API: router → service → state machine"] direction TB R["router — HTTP only"] --> S["service — business logic"] S --> T["state_machine.transition() — the only status writer"] @@ -55,7 +55,7 @@ flowchart LR |--------------|----------------| | `routers/` | HTTP only — validate input, call one service, serialize a schema. RBAC declared here via `require_roles(...)`. | | `schemas/` | Pydantic v2 request/response models — no raw dicts cross the boundary. | -| `services/` | Business logic. Every status change goes through `state_machine.transition()` — enforced by a commit hook. | +| `services/` | Business logic. Every status change goes through `state_machine.transition()`; a hook flags raw `.status =` writes. | | `models/` | SQLAlchemy 2.0 ORM (`Mapped[...]`), Alembic migrations. | | `db.py` | Engine + `get_db` — one session per request, commit on success / rollback on error. | | `deps.py` | Auth + RBAC dependencies. | @@ -180,13 +180,14 @@ make migrate m="add loan_files" ## Repo layout ``` -api/ FastAPI service + SQLAlchemy models + Alembic migrations + pytest -worker/ APScheduler job runner (housekeeping / purge) -web/ React + TypeScript SPA (Vite) — foundation lands in P5 -infra/ deployment config — added in P8 -docs/ roadmap, architecture, state machine, ADRs, AI-workflow write-up -.claude/ committed Claude Code config: CLAUDE.md pointer, hooks, subagents, skills -scripts/ hook scripts and helpers +CLAUDE.md project brief loaded into every Claude Code session +api/ FastAPI service + SQLAlchemy models + Alembic migrations + pytest +worker/ APScheduler job runner (housekeeping / purge) +web/ React + TypeScript SPA (Vite) — foundation lands in P5 +infra/ deployment config — added in P8 +docs/ roadmap, architecture, state machine, ADRs, AI-workflow write-up +.claude/ committed Claude Code config: hooks, subagents, skills +scripts/ hook scripts and helpers ``` ## Tech stack