From cd159ce2079853f4f9e6f7a8c6a4b5c652268682 Mon Sep 17 00:00:00 2001 From: Brian Obot Date: Mon, 13 Jul 2026 23:53:46 +0100 Subject: [PATCH 1/4] Improve on security --- .coverage | Bin 53248 -> 53248 bytes .github/workflows/audit.yml | 14 +-- .github/workflows/create_pull_request.yml | 33 ++++++- Makefile | 6 ++ README.md | 4 +- UPGRADING.md | 100 ++++++++++++++++++++++ app/main.py | 7 +- app/middlewares.py | 18 ++++ app/routers/tests/test_auth.py | 19 ++++ app/services/auth.py | 66 +++++++++++--- app/services/tests/test_auth.py | 24 ++++++ app/tests/test_main.py | 7 ++ 12 files changed, 274 insertions(+), 24 deletions(-) diff --git a/.coverage b/.coverage index b3c756866b9a6002b775b8286b3a495b1257bb21..d26a5b1e9c3104377d75f8fd53ae56f2df4d2e7e 100644 GIT binary patch delta 1017 zcmXBSe`phD7zgmYy~}$)ntPwSBu$#+nxtuBHj!G1Lo-SI^G4pkzUfBJuwRyw=Gw=}pfHAlMgK!?sKrbAHZpgxVcn*SKuqSMm z{lg0E8oR{KvOMcx8McS*Vo4TZnzo|NX!o?=wJ~kvlA(X7S#m-oiaU{AU^LDXixa7i ztJjrsT8sxX_UN6(fBX9$|E<^ZGFH3#WM5(5so!2`U6x%@-U&vV7ruF~cVd$sv9Cvl zl8?UHRnE@bZ3@va@31ggU&Gy7Q+(Doo*vv;-?}h-DmQjMH=QaZkGC8(tlLLM-YlKm zG2SLn(>`GmImC5`dVfyQpxp_QWPtltpqyJSm)~gI(4NXH&Myq?TYF&czz1{Xi&Iyp zzG!+dCk}&B%?G{J1G!<7`t7rRqWJ8VZ)nevV^GCov}$Ogux7UX#OWU5^NJQx>b;BC zHcIJRud=pV_gg+mPpuKD$3E;KlAEi$R};U%LxWMi4*k~7$4_teNaI_U??;M)q}vm^ zZ=(x%2tszr61zJkmGR2QTIO0CwO2GJQ9g2L#mUU!b*XJ1ukG1dy7|)9;?wx#vpd#_ z`H4R>$<+Rm=f&%1zHI$*FugR|H9Ng&_Y-+}OULp3=HMcphR90xxJFP`1LC z@f@@4_alh}5|1Nwbx16R)Yc-=C=!Vv;V@EDgP0}~3L(KD5(psG)rjAZ_^OZIs2M@yA^$~MCe^Y78MmyABv(<=tTsTMr+x+V-yiZ zk}QQmr9=-349uXiQ1C;s6mC%vy@P5aF^v^0?O}J$r{DSi?`38mr>WiC)NVcHK>4Q(D|zL4m&1No(H#jvk|qr`x*M`tU&WK$zDf{F1tQtwg_IKKgBBau zI{xii+mAO8<1rGQdZz7V+4WsLe;jg@7yihGu`4^yzFK0B@DvMoPae3qX*Bu$_2K{x z^M-|pEyPtV+h5bxh2=p$Zw(Ipt?QqCZ(~_zMsrYKUO2O^zs^iGn32&h?}xR~#(3=JZ~^P~CTL_U~-taDGY6ujB7l=F8uvg|F0a zJVPc{AJ-W;`>29?jmA%yhZ`y`D|aN0MufXSRin2#6D0y4R%9BReN$3hOH&t{eiF&e zE4TM%>VZaG#x)Orm{F(Tto)0Us{{Kb^Cikuri*UiR2igig8{_ ztg_LF8#<{}%qO=s`S1g!Hsg@mWQpr9>3)^dw&k~|R-VcnqrLk0BTH`8p93|?0!-`; z%c5}$lD~B!-5}a1Pb$+lk5n&EL<=-{i=3Ok?%mct^xTuh=@!07Bw>6w5-&w!aU>c;B2gq9K|*1~5<-GOMAHx@ zfXIF%;6wagq}YSlRK!<;c->Uc`FlMsjH(lHI}n#0kpM|Bq*Osf8L<{46n|P$B4Kh^ c5fBkZ5xWI33y5GwoJIU!FiTR=rS|Fn0aokF9RL6T diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index 8f00fed..0e05d34 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -23,14 +23,14 @@ jobs: if [ -f requirements.txt ]; then pip install -r requirements.txt; fi # If using Poetry, add those steps here instead - - name: Scan with pip-audit + - name: Scan dependencies with pip-audit run: | pip install pip-audit - # This will fail the build if a vulnerability is found - pip-audit + # Fails the build on a vulnerable dependency + pip-audit -r requirements.txt - - name: Scan with Safety + - name: Scan source with Bandit (SAST) run: | - pip install safety - # Scans the current environment - safety check + pip install bandit + # Static analysis of app code; fail on medium+ findings, skip tests + bandit -r app -x app/models/tests,app/routers/tests,app/services/tests,app/schemas/tests,app/tests --severity-level medium diff --git a/.github/workflows/create_pull_request.yml b/.github/workflows/create_pull_request.yml index 0c9cbd4..a589cdd 100644 --- a/.github/workflows/create_pull_request.yml +++ b/.github/workflows/create_pull_request.yml @@ -61,6 +61,7 @@ jobs: run: | mkdir logs coverage run -m pytest + coverage report --fail-under=90 PERCENT=$(coverage report | grep TOTAL | awk '{print $NF}' | sed 's/%//') echo "PERCENTAGE=$PERCENT" >> $GITHUB_OUTPUT @@ -76,9 +77,39 @@ jobs: maxColorRange: 100 minColorRange: 0 + lint: + runs-on: ubuntu-latest + steps: + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + cache: 'pip' + + - name: Install tools + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + pip install ruff black isort + + - name: Ruff + run: ruff check app conftest.py + + - name: Black + run: black --check app conftest.py + + - name: isort + run: isort --profile black --check-only app conftest.py + + - name: Mypy + run: mypy app + create_pull_request: runs-on: ubuntu-latest - needs: test # This creates the dependency link + needs: [test, lint] # gate the auto-PR on tests AND lint/type checks if: github.actor == 'brianobot' permissions: contents: write diff --git a/Makefile b/Makefile index 175da52..78711ab 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,12 @@ run-local: fastapi dev app/main.py +# Production: --proxy-headers makes rate limiting and logging see the real +# client IP behind a load balancer. Restrict --forwarded-allow-ips to your +# proxy's address(es) instead of "*" in a real deployment. +run-prod: + uvicorn app.main:app --host 0.0.0.0 --port 8000 --proxy-headers --forwarded-allow-ips="*" + test-local: pytest -s --cov diff --git a/README.md b/README.md index 7a1e850..59dcb7a 100644 --- a/README.md +++ b/README.md @@ -143,7 +143,9 @@ Things that are easy to trip over when building on this template: - **Docs are gated by `DEBUG` OR the IP allowlist.** The [`AllowAuthorizedDocAccess`](./app/middlewares.py) middleware serves `/docs`, `/redoc`, and `/openapi.json` only when `settings.DEBUG` is true **or** the client IP is in `allowed_ips` (default `127.0.0.1`); otherwise it returns a 404 that hides their existence. Note this middleware runs **before** `TrustedHostMiddleware`, so a request that clears the docs gate must still use a host listed in `main.py`'s `allowed_hosts`. - **Access and refresh tokens are not interchangeable.** Each carries a `type` claim (`access` / `refresh`). `get_current_user` rejects anything that isn't an access token; the `refresh_token` endpoint rejects anything that isn't a refresh token. - **Refresh tokens are single-use (rotated).** Each call to `/refresh_token` blacklists the presented refresh token and returns a fresh access **and** refresh token, so a leaked refresh token is usable at most once. -- **Logout is global.** It blacklists the presented token(s) for their remaining lifetime *and* bumps a per-user token version in Redis, so **every** token issued before the logout is invalidated across all devices. Tokens carry a `ver` claim checked on each request; pass the refresh token in the logout body (`{"refresh_token": "..."}`) to blacklist it explicitly too. +- **Logout is global, and so is a password change.** Logout blacklists the presented token(s) *and* bumps a per-user token version in Redis, so **every** token issued before it is invalidated across all devices (tokens carry a `ver` claim checked on each request). A successful **password reset or change** bumps the same version — revoking all existing sessions, including the current one. Reusing an already-rotated refresh token is treated as theft and revokes the whole family. +- **Baseline security headers** are added to every response by `SecurityHeadersMiddleware` (`X-Content-Type-Options`, `X-Frame-Options`, `Referrer-Policy`, and HSTS outside `DEBUG`). No CSP is set, to avoid breaking Swagger UI. +- **Behind a proxy, run with forwarded headers** (`make run-prod` / `uvicorn --proxy-headers --forwarded-allow-ips=...`) — otherwise per-IP rate limiting and logging see the load balancer's IP, not the client's. - **Repeated bad codes lock the account.** After `MAX_CODE_ATTEMPTS` (default 5) wrong activation/reset codes, that account is locked for `CODE_LOCKOUT_SECONDS`; a successful attempt clears the counter. - **`/health` and startup checks.** `/health` returns 503 if the DB or Redis is unreachable. On boot the app pings both; in production (`DEBUG=False`) it refuses to start if either is down, in `DEBUG` it only logs. - **Sign-in requires a verified email.** `signin_user` returns `403 Email not verified` until activation flips `is_verified`. In tests, `UserFactory` builds verified users; use `create_user`/an unverified user to exercise the rejection. diff --git a/UPGRADING.md b/UPGRADING.md index 379f578..75400b9 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -28,6 +28,10 @@ independent, so apply them in any order you like and run your tests after each. | Global logout (token versioning) | **Breaking (tokens)** | Adds `ver` claim; see §11 | | Account lockout on codes | Behavioral | See §12 | | Rate limiting | **Optional** | Skip it — see §13 | +| Revoke sessions on password change + reuse detection | Behavioral | See §14 | +| Security response headers | Safe (additive) | See §15 | +| Proxy headers behind a load balancer | Deploy config | See §16 | +| CI hardening (SAST, lint gate, coverage floor) | Safe (additive) | See §17 | **Backup first:** commit or branch before starting, and snapshot your database before running the migration in §6. @@ -549,6 +553,102 @@ Redis-backed counter persists across runs within the window) to avoid flakiness. --- +## 14. Revoke sessions on password change + refresh-reuse detection + +Builds on the token versioning from §11. + +**Kill all sessions when the password changes.** A reset/change usually means the +old credentials are compromised, so bump the token version after a successful +`reset_password` (and after a password change in `update_user`): + +```python +async def invalidate_all_sessions(email: str) -> None: + await redis_manager.increment(token_version_key(email)) + +# reset_password(), after the password UPDATE commits: +await invalidate_all_sessions(reset_data.email) + +# update_user(), after the UPDATE commits, only when the password changed: +if new_password: + await invalidate_all_sessions(email) +``` + +> A password *change* logs the user out of their **current** session too (the +> access token used for the request is invalidated for subsequent calls). That's +> the secure default; have the client re-authenticate afterward. + +**Refresh-reuse detection.** A rotated refresh token should never come back. If a +blacklisted refresh token is presented again, treat it as theft and revoke the +whole family: + +```python +if await redis_manager.get_json_item(token_data.refresh_token): + try: + stale = jwt.decode(token_data.refresh_token, JWT_SECRET, + algorithms=[JWT_ALGORITHM], options={"verify_exp": False}) + if isinstance(stale.get("sub"), str): + await invalidate_all_sessions(stale["sub"]) + except InvalidTokenError: + pass + raise HTTPException(status_code=401, detail="Invalid Refresh Token") +``` + +**Readability tip (optional):** while you're here, move the Redis key formats into +builder functions (`activation_code_key`, `reset_code_key`, `failed_attempts_key`, +`token_version_key`) so formats and TTLs live in one place. + +--- + +## 15. Security response headers + +Add a middleware that stamps hardening headers on every response: + +```python +class SecurityHeadersMiddleware(BaseHTTPMiddleware): + async def dispatch(self, request, call_next): + response = await call_next(request) + response.headers["X-Content-Type-Options"] = "nosniff" + response.headers["X-Frame-Options"] = "DENY" + response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin" + if not settings.DEBUG: # HSTS only over HTTPS + response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains" + return response + +app.add_middleware(SecurityHeadersMiddleware) +``` + +> A strict `Content-Security-Policy` is intentionally omitted — it breaks the +> Swagger UI (CDN + inline scripts). Add one scoped to your own frontend if needed. + +--- + +## 16. Proxy headers behind a load balancer + +Per-IP rate limiting and request logging read `request.client.host`. Behind a +proxy that's the **proxy's** IP unless you enable forwarded headers — so run: + +```bash +uvicorn app.main:app --proxy-headers --forwarded-allow-ips="" +``` + +Restrict `--forwarded-allow-ips` to your proxy's address(es); `"*"` trusts any +client's `X-Forwarded-For` and is spoofable if you're not actually behind a proxy. + +--- + +## 17. CI hardening (optional) + +- **Replace the dead `safety check`** (it now needs an account) with SAST: + `pip install bandit && bandit -r app -x --severity-level medium`. +- **Add a lint/type gate** — run `ruff`, `black --check`, `isort --check-only`, + and `mypy app` as a CI job, not just in pre-commit. +- **Floor your coverage**: `coverage report --fail-under=90` so it can't silently + regress. +- **Bump CI Python to 3.12+** if you adopted the `PaginatedResponse[T]` generic + (PEP 695), and pin pre-commit `mypy` to ≥ 1.12. + +--- + ## Verify After applying the steps you want: diff --git a/app/main.py b/app/main.py index 67b588e..46c16c5 100644 --- a/app/main.py +++ b/app/main.py @@ -16,7 +16,11 @@ from app.database import AsyncSessionLocal from app.limiter import limiter from app.logger import logger -from app.middlewares import AllowAuthorizedDocAccess, log_request_middleware +from app.middlewares import ( + AllowAuthorizedDocAccess, + SecurityHeadersMiddleware, + log_request_middleware, +) from app.redis_manager import redis_manager from app.routers.health import router as health_router from app.settings import settings @@ -83,6 +87,7 @@ def initiate_app(): ], ) app.add_middleware(AllowAuthorizedDocAccess) + app.add_middleware(SecurityHeadersMiddleware) app.add_middleware(BaseHTTPMiddleware, dispatch=log_request_middleware) # Enforce the rate limits declared via @limiter.limit(...) on the routes. diff --git a/app/middlewares.py b/app/middlewares.py index 356716f..04de094 100644 --- a/app/middlewares.py +++ b/app/middlewares.py @@ -23,6 +23,24 @@ async def log_request_middleware(request: Request, call_next): return response +class SecurityHeadersMiddleware(BaseHTTPMiddleware): + """Add baseline hardening headers to every response.""" + + async def dispatch( + self, request: Request, call_next: RequestResponseEndpoint + ) -> Response: + response = await call_next(request) + response.headers["X-Content-Type-Options"] = "nosniff" + response.headers["X-Frame-Options"] = "DENY" + response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin" + # HSTS only makes sense over HTTPS; enable it outside local/dev. + if not settings.DEBUG: + response.headers[ + "Strict-Transport-Security" + ] = "max-age=31536000; includeSubDomains" + return response + + class AllowAuthorizedDocAccess(BaseHTTPMiddleware): allowed_ips = [ "127.0.0.1", # allows Viewing Docs in Local Development Environment diff --git a/app/routers/tests/test_auth.py b/app/routers/tests/test_auth.py index 4122b73..b37efac 100644 --- a/app/routers/tests/test_auth.py +++ b/app/routers/tests/test_auth.py @@ -191,6 +191,25 @@ async def test_logout_with_refresh_token_revokes_it(client: AsyncClient, user: U assert refreshed.status_code == 401 +async def test_password_reset_revokes_existing_sessions( + client: AsyncClient, user: UserDB +): + login = {"username": user.email, "password": "password"} + access = (await client.post("/v1/auth/token", data=login)).json()["access_token"] + header = {"Authorization": f"Bearer {access}"} + assert (await client.get("/v1/auth/me", headers=header)).status_code == 200 + + await redis_manager.cache_json_item(f"reset-code-{user.email}", {"code": "000000"}) + reset = await client.post( + "/v1/auth/reset_password", + json={"code": "000000", "email": user.email, "new_password": "brandnewpass"}, + ) + assert reset.status_code == 200 + + # The pre-reset access token must no longer authenticate. + assert (await client.get("/v1/auth/me", headers=header)).status_code == 401 + + async def test_logout_invalidates_all_user_tokens(client: AsyncClient, user: UserDB): login = {"username": user.email, "password": "password"} first = (await client.post("/v1/auth/token", data=login)).json()["access_token"] diff --git a/app/services/auth.py b/app/services/auth.py index 76a42ef..5372598 100644 --- a/app/services/auth.py +++ b/app/services/auth.py @@ -26,23 +26,41 @@ oauth2_scheme = OAuth2PasswordBearer(tokenUrl="v1/auth/token") -# Per-user token version. Bumped on logout to invalidate every token issued -# before it (global logout). Failure counters gate code brute-forcing. +# Per-user token version. Bumped on logout / password change to invalidate +# every token issued before it. Failure counters gate code brute-forcing. MAX_CODE_ATTEMPTS = 5 CODE_LOCKOUT_SECONDS = 15 * 60 +# --- Redis key builders (single source of truth for key formats) ------------ def token_version_key(email: str) -> str: return f"token-version-{email}" +def activation_code_key(email: str) -> str: + return f"activation-code-{email}" + + +def reset_code_key(email: str) -> str: + return f"reset-code-{email}" + + +def failed_attempts_key(scope: str, email: str) -> str: + return f"failed-{scope}-{email}" + + +async def invalidate_all_sessions(email: str) -> None: + """Bump the user's token version so every existing token is rejected.""" + await redis_manager.increment(token_version_key(email)) + + async def guard_code_attempts(scope: str, email: str) -> str: """ Throttle brute-forcing of the 6-digit codes: after MAX_CODE_ATTEMPTS wrong submissions for (scope, email), lock the account out for CODE_LOCKOUT_SECONDS. Returns the counter key so the caller can register a failure or clear it. """ - key = f"failed-{scope}-{email}" + key = failed_attempts_key(scope, email) if await redis_manager.get_int(key) >= MAX_CODE_ATTEMPTS: raise HTTPException( status_code=status.HTTP_429_TOO_MANY_REQUESTS, @@ -106,7 +124,7 @@ async def initiate_password_reset( code = generate_random_code(6) await redis_manager.cache_json_item( - f"reset-code-{email}", {"code": code}, ttl=60 * 30 + reset_code_key(email), {"code": code}, ttl=60 * 30 ) background_task.add_task( @@ -124,7 +142,7 @@ async def reset_password( reset_data: auth_schema.PasswordResetData, session: AsyncSession ): attempt_key = await guard_code_attempts("reset", reset_data.email) - data = await redis_manager.get_json_item(f"reset-code-{reset_data.email}") + data = await redis_manager.get_json_item(reset_code_key(reset_data.email)) if not data or data.get("code") != reset_data.code: await redis_manager.increment(attempt_key, ttl=CODE_LOCKOUT_SECONDS) raise HTTPException(status_code=400, detail="Invalid Reset Code") @@ -145,8 +163,11 @@ async def reset_password( await session.commit() # Consume the code and clear the failure counter on success. - await redis_manager.delete_key(f"reset-code-{reset_data.email}") + await redis_manager.delete_key(reset_code_key(reset_data.email)) await redis_manager.delete_key(attempt_key) + # A password reset must revoke every existing session (the point of a reset + # is often that the old credentials/tokens are compromised). + await invalidate_all_sessions(reset_data.email) return {"detail": "Password Reset Successfully"} @@ -183,12 +204,15 @@ async def update_user( ) result = await session.execute(stmt) await session.commit() + # Changing the password revokes existing sessions on other devices. + if new_password: + await invalidate_all_sessions(email) return result.scalar_one() def create_access_token( data: dict[str, str | int | datetime], expires_delta: timedelta | None = None -): +) -> str: to_encode = data.copy() expire = datetime.now(UTC) + (expires_delta or ACCESS_TOKEN_LIFESPAN) # jti makes every token unique (so distinct logins never collide); type and @@ -199,7 +223,7 @@ def create_access_token( def create_refresh_token( data: dict[str, str | int | datetime], expires_delta: timedelta | None = None -): +) -> str: to_encode = data.copy() expire = datetime.now(UTC) + (expires_delta or REFRESH_TOKEN_LIFESPAN) to_encode.update({"exp": expire, "type": "refresh", "jti": secrets.token_hex(16)}) @@ -226,7 +250,7 @@ async def signup_user( code = generate_random_code(6) await redis_manager.cache_json_item( - f"activation-code-{data.email}", {"code": code}, ttl=60 * 30 + activation_code_key(data.email), {"code": code}, ttl=60 * 30 ) bg_task.add_task( @@ -249,7 +273,7 @@ async def resend_activation_code( code = generate_random_code(6) await redis_manager.cache_json_item( - f"activation-code-{email}", {"code": code}, ttl=60 * 30 + activation_code_key(email), {"code": code}, ttl=60 * 30 ) bg_task.add_task( @@ -269,7 +293,7 @@ async def activate_user( ): attempt_key = await guard_code_attempts("activation", verification_data.email) data = await redis_manager.get_json_item( - f"activation-code-{verification_data.email}" + activation_code_key(verification_data.email) ) if not data or data.get("code") != verification_data.code: @@ -285,7 +309,7 @@ async def activate_user( await session.commit() # Consume the code and clear the failure counter on success. - await redis_manager.delete_key(f"activation-code-{verification_data.email}") + await redis_manager.delete_key(activation_code_key(verification_data.email)) await redis_manager.delete_key(attempt_key) bg_task.add_task( @@ -355,8 +379,22 @@ async def blacklist_token(token: str) -> None: async def refresh_token( token_data: auth_schema.RefreshTokenModel, session: AsyncSession ): - # Reject tokens blacklisted at logout or by a previous rotation. + # A refresh token already blacklisted (by logout or a prior rotation) but + # presented again is a reuse signal - a rotated token should never come back. + # Treat it as possible theft and kill the whole session family. if await redis_manager.get_json_item(token_data.refresh_token): + try: + stale = jwt.decode( + token_data.refresh_token, + JWT_SECRET, + algorithms=[JWT_ALGORITHM], + options={"verify_exp": False}, + ) + stale_sub = stale.get("sub") + if isinstance(stale_sub, str): + await invalidate_all_sessions(stale_sub) + except InvalidTokenError: + pass raise HTTPException(status_code=401, detail="Invalid Refresh Token") try: @@ -419,5 +457,5 @@ async def logout( # Bump the user's token version so EVERY token issued before now (across all # devices) is invalidated, not just the pair presented here. if email: - await redis_manager.increment(token_version_key(email)) + await invalidate_all_sessions(email) return {"detail": "User Logged Out Successfully"} diff --git a/app/services/tests/test_auth.py b/app/services/tests/test_auth.py index 2a9e56b..d568278 100644 --- a/app/services/tests/test_auth.py +++ b/app/services/tests/test_auth.py @@ -299,6 +299,30 @@ async def test_refresh_token_payload_return_none(session: AsyncSession): assert err.value.detail == "Invalid Refresh Token" +async def test_refresh_reuse_kills_session_family(user: UserDB, session: AsyncSession): + initial = auth_services.create_refresh_token( + {"sub": user.email, "ver": 0}, auth_services.REFRESH_TOKEN_LIFESPAN + ) + rotated = await auth_services.refresh_token( + auth_schemas.RefreshTokenModel(refresh_token=initial), session + ) + + # Replaying the old (rotated) token is a reuse signal -> escalate. + with pytest.raises(HTTPException): + await auth_services.refresh_token( + auth_schemas.RefreshTokenModel(refresh_token=initial), session + ) + + # The escalation bumped the token version, so even the freshly-issued + # refresh token from the first rotation is now rejected. + with pytest.raises(HTTPException) as err: + await auth_services.refresh_token( + auth_schemas.RefreshTokenModel(refresh_token=rotated.refresh_token), + session, + ) + assert err.value.status_code == 401 + + async def test_reset_password_locks_out_after_max_attempts( user: UserDB, session: AsyncSession ): diff --git a/app/tests/test_main.py b/app/tests/test_main.py index f342443..02d7321 100644 --- a/app/tests/test_main.py +++ b/app/tests/test_main.py @@ -20,6 +20,13 @@ async def test_health_endpoint_ok(client): assert body["checks"] == {"database": "ok", "redis": "ok"} +async def test_security_headers_present(client): + response = await client.get("/health") + assert response.headers["x-content-type-options"] == "nosniff" + assert response.headers["x-frame-options"] == "DENY" + assert "referrer-policy" in response.headers + + async def test_http_exception_handler(): async with AsyncClient( transport=ASGITransport(app=app), base_url="http://test" From d15f70913288ba0fd6220778e34dc1214840ded2 Mon Sep 17 00:00:00 2001 From: Brian Obot Date: Tue, 14 Jul 2026 00:26:58 +0100 Subject: [PATCH 2/4] Harden security --- .coverage | Bin 53248 -> 53248 bytes .github/dependabot.yml | 14 +++++ .github/workflows/audit.yml | 3 + Makefile | 8 ++- README.md | 4 +- UPGRADING.md | 98 ++++++++++++++++++++++++++++++++ app/limiter.py | 1 + app/main.py | 7 ++- app/middlewares.py | 23 ++++++++ app/routers/auth.py | 4 +- app/routers/tests/test_auth.py | 16 +++++- app/services/auth.py | 45 ++++++++++++++- app/services/tests/test_auth.py | 24 +++++++- app/settings.py | 25 ++++++-- app/tests/test_main.py | 32 +++++++++++ 15 files changed, 285 insertions(+), 19 deletions(-) create mode 100644 .github/dependabot.yml diff --git a/.coverage b/.coverage index d26a5b1e9c3104377d75f8fd53ae56f2df4d2e7e..1aed8ead50052c95eacb56f6a75760dfb5db611f 100644 GIT binary patch delta 1046 zcmW;Ke`phD7zgmY$>qIVdiOq;wOi3CO#6o`W2{i{55eM$I;TUy;>4&m z^(qR}jcwM74(663OktqXR&i~kvI*9hv>DrAXyPwyIuxz08J(%C^}YA6&+~lW_uluo zd%Ai}UA?BGJ|uLjE0NH*{*2Z#V1SqK946o|7=vHoB76sjp$m4wHuxCUz=yCB7C;c( zAdyF;K(gdI86U|ONqOmpG%#S3H%OW-#tFi_$xfvslkqg>@;jS= zk8((mBU4E!LJKwgWYf&!v#A#aB~078P+MJ7s&mcU$hMPzE{?yph#{JKgKd=`x16~B zz8s`oI51TET6OqM3wfjztG-9`&coA_?{Z;jE*UP>_snshpSC{hRL4MiI(C7O+6GsPREbDt6NCc`^SiQgh;n3^4*hhCjeC z`x!Mg3{_=FK8EOJ_&f}+n^EOrSe*<{HN&l7PNVm_9ISQP8H$bJkQo9nVuTTu7`(`^ mR5397vZz{Moz21ko8_zg>!HUI^iI+LmHODLI{GJy=4>Z zAC_fT*+up%%dlpaV%ykeR?Z?!)8@4??YZ_yd!P+mbn9PhJLH5&6i*^O&1jq_7AH~} zS1-%8YB3(n*z3<{UUhf9`BN|DWvq13v99dSAO3j1aZYwcc_$dHoBrjC&f(R1#6FMo zmB0RJb3Q%xtR_Umyu-p|c?oyNk&1(cb+R_UX|}Vnx4!z-P=9^H^LqydKFb~3IJ8Tk zraf#DImGpZI&UXw(C!3Dd4RhvkZ+yK=Rd8kY)YnPrlxy#F5NS^=c~#5h0!acCu{zi z6#GFb=7YZCp4NVo`t7@Zq8N6|=-YPSFcgWzQldqD!`a0XO-E035W~k0m-uF`u8^wk ze8hI$dC~A))#zf8dhN?zB6+yFXA$wcd8lybuR*s}|K{Dz4ryrZ+>6M#AnEpmZme(t z4?#%JT4H;Pq%vL^tYNaTT3e+#iSms@8y`vSUuGrOf4j7!Hh1%b+VOYsk$>+Wof>|U zDo^gtc~@RL^<(3W-m2NbwuzUkx4f0-);1s6ZC;98uc9vgIOZa(kQ**svdo>>7Zoo9 zRqej&aVgxOvZ4LLZ(})8k$L5l)3-*bEb-`(>dsS2?NMr}s*?x9x&0YE{;?$Ttcz!e zCh%YiEnVJh^QHMTEwG0Rh?|ItvS7T;*D^R5NZ!vjeF&<<;YhG~2h740U&WU&0#D%~ z+=T(?hu@(Gx*-E6;Ct8)`=E`lWeUE8txyN+VGY!Pw;C#;0^Wldgun+HP&Uu^@GP_J z?<0u>5|1NgWk@WBl$IjVC=!Vv;V@ECf|w=}3L(KD5(ps0#faaJ7zR>Qg!p{avS07@ zV(IZ9Za1Rq2mrzuqG^cBg%lPds){HIBFl&*A^-mX;&fsqiijW}1qGrgD2noPrs6+u C>Fr?v diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..9e1bf1a --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,14 @@ +version: 2 +updates: + # Keep Python dependencies patched (surfaces vulnerable pins as PRs). + - package-ecosystem: "pip" + directory: "/" + schedule: + interval: "weekly" + open-pull-requests-limit: 10 + + # Keep the GitHub Actions themselves up to date. + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index 0e05d34..a83f1e8 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -3,6 +3,9 @@ name: Dependency Security Audit on: push: branches: [ "dev" ] + schedule: + # Weekly scan catches newly-disclosed CVEs in already-pinned deps. + - cron: "0 6 * * 1" jobs: security-scan: diff --git a/Makefile b/Makefile index 78711ab..a1bb545 100644 --- a/Makefile +++ b/Makefile @@ -4,10 +4,12 @@ run-local: fastapi dev app/main.py # Production: --proxy-headers makes rate limiting and logging see the real -# client IP behind a load balancer. Restrict --forwarded-allow-ips to your -# proxy's address(es) instead of "*" in a real deployment. +# client IP. --forwarded-allow-ips must list ONLY your proxy's address(es) - +# never "*", which lets any client spoof X-Forwarded-For and bypass rate +# limiting. Default below trusts a co-located (loopback) proxy; change it to +# your proxy's real IP/CIDR. run-prod: - uvicorn app.main:app --host 0.0.0.0 --port 8000 --proxy-headers --forwarded-allow-ips="*" + uvicorn app.main:app --host 0.0.0.0 --port 8000 --proxy-headers --forwarded-allow-ips="127.0.0.1" test-local: pytest -s --cov diff --git a/README.md b/README.md index 59dcb7a..da96085 100644 --- a/README.md +++ b/README.md @@ -145,7 +145,9 @@ Things that are easy to trip over when building on this template: - **Refresh tokens are single-use (rotated).** Each call to `/refresh_token` blacklists the presented refresh token and returns a fresh access **and** refresh token, so a leaked refresh token is usable at most once. - **Logout is global, and so is a password change.** Logout blacklists the presented token(s) *and* bumps a per-user token version in Redis, so **every** token issued before it is invalidated across all devices (tokens carry a `ver` claim checked on each request). A successful **password reset or change** bumps the same version — revoking all existing sessions, including the current one. Reusing an already-rotated refresh token is treated as theft and revokes the whole family. - **Baseline security headers** are added to every response by `SecurityHeadersMiddleware` (`X-Content-Type-Options`, `X-Frame-Options`, `Referrer-Policy`, and HSTS outside `DEBUG`). No CSP is set, to avoid breaking Swagger UI. -- **Behind a proxy, run with forwarded headers** (`make run-prod` / `uvicorn --proxy-headers --forwarded-allow-ips=...`) — otherwise per-IP rate limiting and logging see the load balancer's IP, not the client's. +- **Behind a proxy, run with forwarded headers** (`make run-prod` / `uvicorn --proxy-headers --forwarded-allow-ips=...`) — otherwise per-IP rate limiting and logging see the load balancer's IP, not the client's. Set `--forwarded-allow-ips` to your proxy's IP, never `"*"` (spoofable). This also keeps the docs IP-allowlist meaningful — without it a co-located proxy makes every client look like `127.0.0.1`. +- **Auth is built to resist enumeration.** Login runs bcrypt even for unknown emails (constant-ish timing), and `/signup` returns the same generic message whether or not the email is registered (so it no longer returns the created user). Reset/activation emails are additionally throttled per-account (cooldown) on top of the per-IP rate limit. +- **`JWT_SECRET` must be ≥ 32 chars in production**, requests over `MAX_REQUEST_BODY_BYTES` (1 MB) get a 413, and every route has a `120/minute` default rate-limit backstop beneath the stricter per-route limits. - **Repeated bad codes lock the account.** After `MAX_CODE_ATTEMPTS` (default 5) wrong activation/reset codes, that account is locked for `CODE_LOCKOUT_SECONDS`; a successful attempt clears the counter. - **`/health` and startup checks.** `/health` returns 503 if the DB or Redis is unreachable. On boot the app pings both; in production (`DEBUG=False`) it refuses to start if either is down, in `DEBUG` it only logs. - **Sign-in requires a verified email.** `signin_user` returns `403 Email not verified` until activation flips `is_verified`. In tests, `UserFactory` builds verified users; use `create_user`/an unverified user to exercise the rejection. diff --git a/UPGRADING.md b/UPGRADING.md index 75400b9..d976708 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -32,6 +32,11 @@ independent, so apply them in any order you like and run your tests after each. | Security response headers | Safe (additive) | See §15 | | Proxy headers behind a load balancer | Deploy config | See §16 | | CI hardening (SAST, lint gate, coverage floor) | Safe (additive) | See §17 | +| Non-enumerable login + signup | **Breaking (API)** | Signup returns a message, not the user; see §18 | +| Per-account email cooldown | Behavioral | See §19 | +| Stronger `JWT_SECRET` (min length) | Behavioral | See §20 | +| Body-size limit + global rate-limit backstop | Safe (additive) | See §21 | +| Dependabot + scheduled/hashed deps | Safe (additive) | See §22 | **Backup first:** commit or branch before starting, and snapshot your database before running the migration in §6. @@ -649,6 +654,99 @@ client's `X-Forwarded-For` and is spoofable if you're not actually behind a prox --- +## 18. Non-enumerable authentication + +Stop attackers from discovering which emails have accounts. + +- **Constant-time login.** `authenticate_user` must spend the same bcrypt time + whether or not the user exists — otherwise response timing leaks account + existence. Compare against a precomputed dummy hash on the missing-user path: + + ```python + _DUMMY_PASSWORD_HASH = get_password_hash(secrets.token_urlsafe(32)) + + user = await get_user(username, session) + if not user: + verify_password(password, _DUMMY_PASSWORD_HASH) # equalize timing + return False + ``` + +- **Non-enumerable signup.** Don't return `400 "Email already registered"`. + Respond with the same generic message (and 200) whether or not the email is + taken; run a dummy `get_password_hash` on the existing-email path to match + timing, and send nothing for an existing account. The signup route no longer + uses `response_model=UserModel` — it returns `{"detail": "..."}`. + +## 19. Per-account email cooldown + +Rate limits are per-IP; add a per-account cooldown so a distributed attacker +can't email-bomb a victim with reset/activation mail: + +```python +CODE_EMAIL_COOLDOWN_SECONDS = 60 + +async def email_cooldown_active(scope: str, email: str) -> bool: + key = f"cooldown-{scope}-{email}" + if await redis_manager.get_int(key): + return True + await redis_manager.increment(key, ttl=CODE_EMAIL_COOLDOWN_SECONDS) + return False + +# in initiate_password_reset / resend_activation, before sending: +if not user or await email_cooldown_active("reset", email): + return {"detail": "Password Reset Code Sent"} # same generic response +``` + +## 20. Stronger JWT secret + +HS256 security is only as strong as the secret. Require real length in +production (not just non-empty) so a guessable secret can't be used to forge +tokens: + +```python +MIN_JWT_SECRET_LENGTH = 32 + +@model_validator(mode="after") +def _require_strong_jwt_secret_in_production(self): + if not self.DEBUG and len(self.JWT_SECRET) < self.MIN_JWT_SECRET_LENGTH: + raise ValueError("JWT_SECRET must be >= 32 chars when DEBUG is False") + return self +``` + +## 21. Request size limit + global rate-limit backstop + +- **Body-size middleware** rejects oversized requests before they're buffered + (memory-exhaustion DoS): + + ```python + class MaxBodySizeMiddleware(BaseHTTPMiddleware): + async def dispatch(self, request, call_next): + cl = request.headers.get("content-length") + if cl and cl.isdigit() and int(cl) > settings.MAX_REQUEST_BODY_BYTES: + return JSONResponse(status_code=413, content={"detail": "Request body too large"}) + return await call_next(request) + ``` + +- **Global rate-limit backstop.** Per-route `@limiter.limit` leaves unlimited + everything you didn't decorate (e.g. `/health`). Give the limiter + `default_limits=["120/minute"]` and add `SlowAPIMiddleware` so every route has + a floor; per-route limits still override. + +## 22. Deployment hardening + +- **Proxy headers (see §16):** run with `--proxy-headers --forwarded-allow-ips` + set to your proxy's IP — **never `"*"`** (spoofable → rate-limit bypass). This + also makes the docs IP-allowlist trustworthy: without it, a co-located proxy + makes every client look like `127.0.0.1` and exposes `/docs`. Prefer gating + docs on `DEBUG` in production. +- **Dependabot** (`.github/dependabot.yml`) for weekly `pip` + `github-actions` + update PRs, and a **scheduled** run of the audit workflow (cron) so + newly-disclosed CVEs in already-pinned deps are caught between pushes. +- **Pin with hashes** (`pip-compile --generate-hashes`) to defeat registry + substitution — do this with pip-tools rather than by hand. + +--- + ## Verify After applying the steps you want: diff --git a/app/limiter.py b/app/limiter.py index 7970f02..98f807b 100644 --- a/app/limiter.py +++ b/app/limiter.py @@ -10,5 +10,6 @@ # the app in main.py. limiter = Limiter( key_func=get_remote_address, + default_limits=[settings.RATE_LIMIT_DEFAULT], storage_uri=f"redis://{settings.REDIS_HOST}:{settings.REDIS_PORT}", ) diff --git a/app/main.py b/app/main.py index 46c16c5..16d8981 100644 --- a/app/main.py +++ b/app/main.py @@ -8,6 +8,7 @@ from fastapi.responses import JSONResponse from slowapi import _rate_limit_exceeded_handler from slowapi.errors import RateLimitExceeded +from slowapi.middleware import SlowAPIMiddleware from sqlalchemy import text from starlette.middleware.base import BaseHTTPMiddleware from starlette.middleware.trustedhost import TrustedHostMiddleware @@ -18,6 +19,7 @@ from app.logger import logger from app.middlewares import ( AllowAuthorizedDocAccess, + MaxBodySizeMiddleware, SecurityHeadersMiddleware, log_request_middleware, ) @@ -88,11 +90,14 @@ def initiate_app(): ) app.add_middleware(AllowAuthorizedDocAccess) app.add_middleware(SecurityHeadersMiddleware) + app.add_middleware(MaxBodySizeMiddleware) app.add_middleware(BaseHTTPMiddleware, dispatch=log_request_middleware) - # Enforce the rate limits declared via @limiter.limit(...) on the routes. + # Enforce rate limits: the default backstop (SlowAPIMiddleware) on every + # route, plus stricter per-route @limiter.limit(...) declarations. app.state.limiter = limiter app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) # type: ignore + app.add_middleware(SlowAPIMiddleware) app.include_router(api) app.include_router(health_router) diff --git a/app/middlewares.py b/app/middlewares.py index 04de094..93ae239 100644 --- a/app/middlewares.py +++ b/app/middlewares.py @@ -23,6 +23,24 @@ async def log_request_middleware(request: Request, call_next): return response +class MaxBodySizeMiddleware(BaseHTTPMiddleware): + """Reject over-sized request bodies before they are buffered into memory.""" + + async def dispatch( + self, request: Request, call_next: RequestResponseEndpoint + ) -> Response: + content_length = request.headers.get("content-length") + if ( + content_length is not None + and content_length.isdigit() + and int(content_length) > settings.MAX_REQUEST_BODY_BYTES + ): + return JSONResponse( + status_code=413, content={"detail": "Request body too large"} + ) + return await call_next(request) + + class SecurityHeadersMiddleware(BaseHTTPMiddleware): """Add baseline hardening headers to every response.""" @@ -42,6 +60,11 @@ async def dispatch( class AllowAuthorizedDocAccess(BaseHTTPMiddleware): + # WARNING: `request.client.host` is the *proxy's* IP unless the app runs + # with --proxy-headers behind a trusted proxy. If your reverse proxy is + # co-located (e.g. on 127.0.0.1) and proxy headers are NOT enabled, every + # request appears to come from this list and the docs are exposed to all. + # Prefer gating docs by DEBUG in production, or ensure --proxy-headers. allowed_ips = [ "127.0.0.1", # allows Viewing Docs in Local Development Environment ] diff --git a/app/routers/auth.py b/app/routers/auth.py index 1697aa1..f2fe087 100644 --- a/app/routers/auth.py +++ b/app/routers/auth.py @@ -21,7 +21,7 @@ CurrentUserDep = Annotated[UserDB, Depends(get_current_user)] -@router.post("/signup", response_model=auth_schemas.UserModel) +@router.post("/signup") @limiter.limit("5/minute") async def signup( request: Request, @@ -29,6 +29,8 @@ async def signup( bg_task: BackgroundTasks, # needed to send verification/welcome email payload: auth_schemas.UserSignUpData, ): + # Returns a generic message (not the user) so the response is identical + # whether or not the email is already registered - see signup_user. return await auth_services.signup_user(payload, db, bg_task) diff --git a/app/routers/tests/test_auth.py b/app/routers/tests/test_auth.py index b37efac..f04ca62 100644 --- a/app/routers/tests/test_auth.py +++ b/app/routers/tests/test_auth.py @@ -4,14 +4,24 @@ from app.models import User as UserDB from app.redis_manager import redis_manager from app.schemas import auth as auth_schemas -from app.schemas.auth import UserModel +from app.services.auth import GENERIC_SIGNUP_MESSAGE async def test_signup_succeeds(client: AsyncClient, signup_data: dict[str, str]): response = await client.post("/v1/auth/signup", json=signup_data) assert response.status_code == 200 - response_data = response.json() - assert UserModel.model_validate(response_data) + # Non-enumerable: a generic message, not the created user. + assert response.json()["detail"] == GENERIC_SIGNUP_MESSAGE + + +async def test_signup_is_non_enumerable(client: AsyncClient, user: UserDB): + # Signing up with an ALREADY-registered email returns the same 200 + + # message as a fresh signup, so accounts can't be enumerated. + response = await client.post( + "/v1/auth/signup", json={"email": user.email, "password": "password123"} + ) + assert response.status_code == 200 + assert response.json()["detail"] == GENERIC_SIGNUP_MESSAGE @pytest.mark.parametrize( diff --git a/app/services/auth.py b/app/services/auth.py index 5372598..6f0d287 100644 --- a/app/services/auth.py +++ b/app/services/auth.py @@ -30,6 +30,12 @@ # every token issued before it. Failure counters gate code brute-forcing. MAX_CODE_ATTEMPTS = 5 CODE_LOCKOUT_SECONDS = 15 * 60 +# Minimum gap between code emails to the same account (anti email-bombing). +CODE_EMAIL_COOLDOWN_SECONDS = 60 + +# Signup responds identically whether or not the email is already registered, +# so an attacker can't enumerate accounts through the signup endpoint. +GENERIC_SIGNUP_MESSAGE = "Please check your email to activate your account." # --- Redis key builders (single source of truth for key formats) ------------ @@ -49,6 +55,22 @@ def failed_attempts_key(scope: str, email: str) -> str: return f"failed-{scope}-{email}" +def email_cooldown_key(scope: str, email: str) -> str: + return f"cooldown-{scope}-{email}" + + +async def email_cooldown_active(scope: str, email: str) -> bool: + """ + Rate-limit code emails per account: True if one was sent within the last + CODE_EMAIL_COOLDOWN_SECONDS (and the caller should skip sending another). + """ + key = email_cooldown_key(scope, email) + if await redis_manager.get_int(key): + return True + await redis_manager.increment(key, ttl=CODE_EMAIL_COOLDOWN_SECONDS) + return False + + async def invalidate_all_sessions(email: str) -> None: """Bump the user's token version so every existing token is rejected.""" await redis_manager.increment(token_version_key(email)) @@ -83,6 +105,12 @@ def get_password_hash(password: str): ).decode() +# A real hash to verify against when the account doesn't exist, so a missing +# user costs the same bcrypt work as a wrong password - defeating timing-based +# user enumeration on the login endpoint. +_DUMMY_PASSWORD_HASH = get_password_hash(secrets.token_urlsafe(32)) + + def generate_random_code(n: int = 4) -> str: # secrets.choice is cryptographically secure - important for OTP / reset # / activation codes that gate account access. @@ -119,7 +147,7 @@ async def initiate_password_reset( email: str, session: AsyncSession, background_task: BackgroundTasks ): user = await get_user(email, session) - if not user: + if not user or await email_cooldown_active("reset", email): return {"detail": "Password Reset Code Sent"} code = generate_random_code(6) @@ -235,6 +263,9 @@ async def authenticate_user( ) -> UserDB | Literal[False]: user: UserDB | None = await get_user(username, session) if not user: + # Spend the same bcrypt time as a real comparison so response timing + # doesn't reveal whether the account exists. + verify_password(password, _DUMMY_PASSWORD_HASH) return False if not verify_password(password, user.password_hash): return False @@ -246,6 +277,14 @@ async def signup_user( session: AsyncSession, bg_task: BackgroundTasks, ): + # Non-enumerable: respond identically whether or not the email is taken. + existing = await get_user(data.email, session) + if existing: + # Match the bcrypt cost of a real signup so timing doesn't leak, and + # send nothing (the real owner already has an account). + get_password_hash(data.password) + return {"detail": GENERIC_SIGNUP_MESSAGE} + user = await create_user(data, session) code = generate_random_code(6) @@ -260,7 +299,7 @@ async def signup_user( payload={"username": user.email.split("@")[0], "code": code}, template="auth/verification.html", ) - return user + return {"detail": GENERIC_SIGNUP_MESSAGE} async def resend_activation_code( @@ -268,7 +307,7 @@ async def resend_activation_code( ): user = await get_user(email, session) - if not user: + if not user or await email_cooldown_active("activation", email): return {"detail": "Activation Code Sent"} code = generate_random_code(6) diff --git a/app/services/tests/test_auth.py b/app/services/tests/test_auth.py index d568278..c00ab50 100644 --- a/app/services/tests/test_auth.py +++ b/app/services/tests/test_auth.py @@ -168,8 +168,28 @@ async def test_signup_user(session: AsyncSession): session, BackgroundTasks(tasks=[]), ) - assert isinstance(result, UserDB) - assert result.email == email + # Returns a generic message (non-enumerable), but the account is created. + assert result == {"detail": auth_services.GENERIC_SIGNUP_MESSAGE} + assert await auth_services.get_user(email, session) is not None + + +async def test_signup_user_existing_email_is_silent( + user: UserDB, session: AsyncSession +): + # Same message for an already-registered email; nothing leaks. + result = await auth_services.signup_user( + auth_schemas.UserSignUpData(email=user.email, password="password"), + session, + BackgroundTasks(tasks=[]), + ) + assert result == {"detail": auth_services.GENERIC_SIGNUP_MESSAGE} + + +async def test_email_cooldown_blocks_rapid_resends(): + email = faker.email() + # First send is allowed, an immediate second is throttled. + assert await auth_services.email_cooldown_active("reset", email) is False + assert await auth_services.email_cooldown_active("reset", email) is True @pytest.mark.parametrize( diff --git a/app/settings.py b/app/settings.py index e872916..83a8e54 100644 --- a/app/settings.py +++ b/app/settings.py @@ -17,6 +17,12 @@ class Settings(BaseSettings): REDIS_HOST: str = "localhost" REDIS_PORT: int = 6379 + # Reject request bodies larger than this (anti memory-exhaustion DoS). + MAX_REQUEST_BODY_BYTES: int = 1024 * 1024 # 1 MB + # Backstop rate limit applied to every route (stricter per-route limits + # via @limiter.limit still take precedence). + RATE_LIMIT_DEFAULT: str = "120/minute" + MAIL_USERNAME: str # required environment variable MAIL_PASSWORD: str # required environment variable MAIL_FROM: str # required environment variable @@ -24,12 +30,21 @@ class Settings(BaseSettings): MAIL_SERVER: str # required environment variable MAIL_FROM_NAME: str # required environment variable + # HS256 security rests entirely on this secret's strength; a short/guessable + # value lets an attacker forge tokens and bypass every downstream control. + MIN_JWT_SECRET_LENGTH: int = 32 + @model_validator(mode="after") - def _require_jwt_secret_in_production(self) -> "Settings": - # An empty JWT_SECRET signs forgeable tokens. Allow it only in DEBUG - # (local/dev); refuse to boot in production so the misconfig is loud. - if not self.DEBUG and not self.JWT_SECRET: - raise ValueError("JWT_SECRET must be set when DEBUG is False") + def _require_strong_jwt_secret_in_production(self) -> "Settings": + # Enforced only outside DEBUG so local/dev stays frictionless; production + # refuses to boot on a missing or weak secret. + if not self.DEBUG and len(self.JWT_SECRET) < self.MIN_JWT_SECRET_LENGTH: + raise ValueError( + "JWT_SECRET must be set to at least " + f"{self.MIN_JWT_SECRET_LENGTH} characters when DEBUG is False. " + 'Generate one with: python -c "import secrets; ' + 'print(secrets.token_urlsafe(64))"' + ) return self diff --git a/app/tests/test_main.py b/app/tests/test_main.py index 02d7321..be471ee 100644 --- a/app/tests/test_main.py +++ b/app/tests/test_main.py @@ -1,5 +1,6 @@ from datetime import datetime +import pytest from httpx import ASGITransport, AsyncClient from app.main import app @@ -27,6 +28,37 @@ async def test_security_headers_present(client): assert "referrer-policy" in response.headers +async def test_oversized_request_body_rejected(client): + from app.settings import settings + + big_body = "x" * (settings.MAX_REQUEST_BODY_BYTES + 1) + response = await client.post( + "/v1/auth/signup", + content=big_body, + headers={"content-type": "application/json"}, + ) + assert response.status_code == 413 + + +def test_weak_jwt_secret_rejected_in_production(): + from pydantic import ValidationError + + from app.settings import Settings + + with pytest.raises(ValidationError): + Settings( + DEBUG=False, + JWT_SECRET="too-short", + DATABASE_URL="x", + MAIL_USERNAME="a", + MAIL_PASSWORD="b", + MAIL_FROM="c", + MAIL_PORT="1", + MAIL_SERVER="d", + MAIL_FROM_NAME="e", + ) + + async def test_http_exception_handler(): async with AsyncClient( transport=ASGITransport(app=app), base_url="http://test" From 1373a279bfcfd7455573169273d836b653c2c00b Mon Sep 17 00:00:00 2001 From: Brian Obot Date: Tue, 14 Jul 2026 00:30:36 +0100 Subject: [PATCH 3/4] Add Start script to the project --- start.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 start.sh diff --git a/start.sh b/start.sh new file mode 100644 index 0000000..44f5f0f --- /dev/null +++ b/start.sh @@ -0,0 +1,19 @@ +#!/bin/bash +set -e # Exit if any command fails + +source venv/bin/activate + +git pull + +pip install -r requirements.txt + +# apply migration +alembic upgrade head +echo "✅ 1/3 Successfully Applied Database Migration" + +lsof -ti :9090 | xargs --no-run-if-empty kill -9 +echo "✅ 2/3 Kill the Former Process on the Same Port" + +# start the Web App again +nohup uvicorn app.main:app --host 0.0.0.0 --port 9090 --forwarded-allow-ips="127.0.0.1" > uvicorn.log 2>&1 & +echo "✅ 3/3 Deployment successful!" From 4fb4ab18e5d7667e3ff36e9e86ab353efebf6c0c Mon Sep 17 00:00:00 2001 From: Brian Obot Date: Wed, 15 Jul 2026 10:57:35 +0100 Subject: [PATCH 4/4] Finesse --- .coverage | Bin 53248 -> 53248 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/.coverage b/.coverage index 1aed8ead50052c95eacb56f6a75760dfb5db611f..a77b9d15c9ffd261458cfbc6f0aaab0cb8c4c1f2 100644 GIT binary patch delta 679 zcmZozz}&Eac|sCX)`g8JGxeE`^-MMyum4sO+Rz?Z_TfqeER9+cV|w&eY*FK#U7pb%BMQ!6DNAJGco{; zW3vY`nG}EyR0{^WlT8NbE5c4nZllQ;G$ zS=k#uo%8(7PJVVVP#|!9V%87^GJ+((#ccX{`?mD&J7>=Bd3pKy?sa?LoPEFZ?CsN= zzE8>*+j;);_sKl{vNF6A8tVnYjuQE`&!YZ`e*MqW`unSYKAXN-eB#7x4DX16ydwbe zjxZw+KaiH<46EC>eR5d8oD(}IH&8%?qwLsyHa0Gh8a6ILP9Uu!y6fioH+MJf_c!=; z=6JI<2hhT{Fu`y2@^j`1p7<z8r>0H(nYnE(I) delta 512 zcmV+b0{{JhpaX!Q1CU|@mB_JVnJ)u0E-=1OW*gBI;!N>8GFY z+Y}-M0SPK0S}H%EeOdpd&N;bUE?@b) zmwe5yeC9LHC-XD$viW4cwQqTo%jJ{q%**BFz1%pwBwu;uEH?%m1OW*S905fe1px_x z3JvbxZ;M6D%uM){;LGEKBv0J)-Oo%(pMCvyGLt9s31*#N(#|j8+?@U7zIXEu zBQSgff2X_2JdWFs;2e`aiy}EO5ha8m1h@|d76bta4hj+|6$AkZQW^GKviaNF8~NPK z&AjQ;=ew@G?9K1F&D+b8_mXcGiO&=8lbVYiEsc>D5d;AVE)@E{I0xh)2RX