fix(dns-webui): store JWTs in HttpOnly cookies, not localStorage (XSS hardening) - #97
Merged
Merged
Conversation
… hardening) dns-webui kept access + refresh JWTs in localStorage, so any JavaScript (app or a compromised dependency) could exfiltrate them. Moved to HttpOnly, Secure, SameSite=Strict cookies that JS cannot read. Change is ADDITIVE — bearer-token clients (manager/frontend, Go client) are unaffected. Backend (manager): - app/services/cookie_auth.py (new): set/clear HttpOnly+Secure+SameSite=Strict access/refresh cookies + a JS-readable csrf_token cookie; double-submit CSRF check (secrets.compare_digest). - middleware/auth.py: token_required accepts the cookie as a fallback to the Authorization header; enforces CSRF on cookie-sourced mutating requests only (header clients exempt). - blueprints/auth.py (login/refresh/logout), mfa.py (mfa_verify): set cookies alongside the existing JSON tokens; logout clears cookies + revokes the cookie-sourced refresh token. - config.py: COOKIE_SECURE/COOKIE_DOMAIN (dev/test default to non-Secure). Frontend (dns-webui): - Removed ALL localStorage token read/write; api uses withCredentials:true and auto-attaches the CSRF header on mutating requests; 401 -> cookie-based refresh. - useAuth tracks auth state via /auth/me, never holding the JWT in JS. Tests: 16 backend cookie/CSRF tests + updated frontend Login test. Backend suite 492 passed (1 pre-existing env-only observability failure); dns-webui vitest 9 passed. flake8 clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
dns-webui stored access + refresh JWTs in
localStorage, so any JavaScript (app or a compromised dependency) could exfiltrate them (persistent account takeover via the refresh token). Moved to HttpOnly, Secure, SameSite=Strict cookies that JS cannot read. The change is additive — bearer-token clients (manager/frontend, Go client) are unaffected.Backend (manager)
app/services/cookie_auth.py(new): sets/clears HttpOnly+Secure+SameSite=Strictaccess_token/refresh_tokencookies + a JS-readablecsrf_tokencookie; double-submit CSRF check (secrets.compare_digest).middleware/auth.py:token_requiredaccepts the cookie as a fallback to theAuthorizationheader; enforces CSRF on cookie-sourced mutating requests only (header clients exempt).blueprints/auth.py(login/refresh/logout),mfa.py(mfa_verify): set cookies alongside the existing JSON tokens; logout clears cookies + revokes the cookie-sourced refresh token.config.py:COOKIE_SECURE/COOKIE_DOMAIN(dev/test default to non-Secure for plain HTTP).Frontend (dns-webui)
localStoragetoken read/write; API useswithCredentials: trueand auto-attaches the CSRF header on mutating requests; 401 → cookie-based refresh.useAuthtracks auth state via/auth/me, never holding the JWT in JS.document.cookiecannot see the HttpOnly token cookies.Why cookies + CSRF (not just cookies)
A cookie-auth flow without CSRF protection trades XSS for CSRF, so the double-submit token is part of the fix, not an add-on.
Verification
Pre-existing issues surfaced (NOT introduced here, not fixed)
tsc/vite buildfail on@penguintechinc/react-libs'spackage.jsonexports["."]ordering (importbeforetypes) — breaksmoduleResolution: bundlerfor every consumer; fix belongs inpenguin-libs. Confirmed identical error on untouched files.npm run lintfails:eslintreferenced but not a devDependency.vite.config.ts/nginx.confstill point at a legacyflask-apitarget that no longer exists — dead config, separate cleanup.🤖 Generated with Claude Code