Skip to content

feat: parameterize cookie names and harden logout revocation#20

Merged
amrtgaber merged 1 commit into
mainfrom
feat/cookie-prefix-and-logout-fix
Apr 22, 2026
Merged

feat: parameterize cookie names and harden logout revocation#20
amrtgaber merged 1 commit into
mainfrom
feat/cookie-prefix-and-logout-fix

Conversation

@amrtgaber
Copy link
Copy Markdown
Contributor

Summary

  • Adds COOKIE_PREFIX setting so forks can namespace auth cookies per service. Default "app" keeps local dev working out of the box; production validation rejects the template defaults ("", "app", "api-template") so forks can't accidentally deploy with the unchanged default. Same treatment added for otel_service_name.
  • Hardens logout: extracts a decode_refresh_token helper with a narrow jwt.PyJWTError catch, shared between the refresh-validation and logout paths. Drops the broad except Exception: in logout that was swallowing DB errors alongside decode failures — decode failures still log and return 204, but DB errors now propagate so real bugs surface.
  • Also fixes a pre-existing hardcoded key="app_access" in the refresh handler (now reads cookie_transport.cookie_name), and switches Cookie params to Cookie(..., alias=REFRESH_COOKIE_NAME) so the Python parameter name is decoupled from the wire cookie name.

Context

Ports two improvements from criticalbit-auth-api PR #24. Template-audit findings worth knowing:

  • This template uses HS256 end-to-end, so the specific algorithm-mismatch symptom that bit criticalbit (RS256 tokens decoded via the HS256-default wrapper) doesn't manifest here — but the broad-except shape of the logout bug was still present, hence the helper extraction is still worthwhile.
  • No OAuth router exists in this template, so the csrf_token_cookie_secure and OAuth-CSRF-cookie parts of PR Add global rate limit and /.well-known/security.txt #24 were out of scope.
  • REFRESH_AUDIENCE = [\"app:refresh\"] remains hardcoded. It's a JWT-internal claim (not a cookie name), so collisions across services aren't possible via the browser. Parameterizing it would be a separate decision — flagging for future consideration but intentionally not in this PR.

Test plan

  • uv run pytest tests/ — 27 passed (6 new in tests/test_auth.py)
  • uv run ruff check app/ tests/ — clean
  • uv run ruff format --check . — clean
  • Prod config validation smoke-tested: rejects COOKIE_PREFIX=app, rejects OTEL_SERVICE_NAME=api-template, accepts explicit overrides
  • In-process verification with a non-default prefix: live login produced Set-Cookie: criticalbit_verify_access=...; criticalbit_verify_refresh=... (no app_* anywhere), and post-logout DB inspection showed is_revoked=True on all rows of the token family

Fork migration note

When pulling this template update into an existing fork, set COOKIE_PREFIX in .env and the production environment to a service-scoped value before redeploying. The rename effectively forces a logout for every active session (old app_* cookies are no longer recognized), so plan the rollout accordingly.

- Add COOKIE_PREFIX setting (default "app") so forks can namespace auth
  cookies per service. Access and refresh cookie names, plus the Cookie
  param aliases in the refresh and logout handlers, now derive from the
  setting.

- Extract decode_refresh_token helper in app/auth/refresh.py with a
  narrow jwt.PyJWTError catch. Share it between
  validate_and_rotate_refresh_token and the logout handler, eliminating
  duplicate decode logic.

- Replace the broad except Exception: in logout that was swallowing DB
  errors alongside decode failures. Decode failures still log and
  return 204; DB errors now propagate so real bugs surface.

- Fix a hardcoded key="app_access" in the refresh handler; reads
  cookie_transport.cookie_name so both cookie paths share one source
  of truth.

- Extend production config validation to reject weak defaults for
  cookie_prefix ({"", "app", "api-template"}) and otel_service_name
  ({"", "api-template"}).

- Tests: logout revokes single and multi-token families, handles
  garbage/missing cookies gracefully, and a subprocess-reload check
  proves COOKIE_PREFIX wires through end-to-end with a non-default
  value.
@amrtgaber amrtgaber merged commit 3eb83f0 into main Apr 22, 2026
2 checks passed
@amrtgaber amrtgaber deleted the feat/cookie-prefix-and-logout-fix branch April 22, 2026 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant