Skip to content
Merged

Dev #24

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified .coverage
Binary file not shown.
14 changes: 14 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
@@ -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"
17 changes: 10 additions & 7 deletions .github/workflows/audit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -23,14 +26,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
33 changes: 32 additions & 1 deletion .github/workflows/create_pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@
run-local:
fastapi dev app/main.py

# Production: --proxy-headers makes rate limiting and logging see the real
# 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="127.0.0.1"

test-local:
pytest -s --cov

Expand Down
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,11 @@ 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. 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.
Expand Down
198 changes: 198 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ 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 |
| 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.
Expand Down Expand Up @@ -549,6 +558,195 @@ 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="<proxy-ip>"
```

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 <tests> --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.

---

## 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:
Expand Down
1 change: 1 addition & 0 deletions app/limiter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}",
)
14 changes: 12 additions & 2 deletions app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -16,7 +17,12 @@
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,
MaxBodySizeMiddleware,
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
Expand Down Expand Up @@ -83,11 +89,15 @@ 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)
Expand Down
Loading
Loading