diff --git a/.coverage b/.coverage index b3c7568..a77b9d1 100644 Binary files a/.coverage and b/.coverage differ 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 8f00fed..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: @@ -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 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..a1bb545 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/README.md b/README.md index 7a1e850..da96085 100644 --- a/README.md +++ b/README.md @@ -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. diff --git a/UPGRADING.md b/UPGRADING.md index 379f578..d976708 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -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. @@ -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="" +``` + +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. + +--- + +## 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 67b588e..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 @@ -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 @@ -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) diff --git a/app/middlewares.py b/app/middlewares.py index 356716f..93ae239 100644 --- a/app/middlewares.py +++ b/app/middlewares.py @@ -23,7 +23,48 @@ 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.""" + + 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): + # 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 4122b73..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( @@ -191,6 +201,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..6f0d287 100644 --- a/app/services/auth.py +++ b/app/services/auth.py @@ -26,23 +26,63 @@ 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 +# 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) ------------ 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}" + + +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)) + + 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, @@ -65,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. @@ -101,12 +147,12 @@ 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) 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 +170,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 +191,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 +232,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 +251,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)}) @@ -211,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 @@ -222,11 +277,19 @@ 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) 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( @@ -236,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( @@ -244,12 +307,12 @@ 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) 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 +332,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 +348,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 +418,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 +496,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..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( @@ -299,6 +319,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/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 f342443..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 @@ -20,6 +21,44 @@ 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_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" 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!"