feat: parameterize cookie names and harden logout revocation#20
Merged
Conversation
- 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
COOKIE_PREFIXsetting 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 forotel_service_name.decode_refresh_tokenhelper with a narrowjwt.PyJWTErrorcatch, shared between the refresh-validation and logout paths. Drops the broadexcept 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.key="app_access"in the refresh handler (now readscookie_transport.cookie_name), and switches Cookie params toCookie(..., 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:
csrf_token_cookie_secureand 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 intests/test_auth.py)uv run ruff check app/ tests/— cleanuv run ruff format --check .— cleanCOOKIE_PREFIX=app, rejectsOTEL_SERVICE_NAME=api-template, accepts explicit overridesSet-Cookie: criticalbit_verify_access=...; criticalbit_verify_refresh=...(noapp_*anywhere), and post-logout DB inspection showedis_revoked=Trueon all rows of the token familyFork migration note
When pulling this template update into an existing fork, set
COOKIE_PREFIXin.envand the production environment to a service-scoped value before redeploying. The rename effectively forces a logout for every active session (oldapp_*cookies are no longer recognized), so plan the rollout accordingly.