diff --git a/manager/backend/app/blueprints/auth.py b/manager/backend/app/blueprints/auth.py index 637a70b4..0812db01 100644 --- a/manager/backend/app/blueprints/auth.py +++ b/manager/backend/app/blueprints/auth.py @@ -5,6 +5,13 @@ from flask import Blueprint, request, jsonify, current_app, g from app.services.auth_service import AuthService +from app.services.cookie_auth import ( + REFRESH_COOKIE_NAME, + clear_auth_cookies, + csrf_check_passes, + generate_csrf_token, + set_auth_cookies, +) from app.middleware.auth import token_required, get_current_user from app.utils.decorators import validate_json, audit_log from app.extensions import limiter @@ -85,7 +92,7 @@ def login(): ) refresh_token = AuthService.create_refresh_token(user['id']) - return jsonify({ + response = jsonify({ 'accessToken': access_token, 'refreshToken': refresh_token, 'user': { @@ -95,12 +102,18 @@ def login(): 'global_role': user['global_role'], 'team_roles': user['team_roles'] } - }), 200 + }) + # Additive: cookie-based clients (dns-webui) authenticate via these + # HttpOnly cookies instead of reading the tokens above out of the JSON + # body -- see app/services/cookie_auth.py. Bearer-token clients ignore + # the cookies and keep using the JSON tokens unchanged. + csrf_token = generate_csrf_token() + set_auth_cookies(response, access_token, refresh_token, csrf_token) + return response, 200 @auth_bp.route('/api/v1/auth/refresh', methods=['POST']) @limiter.limit('20/minute') -@validate_json('refreshToken') @audit_log('token_refresh', resource_type='user') def refresh(): """ @@ -110,9 +123,16 @@ def refresh(): access + refresh token pair is issued. Reusing a rotated or revoked refresh token returns 401. + The refresh token is read from the JSON body (existing bearer-token + clients) if present, otherwise from the HttpOnly refresh_token cookie + (dns-webui, which never has JS-level access to the token). Cookie-based + requests must also present a matching X-CSRF-Token header (double-submit + against the csrf_token cookie) since the browser attaches the cookie + automatically. + Request: { - "refreshToken": "..." + "refreshToken": "..." # optional if refresh_token cookie is set } Response: @@ -121,8 +141,21 @@ def refresh(): "refreshToken": "..." } """ - data = request.get_json() - refresh_token = data['refreshToken'] + data = request.get_json(silent=True) or {} + refresh_token = data.get('refreshToken') + from_cookie = False + if not refresh_token: + refresh_token = request.cookies.get(REFRESH_COOKIE_NAME) + from_cookie = bool(refresh_token) + + if not refresh_token: + return jsonify({ + 'error': 'Missing required fields', + 'missing_fields': ['refreshToken'] + }), 400 + + if from_cookie and not csrf_check_passes(): + return jsonify({'error': 'Invalid or missing CSRF token'}), 403 # Resolve the token's subject for audit attribution before rotation -- # get_current_user() has nothing to return here (no access token on this @@ -137,10 +170,13 @@ def refresh(): if not tokens: return jsonify({'error': 'Invalid or expired refresh token'}), 401 - return jsonify({ + response = jsonify({ 'accessToken': tokens['access_token'], 'refreshToken': tokens['refresh_token'] - }), 200 + }) + csrf_token = generate_csrf_token() + set_auth_cookies(response, tokens['access_token'], tokens['refresh_token'], csrf_token) + return response, 200 @auth_bp.route('/api/v1/auth/logout', methods=['POST']) @@ -148,15 +184,17 @@ def refresh(): @audit_log('user_logout') def logout(): """ - Logout user: revoke the refresh token server-side. + Logout user: revoke the refresh token server-side and clear auth cookies. Request (optional body): { - "refreshToken": "..." + "refreshToken": "..." # falls back to the refresh_token cookie } The access token (15 min) simply ages out; the long-lived refresh token - is revoked here so it cannot mint new access tokens after logout. + is revoked here so it cannot mint new access tokens after logout. This + route requires @token_required, so a cookie-authenticated request has + already passed the CSRF check there. Response: { @@ -164,13 +202,15 @@ def logout(): } """ data = request.get_json(silent=True) or {} - refresh_token = data.get('refreshToken') + refresh_token = data.get('refreshToken') or request.cookies.get(REFRESH_COOKIE_NAME) if refresh_token: AuthService.revoke_refresh_token(refresh_token, reason='logout') - return jsonify({ + response = jsonify({ 'message': 'Logged out successfully' - }), 200 + }) + clear_auth_cookies(response) + return response, 200 @auth_bp.route('/api/v1/auth/me', methods=['GET']) diff --git a/manager/backend/app/blueprints/mfa.py b/manager/backend/app/blueprints/mfa.py index 82962daf..0e75c525 100644 --- a/manager/backend/app/blueprints/mfa.py +++ b/manager/backend/app/blueprints/mfa.py @@ -10,6 +10,7 @@ from flask import Blueprint, request, jsonify, current_app, g from app.middleware.auth import token_required, get_current_user from app.services.auth_service import AuthService +from app.services.cookie_auth import generate_csrf_token, set_auth_cookies from app.services.mfa_service import MFAService from app.utils.decorators import validate_json, audit_log from app.extensions import limiter @@ -303,7 +304,7 @@ def mfa_verify(): ) refresh_token = AuthService.create_refresh_token(user_id) - return jsonify({ + response = jsonify({ 'accessToken': access_token, 'refreshToken': refresh_token, 'user': { @@ -313,4 +314,8 @@ def mfa_verify(): 'global_role': user_record['global_role'], 'team_roles': team_roles } - }), 200 + }) + # Additive, same as /auth/login -- see app/services/cookie_auth.py. + csrf_token = generate_csrf_token() + set_auth_cookies(response, access_token, refresh_token, csrf_token) + return response, 200 diff --git a/manager/backend/app/config.py b/manager/backend/app/config.py index 42a0b36d..76f8acb5 100644 --- a/manager/backend/app/config.py +++ b/manager/backend/app/config.py @@ -39,6 +39,14 @@ class Config: JWT_ACCESS_TOKEN_EXPIRES = timedelta(minutes=15) JWT_REFRESH_TOKEN_EXPIRES = timedelta(days=7) + # Cookie-based auth (dns-webui). Defaults to Secure=True (HTTPS-only) in + # every environment -- only disable for local HTTP development, never + # inferred from DEBUG/TESTING, so a misconfigured prod deploy fails + # closed. COOKIE_DOMAIN is unset (host-only cookie) unless the frontend + # and API are deployed on different subdomains of the same parent. + COOKIE_SECURE = os.getenv('COOKIE_SECURE', 'true').lower() == 'true' + COOKIE_DOMAIN = os.getenv('COOKIE_DOMAIN') or None + # SPIFFE/mTLS service-to-service identity (preferred over per-server JWTs). # The service mesh / gateway terminates mTLS and forwards the verified peer # SPIFFE ID in the XFCC header. When present, it supersedes the legacy @@ -106,6 +114,10 @@ class DevelopmentConfig(Config): # Development-only ephemeral secrets (never use in production) SECRET_KEY = os.getenv('SECRET_KEY', 'dev-ephemeral-secret-key-only') JWT_SECRET_KEY = os.getenv('JWT_SECRET_KEY', SECRET_KEY) + # Local dev typically runs over plain HTTP; still explicit opt-in via + # env var so it's never silently disabled by DEBUG alone in a shared + # or misconfigured environment. + COOKIE_SECURE = os.getenv('COOKIE_SECURE', 'false').lower() == 'true' def __init__(self) -> None: """Generate ephemeral ES256 keypair if not configured.""" @@ -162,6 +174,11 @@ class TestingConfig(Config): # Testing-only ephemeral secrets (never use in production) SECRET_KEY = 'test-ephemeral-secret-key-only' JWT_SECRET_KEY = 'test-ephemeral-jwt-key-only' + # The Flask/Werkzeug test client talks plain HTTP; a Secure cookie set + # here would still be recorded, but keeping this False mirrors real + # local/dev HTTP behavior and avoids masking a client-side secure-cookie + # bug behind the test client's more lenient cookie jar. + COOKIE_SECURE = False def __init__(self) -> None: """Generate ephemeral ES256 keypair for testing.""" diff --git a/manager/backend/app/middleware/auth.py b/manager/backend/app/middleware/auth.py index d99cd404..c20df4b6 100644 --- a/manager/backend/app/middleware/auth.py +++ b/manager/backend/app/middleware/auth.py @@ -6,6 +6,10 @@ from functools import wraps from flask import request, jsonify, current_app, g from app.services.auth_service import AuthService +from app.services.cookie_auth import ( + csrf_check_passes, + extract_bearer_or_cookie_token, +) logger = logging.getLogger(__name__) @@ -13,23 +17,28 @@ def token_required(f): """ Decorator to require valid JWT token. + + Accepts the token from either the Authorization header (existing + bearer-token clients: manager/frontend, the Go client, machine clients) + or the HttpOnly access_token cookie (dns-webui). When the token comes + from the cookie, a matching double-submit CSRF header is required on + state-changing requests -- see app/services/cookie_auth.py. + Extracts user information from token and stores in g.current_user. """ @wraps(f) def decorated(*args, **kwargs): - token = None + token, from_cookie = extract_bearer_or_cookie_token() - # Get token from Authorization header - auth_header = request.headers.get('Authorization') - if auth_header: - try: - token = auth_header.split(' ')[1] # Bearer - except IndexError: - return jsonify({'error': 'Invalid authorization header format'}), 401 + if request.headers.get('Authorization') and not token: + return jsonify({'error': 'Invalid authorization header format'}), 401 if not token: return jsonify({'error': 'Authentication token required'}), 401 + if from_cookie and not csrf_check_passes(): + return jsonify({'error': 'Invalid or missing CSRF token'}), 403 + # Decode and validate token payload = AuthService.decode_token(token) if not payload: diff --git a/manager/backend/app/services/cookie_auth.py b/manager/backend/app/services/cookie_auth.py new file mode 100644 index 00000000..77746761 --- /dev/null +++ b/manager/backend/app/services/cookie_auth.py @@ -0,0 +1,180 @@ +""" +Cookie-based JWT auth helpers for browser clients. + +Squawk's browser client (dns-webui) previously stored access/refresh JWTs in +localStorage, which is readable by any JavaScript running on the page -- +including a compromised dependency -- letting it exfiltrate long-lived +credentials (CWE-522: Insufficiently Protected Credentials). This module +issues the same tokens as HttpOnly, Secure, SameSite cookies instead, so the +token value is never exposed to page JavaScript. + +Moving auth to cookies reintroduces CSRF exposure (the browser auto-attaches +cookies to same-site requests), so this module also implements a +double-submit CSRF check: the CSRF token is delivered in a JS-readable +cookie and must be echoed back in a header on state-changing requests. +SameSite=Strict is the primary defense; the CSRF header is defense in depth. + +This is purely additive: bearer-token clients (manager/frontend, the Go +client, machine clients) are unaffected. Cookies are set *alongside* the +existing JSON token response; `token_required` (app/middleware/auth.py) +falls back to the cookie only when no Authorization header is present. +""" + +from __future__ import annotations + +import secrets +from typing import Optional, Tuple + +from flask import Response, current_app, request + +ACCESS_COOKIE_NAME = "access_token" +REFRESH_COOKIE_NAME = "refresh_token" +CSRF_COOKIE_NAME = "csrf_token" +CSRF_HEADER_NAME = "X-CSRF-Token" + +# The refresh token is only ever needed by the refresh/logout endpoints; +# scoping the cookie's Path to this prefix keeps it out of every other +# request the browser makes, narrowing its exposure. +REFRESH_COOKIE_PATH = "/api/v1/auth" + +# State-changing methods require the double-submit CSRF check when auth +# comes from a cookie. GET/HEAD/OPTIONS are side-effect-free by contract. +_CSRF_PROTECTED_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"}) + + +def _cookie_secure() -> bool: + """Whether cookies carry the Secure flag. + + Defaults to True (HTTPS-only) and must be explicitly opted out for local + HTTP development -- never inferred from DEBUG/TESTING, so a + misconfigured production deploy fails closed rather than silently + downgrading to non-Secure cookies. + """ + return bool(current_app.config.get("COOKIE_SECURE", True)) + + +def _cookie_domain() -> Optional[str]: + return current_app.config.get("COOKIE_DOMAIN") or None + + +def generate_csrf_token() -> str: + """Generate a fresh random token for the double-submit CSRF cookie.""" + return secrets.token_urlsafe(32) + + +def set_auth_cookies( + response: Response, + access_token: str, + refresh_token: Optional[str] = None, + csrf_token: Optional[str] = None, +) -> None: + """Attach HttpOnly JWT cookies (and a JS-readable CSRF cookie). + + Additive: callers still return tokens in the JSON body for existing + bearer-token clients. Cookie-based clients ignore the JSON tokens and + rely on these cookies, which JavaScript cannot read. + """ + secure = _cookie_secure() + domain = _cookie_domain() + access_max_age = int(current_app.config["JWT_ACCESS_TOKEN_EXPIRES"].total_seconds()) + + response.set_cookie( + ACCESS_COOKIE_NAME, + access_token, + max_age=access_max_age, + httponly=True, + secure=secure, + samesite="Strict", + domain=domain, + path="/", + ) + + if refresh_token is not None: + refresh_max_age = int(current_app.config["JWT_REFRESH_TOKEN_EXPIRES"].total_seconds()) + response.set_cookie( + REFRESH_COOKIE_NAME, + refresh_token, + max_age=refresh_max_age, + httponly=True, + secure=secure, + samesite="Strict", + domain=domain, + path=REFRESH_COOKIE_PATH, + ) + + if csrf_token is not None: + # Lifetime matches the refresh token (the outer bound of the + # session), not the short-lived access token -- otherwise the CSRF + # cookie would expire at the same moment the access token does, + # right when the browser needs it to call /auth/refresh. + csrf_max_age = int( + current_app.config["JWT_REFRESH_TOKEN_EXPIRES"].total_seconds() + ) + # Deliberately NOT httponly -- the SPA must be able to read this + # value to echo it back in the X-CSRF-Token header. + response.set_cookie( + CSRF_COOKIE_NAME, + csrf_token, + max_age=csrf_max_age, + httponly=False, + secure=secure, + samesite="Strict", + domain=domain, + path="/", + ) + + +def clear_auth_cookies(response: Response) -> None: + """Expire all cookie-auth cookies (logout). + + Delete attributes (path/domain/samesite) must match how each cookie was + set, or the browser treats it as a different cookie and leaves the + original in place. + """ + domain = _cookie_domain() + response.delete_cookie(ACCESS_COOKIE_NAME, path="/", domain=domain, samesite="Strict") + response.delete_cookie( + REFRESH_COOKIE_NAME, path=REFRESH_COOKIE_PATH, domain=domain, samesite="Strict" + ) + response.delete_cookie(CSRF_COOKIE_NAME, path="/", domain=domain, samesite="Strict") + + +def extract_bearer_or_cookie_token() -> Tuple[Optional[str], bool]: + """Return ``(token, from_cookie)``, preferring the Authorization header. + + The header always wins when present so existing header-based clients + are unaffected by a stray cookie. ``from_cookie`` is True only when the + token came exclusively from the cookie, telling the caller to enforce + the CSRF check (header-based bearer auth is not CSRF-exposed: browsers + do not auto-attach custom Authorization headers cross-site). + """ + auth_header = request.headers.get("Authorization") + if auth_header: + parts = auth_header.split(" ") + if len(parts) == 2: + return parts[1], False + return None, False # Malformed header -- do not silently fall back + + cookie_token = request.cookies.get(ACCESS_COOKIE_NAME) + if cookie_token: + return cookie_token, True + + return None, False + + +def csrf_check_passes() -> bool: + """Double-submit CSRF check for cookie-authenticated requests. + + The CSRF cookie value must match the ``X-CSRF-Token`` header exactly. + An attacker's cross-site page can trigger the cookie to be sent + automatically but cannot read it (different origin) to also set the + matching header. + """ + if request.method not in _CSRF_PROTECTED_METHODS: + return True + + cookie_value = request.cookies.get(CSRF_COOKIE_NAME) + header_value = request.headers.get(CSRF_HEADER_NAME) + if not cookie_value or not header_value: + return False + return secrets.compare_digest(cookie_value, header_value) diff --git a/manager/backend/tests/test_cookie_auth.py b/manager/backend/tests/test_cookie_auth.py new file mode 100644 index 00000000..145fb792 --- /dev/null +++ b/manager/backend/tests/test_cookie_auth.py @@ -0,0 +1,308 @@ +"""Tests for HttpOnly-cookie JWT auth (dns-webui XSS token-theft fix). + +Covers the additive cookie flow set up in app/services/cookie_auth.py: +login/refresh/mfa-verify set HttpOnly access/refresh cookies plus a +JS-readable CSRF cookie; token_required accepts either an Authorization +header (existing bearer clients) or the cookie; state-changing +cookie-authenticated requests must present a matching X-CSRF-Token header +(double-submit); logout clears all three cookies and revokes the refresh +token regardless of which transport supplied it. +""" + +from __future__ import annotations + +from app.services.auth_service import AuthService +from app.services.cookie_auth import ( + ACCESS_COOKIE_NAME, + CSRF_COOKIE_NAME, + REFRESH_COOKIE_NAME, +) + + +def _make_user(db, username='cookie_user', password='CorrectHorse123!'): + user_id = db.auth_user.insert( + username=username, + email=f'{username}@example.com', + password_hash=AuthService.hash_password(password), + global_role='Viewer', + active=True, + mfa_enabled=False, + ) + db.commit() + return user_id + + +def _set_cookie_headers(resp): + """Return the raw Set-Cookie header values for the response.""" + return resp.headers.get_all('Set-Cookie') + + +def _find_cookie_header(resp, name): + for header in _set_cookie_headers(resp): + if header.startswith(f'{name}='): + return header + return None + + +class TestLoginSetsCookies: + def test_login_sets_httponly_secure_samesite_cookies(self, app, db, client): + with app.app_context(): + _make_user(db, username='alice_cookie') + + resp = client.post( + '/api/v1/auth/login', + json={'username': 'alice_cookie', 'password': 'CorrectHorse123!'}, + ) + assert resp.status_code == 200 + + access_header = _find_cookie_header(resp, ACCESS_COOKIE_NAME) + refresh_header = _find_cookie_header(resp, REFRESH_COOKIE_NAME) + csrf_header = _find_cookie_header(resp, CSRF_COOKIE_NAME) + + assert access_header is not None + assert 'HttpOnly' in access_header + assert 'SameSite=Strict' in access_header + assert 'Path=/' in access_header + + assert refresh_header is not None + assert 'HttpOnly' in refresh_header + assert 'Path=/api/v1/auth' in refresh_header + + # CSRF cookie must be readable by JS -- no HttpOnly attribute. + assert csrf_header is not None + assert 'HttpOnly' not in csrf_header + + def test_login_still_returns_tokens_in_json_body(self, app, db, client): + """Bearer-token clients (manager/frontend, Go client) must be unaffected.""" + with app.app_context(): + _make_user(db, username='bob_cookie') + + resp = client.post( + '/api/v1/auth/login', + json={'username': 'bob_cookie', 'password': 'CorrectHorse123!'}, + ) + data = resp.get_json() + assert 'accessToken' in data + assert 'refreshToken' in data + + +class TestCookieAuthenticatesRequests: + def test_protected_route_authenticates_via_cookie_only(self, app, db, client): + """No Authorization header at all -- only the cookie jar from login.""" + with app.app_context(): + _make_user(db, username='carol_cookie') + + login_resp = client.post( + '/api/v1/auth/login', + json={'username': 'carol_cookie', 'password': 'CorrectHorse123!'}, + ) + assert login_resp.status_code == 200 + + # client.get() with no Authorization header; the test client's + # cookie jar resends the Set-Cookie values automatically. + me_resp = client.get('/api/v1/auth/me') + assert me_resp.status_code == 200 + assert me_resp.get_json()['username'] == 'carol_cookie' + + def test_bearer_header_still_works_unaffected(self, app, db, client): + """Existing header-based clients keep working exactly as before.""" + with app.app_context(): + user_id = _make_user(db, username='dave_bearer') + token = AuthService.create_access_token(user_id, 'dave_bearer', 'Viewer') + + resp = client.get( + '/api/v1/auth/me', headers={'Authorization': f'Bearer {token}'} + ) + assert resp.status_code == 200 + assert resp.get_json()['username'] == 'dave_bearer' + + def test_malformed_authorization_header_rejected(self, client): + resp = client.get('/api/v1/auth/me', headers={'Authorization': 'NotBearer'}) + assert resp.status_code == 401 + + def test_no_token_at_all_rejected(self, client): + resp = client.get('/api/v1/auth/me') + assert resp.status_code == 401 + + +class TestCsrfProtectionOnCookiePath: + def test_mutating_cookie_request_without_csrf_header_is_rejected(self, app, db, client): + with app.app_context(): + _make_user(db, username='erin_csrf') + + login_resp = client.post( + '/api/v1/auth/login', + json={'username': 'erin_csrf', 'password': 'CorrectHorse123!'}, + ) + assert login_resp.status_code == 200 + + # logout is a state-changing POST behind @token_required; the + # cookie jar supplies the access_token cookie but no CSRF header. + resp = client.post('/api/v1/auth/logout') + assert resp.status_code == 403 + + def test_mutating_cookie_request_with_correct_csrf_header_succeeds(self, app, db, client): + with app.app_context(): + _make_user(db, username='frank_csrf') + + login_resp = client.post( + '/api/v1/auth/login', + json={'username': 'frank_csrf', 'password': 'CorrectHorse123!'}, + ) + csrf_token = login_resp.headers.get('Set-Cookie') + # Pull the actual csrf_token value out of the client cookie jar. + csrf_value = client.get_cookie(CSRF_COOKIE_NAME).value + + resp = client.post( + '/api/v1/auth/logout', headers={'X-CSRF-Token': csrf_value} + ) + assert resp.status_code == 200 + + def test_mutating_cookie_request_with_wrong_csrf_header_is_rejected(self, app, db, client): + with app.app_context(): + _make_user(db, username='grace_csrf') + + client.post( + '/api/v1/auth/login', + json={'username': 'grace_csrf', 'password': 'CorrectHorse123!'}, + ) + resp = client.post( + '/api/v1/auth/logout', headers={'X-CSRF-Token': 'attacker-guessed-value'} + ) + assert resp.status_code == 403 + + def test_get_request_via_cookie_does_not_require_csrf(self, app, db, client): + """GET is side-effect-free; no CSRF header should be required.""" + with app.app_context(): + _make_user(db, username='henry_csrf') + + client.post( + '/api/v1/auth/login', + json={'username': 'henry_csrf', 'password': 'CorrectHorse123!'}, + ) + resp = client.get('/api/v1/auth/me') + assert resp.status_code == 200 + + +class TestRefreshViaCookie: + def test_refresh_with_no_body_uses_cookie_and_requires_csrf(self, app, db, client): + with app.app_context(): + _make_user(db, username='iris_refresh') + + client.post( + '/api/v1/auth/login', + json={'username': 'iris_refresh', 'password': 'CorrectHorse123!'}, + ) + + # Missing CSRF header -> rejected even though the refresh_token + # cookie is present and valid. + resp = client.post('/api/v1/auth/refresh', json={}) + assert resp.status_code == 403 + + csrf_value = client.get_cookie(CSRF_COOKIE_NAME).value + resp = client.post( + '/api/v1/auth/refresh', json={}, headers={'X-CSRF-Token': csrf_value} + ) + assert resp.status_code == 200 + data = resp.get_json() + assert 'accessToken' in data and 'refreshToken' in data + + # Rotation also re-set the cookies. + assert _find_cookie_header(resp, ACCESS_COOKIE_NAME) is not None + assert _find_cookie_header(resp, REFRESH_COOKIE_NAME) is not None + + def test_refresh_body_token_still_works_for_bearer_clients(self, app, db, client): + with app.app_context(): + user_id = _make_user(db, username='jack_refresh') + refresh_token = AuthService.create_refresh_token(user_id) + + # Fresh client with no cookie jar state -- purely body-driven, as a + # non-browser bearer client would do. + resp = client.post( + '/api/v1/auth/refresh', json={'refreshToken': refresh_token} + ) + assert resp.status_code == 200 + assert 'accessToken' in resp.get_json() + + def test_refresh_missing_token_entirely_returns_400(self, client): + resp = client.post('/api/v1/auth/refresh', json={}) + assert resp.status_code == 400 + + +class TestLogoutClearsCookies: + def test_logout_clears_all_three_cookies(self, app, db, client): + with app.app_context(): + _make_user(db, username='karen_logout') + + client.post( + '/api/v1/auth/login', + json={'username': 'karen_logout', 'password': 'CorrectHorse123!'}, + ) + csrf_value = client.get_cookie(CSRF_COOKIE_NAME).value + + resp = client.post( + '/api/v1/auth/logout', headers={'X-CSRF-Token': csrf_value} + ) + assert resp.status_code == 200 + + for name in (ACCESS_COOKIE_NAME, REFRESH_COOKIE_NAME, CSRF_COOKIE_NAME): + header = _find_cookie_header(resp, name) + assert header is not None + # Expired cookies carry Max-Age=0 (or an epoch Expires date). + assert 'Max-Age=0' in header + + def test_logout_revokes_cookie_sourced_refresh_token(self, app, db, client): + with app.app_context(): + _make_user(db, username='larry_logout') + + login_resp = client.post( + '/api/v1/auth/login', + json={'username': 'larry_logout', 'password': 'CorrectHorse123!'}, + ) + refresh_token = login_resp.get_json()['refreshToken'] + csrf_value = client.get_cookie(CSRF_COOKIE_NAME).value + + client.post('/api/v1/auth/logout', headers={'X-CSRF-Token': csrf_value}) + + # The refresh token that was in the (now-cleared) cookie must be + # revoked server-side, not just forgotten client-side. + with app.app_context(): + assert AuthService.refresh_access_token(refresh_token) is None + + +class TestMfaVerifySetsCookies: + def test_mfa_verify_sets_cookies_on_success(self, app, db, client): + import json as _json + import pyotp + from app.services.mfa_service import MFAService + + with app.app_context(): + secret = pyotp.random_base32() + encrypted = MFAService.encrypt_secret(secret) + _, hashed_codes = MFAService.generate_recovery_codes(count=8) + db.auth_user.insert( + username='mona_mfa', + email='mona_mfa@example.com', + password_hash=AuthService.hash_password('password123'), + global_role='Viewer', + active=True, + mfa_enabled=True, + mfa_secret=encrypted, + mfa_recovery_codes=_json.dumps(hashed_codes), + ) + db.commit() + code = pyotp.TOTP(secret).now() + + login_resp = client.post( + '/api/v1/auth/login', + json={'username': 'mona_mfa', 'password': 'password123'}, + ) + pre_auth_token = login_resp.get_json()['pre_auth_token'] + + resp = client.post( + '/api/v1/auth/mfa-verify', + json={'pre_auth_token': pre_auth_token, 'totp_code': code}, + ) + assert resp.status_code == 200 + assert _find_cookie_header(resp, ACCESS_COOKIE_NAME) is not None + assert _find_cookie_header(resp, CSRF_COOKIE_NAME) is not None diff --git a/services/dns-webui/package-lock.json b/services/dns-webui/package-lock.json index 4ea6c30b..8f1f402a 100644 --- a/services/dns-webui/package-lock.json +++ b/services/dns-webui/package-lock.json @@ -16,6 +16,7 @@ "zustand": "4.4.0" }, "devDependencies": { + "@testing-library/dom": "10.4.1", "@testing-library/jest-dom": "6.6.3", "@testing-library/react": "16.3.2", "@testing-library/user-event": "14.5.2", @@ -1363,7 +1364,6 @@ "resolved": "https://registry.npmjs.org/@testing-library/dom/-/dom-10.4.1.tgz", "integrity": "sha512-o4PXJQidqJl82ckFaXUeoAW+XysPLauYI43Abki5hABd853iMhitooc6znOnczgbTYmEP6U6/y1ZyKAIsvMKGg==", "dev": true, - "peer": true, "dependencies": { "@babel/code-frame": "^7.10.4", "@babel/runtime": "^7.12.5", @@ -1448,8 +1448,7 @@ "version": "5.0.4", "resolved": "https://registry.npmjs.org/@types/aria-query/-/aria-query-5.0.4.tgz", "integrity": "sha512-rfT93uj5s0PRL7EzccGMs3brplhcrghnDoV26NqKhCAS1hVo+WdNsPvE/yb6ilfr5hi2MEk6d5EWJTKdxg8jVw==", - "dev": true, - "peer": true + "dev": true }, "node_modules/@types/babel__core": { "version": "7.20.5", @@ -1520,13 +1519,13 @@ "version": "15.7.15", "resolved": "https://registry.npmjs.org/@types/prop-types/-/prop-types-15.7.15.tgz", "integrity": "sha512-F6bEyamV9jKGAFBEmlQnesRPGOQqS2+Uwi0Em15xenOxHaf2hv6L8YCVn3rPdPJOiJfPiCnLIRyvwVaqMY3MIw==", - "devOptional": true + "dev": true }, "node_modules/@types/react": { "version": "18.2.0", "resolved": "https://registry.npmjs.org/@types/react/-/react-18.2.0.tgz", "integrity": "sha512-0FLj93y5USLHdnhIhABk83rm8XEGA7kH3cr+YUlvxoUGp1xNt/DINUMvqPxLyOQMzLmZe8i4RTHbvb8MC7NmrA==", - "devOptional": true, + "dev": true, "dependencies": { "@types/prop-types": "*", "@types/scheduler": "*", @@ -1546,7 +1545,7 @@ "version": "0.26.0", "resolved": "https://registry.npmjs.org/@types/scheduler/-/scheduler-0.26.0.tgz", "integrity": "sha512-WFHp9YUJQ6CKshqoC37iOlHnQSmxNc795UhB26CyBBttrN9svdIrUjl/NjnNmfcwtncN0h/0PPAFWv9ovP8mLA==", - "devOptional": true + "dev": true }, "node_modules/@vitejs/plugin-react": { "version": "5.2.0", @@ -1727,7 +1726,6 @@ "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.1.tgz", "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==", "dev": true, - "peer": true, "engines": { "node": ">=8" } @@ -2150,7 +2148,7 @@ "version": "3.2.3", "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.2.3.tgz", "integrity": "sha512-z1HGKcYy2xA8AGQfwrn0PAy+PB7X/GSj3UVJW9qKyn43xWa+gl5nXmU4qqLMRzWVLFC8KusUX8T/0kCiOYpAIQ==", - "devOptional": true + "dev": true }, "node_modules/data-urls": { "version": "5.0.0", @@ -2220,8 +2218,7 @@ "version": "0.5.16", "resolved": "https://registry.npmjs.org/dom-accessibility-api/-/dom-accessibility-api-0.5.16.tgz", "integrity": "sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg==", - "dev": true, - "peer": true + "dev": true }, "node_modules/dunder-proto": { "version": "1.0.1", @@ -2894,7 +2891,6 @@ "resolved": "https://registry.npmjs.org/lz-string/-/lz-string-1.5.0.tgz", "integrity": "sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ==", "dev": true, - "peer": true, "bin": { "lz-string": "bin/bin.js" } @@ -3329,7 +3325,6 @@ "resolved": "https://registry.npmjs.org/pretty-format/-/pretty-format-27.5.1.tgz", "integrity": "sha512-Qb1gy5OrP5+zDf2Bvnzdl3jsTf1qXVMazbvCoKhtKqVs4/YK4ozX4gKQJJVyNe+cajNPn0KoC0MC3FUmaHWEmQ==", "dev": true, - "peer": true, "dependencies": { "ansi-regex": "^5.0.1", "ansi-styles": "^5.0.0", @@ -3344,7 +3339,6 @@ "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-5.2.0.tgz", "integrity": "sha512-Cxwpt2SfTzTtXcfOlzGEee8O+c+MmUgGrNiBcXnuWxuFJHe6a5Hz7qwhwe5OgaSYI0IJvkLqWX1ASG+cJOkEiA==", "dev": true, - "peer": true, "engines": { "node": ">=10" }, @@ -3417,8 +3411,7 @@ "version": "17.0.2", "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", "integrity": "sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==", - "dev": true, - "peer": true + "dev": true }, "node_modules/react-refresh": { "version": "0.18.0", diff --git a/services/dns-webui/package.json b/services/dns-webui/package.json index f1e5fed5..d2294d7b 100644 --- a/services/dns-webui/package.json +++ b/services/dns-webui/package.json @@ -21,6 +21,7 @@ "zustand": "4.4.0" }, "devDependencies": { + "@testing-library/dom": "10.4.1", "@testing-library/jest-dom": "6.6.3", "@testing-library/react": "16.3.2", "@testing-library/user-event": "14.5.2", diff --git a/services/dns-webui/src/__tests__/Login.test.tsx b/services/dns-webui/src/__tests__/Login.test.tsx index d3961e0d..e1b03959 100644 --- a/services/dns-webui/src/__tests__/Login.test.tsx +++ b/services/dns-webui/src/__tests__/Login.test.tsx @@ -103,9 +103,14 @@ describe('Login page', () => { expect.objectContaining({ id: 1, email: 'test@example.com', - }), - 'test-token', - 'refresh-token' + }) + ); + // Tokens must never be passed to client-side state -- they arrive + // only as HttpOnly Set-Cookie headers on the login response. + expect(mockSetAuthenticated).not.toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.anything() ); }); }); @@ -143,9 +148,7 @@ describe('Login page', () => { first_name: 'Test', last_name: 'User', is_admin: true, - }), - expect.any(String), - expect.any(String) + }) ); }); }); diff --git a/services/dns-webui/src/hooks/useAuth.ts b/services/dns-webui/src/hooks/useAuth.ts index bb1483a5..39c4efb9 100644 --- a/services/dns-webui/src/hooks/useAuth.ts +++ b/services/dns-webui/src/hooks/useAuth.ts @@ -1,4 +1,12 @@ // Zustand Auth Store for Squawk DNS WebUI +// +// Access/refresh JWTs are never held here or in any other JS-readable +// storage (previously localStorage, vulnerable to token exfiltration via +// XSS/compromised dependencies -- CWE-522). The server sets them as +// HttpOnly cookies (manager/backend app/services/cookie_auth.py); this +// store only tracks UI-facing auth state (current user, isAuthenticated), +// derived from the server via /auth/me, never from a token payload decoded +// client-side. import { create } from 'zustand'; import { auth as authApi } from '../services/api'; @@ -9,7 +17,7 @@ interface AuthState { isAuthenticated: boolean; isLoading: boolean; login: (email: string, password: string) => Promise; - setAuthenticated: (user: User, accessToken: string, refreshToken: string) => void; + setAuthenticated: (user: User) => void; logout: () => void; checkAuth: () => Promise; } @@ -24,9 +32,8 @@ export const useAuth = create((set) => ({ const response = await authApi.login(email, password); if (response.success) { - localStorage.setItem('access_token', response.access_token); - localStorage.setItem('refresh_token', response.refresh_token); - + // Tokens arrive as HttpOnly Set-Cookie headers on this response -- + // there is nothing for this client to store. set({ user: response.user as User, isAuthenticated: true, @@ -41,9 +48,9 @@ export const useAuth = create((set) => ({ } }, - setAuthenticated: (user: User, accessToken: string, refreshToken: string) => { - localStorage.setItem('access_token', accessToken); - localStorage.setItem('refresh_token', refreshToken); + setAuthenticated: (user: User) => { + // Tokens were already set as HttpOnly cookies by the login response + // (see pages/Login.tsx); this only updates client-side UI state. set({ user, isAuthenticated: true, isLoading: false }); }, @@ -52,9 +59,6 @@ export const useAuth = create((set) => ({ // Ignore logout API errors, clear local state anyway }); - localStorage.removeItem('access_token'); - localStorage.removeItem('refresh_token'); - set({ user: null, isAuthenticated: false, @@ -63,13 +67,9 @@ export const useAuth = create((set) => ({ }, checkAuth: async () => { - const token = localStorage.getItem('access_token'); - - if (!token) { - set({ user: null, isAuthenticated: false, isLoading: false }); - return; - } - + // No client-readable token to inspect (HttpOnly cookie) -- ask the + // server whether the current session is valid instead of decoding + // anything locally. try { const user = await authApi.getMe(); set({ @@ -78,8 +78,6 @@ export const useAuth = create((set) => ({ isLoading: false, }); } catch (error) { - localStorage.removeItem('access_token'); - localStorage.removeItem('refresh_token'); set({ user: null, isAuthenticated: false, diff --git a/services/dns-webui/src/pages/Login.tsx b/services/dns-webui/src/pages/Login.tsx index 4168296b..d3cce046 100644 --- a/services/dns-webui/src/pages/Login.tsx +++ b/services/dns-webui/src/pages/Login.tsx @@ -9,20 +9,20 @@ const Login: React.FC = () => { const { setAuthenticated } = useAuth(); const handleSuccess = (response: LoginResponse) => { + // response.token/refreshToken are not used here: the backend already + // set them as HttpOnly cookies on this same login response (see + // manager/backend/app/services/cookie_auth.py). setAuthenticated only + // needs the user for client-side UI state. if (response.token && response.user) { - setAuthenticated( - { - id: Number(response.user.id), - email: response.user.email, - first_name: response.user.name?.split(' ')[0] || '', - last_name: response.user.name?.split(' ').slice(1).join(' ') || '', - is_admin: response.user.roles?.includes('admin') || false, - is_active: true, - created_on: '', - }, - response.token, - response.refreshToken || '', - ); + setAuthenticated({ + id: Number(response.user.id), + email: response.user.email, + first_name: response.user.name?.split(' ')[0] || '', + last_name: response.user.name?.split(' ').slice(1).join(' ') || '', + is_admin: response.user.roles?.includes('admin') || false, + is_active: true, + created_on: '', + }); navigate('/'); } }; diff --git a/services/dns-webui/src/services/api.ts b/services/dns-webui/src/services/api.ts index 3e037a71..e096f78c 100644 --- a/services/dns-webui/src/services/api.ts +++ b/services/dns-webui/src/services/api.ts @@ -14,29 +14,56 @@ import type { BlockedQuery, } from '../types/api'; +// Double-submit CSRF cookie/header pair. This is NOT the auth token: it is +// a JS-readable random value the server also stores in the (non-HttpOnly) +// csrf_token cookie, and must be echoed back in a header on state-changing +// requests. It defends the cookie-auth flow against CSRF; it grants no +// access on its own. See manager/backend/app/services/cookie_auth.py. +const CSRF_COOKIE_NAME = 'csrf_token'; +const CSRF_HEADER_NAME = 'X-CSRF-Token'; +const MUTATING_METHODS = new Set(['post', 'put', 'patch', 'delete']); + +function readCookie(name: string): string | null { + const escaped = name.replace(/([.$?*|{}()[\]\\/+^])/g, '\\$1'); + const match = document.cookie.match(new RegExp(`(?:^|; )${escaped}=([^;]*)`)); + return match ? decodeURIComponent(match[1]) : null; +} + const api = axios.create({ baseURL: import.meta.env.VITE_API_URL || '', headers: { 'Content-Type': 'application/json' }, + // The access/refresh JWTs live in HttpOnly cookies set by the server -- + // never in localStorage or any other JS-readable storage (CWE-522). + // withCredentials sends those cookies automatically and lets the + // browser store the Set-Cookie response from login/refresh; the token + // value itself is never read or attached by this client. + withCredentials: true, }); -// Request interceptor: add Bearer token +// Request interceptor: attach the double-submit CSRF header on +// state-changing requests. GETs are side-effect-free and skip it. api.interceptors.request.use((config: InternalAxiosRequestConfig) => { - const token = localStorage.getItem('access_token'); - if (token && config.headers) { - config.headers.Authorization = `Bearer ${token}`; + const method = (config.method || 'get').toLowerCase(); + if (MUTATING_METHODS.has(method)) { + const csrfToken = readCookie(CSRF_COOKIE_NAME); + if (csrfToken && config.headers) { + config.headers[CSRF_HEADER_NAME] = csrfToken; + } } return config; }); -// Response interceptor: handle 401 with token refresh +// Response interceptor: handle 401 by refreshing via the HttpOnly +// refresh_token cookie -- the browser attaches it automatically, so there +// is no token for this client to read, store, or forward manually. let isRefreshing = false; let failedQueue: Array<{ - resolve: (token: string) => void; + resolve: () => void; reject: (error: unknown) => void; }> = []; -const processQueue = (error: unknown, token: string | null = null) => { - failedQueue.forEach((p) => (error ? p.reject(error) : p.resolve(token!))); +const processQueue = (error: unknown) => { + failedQueue.forEach((p) => (error ? p.reject(error) : p.resolve())); failedQueue = []; }; @@ -47,46 +74,36 @@ api.interceptors.response.use( _retry?: boolean; }; - if (error.response?.status === 401 && !original._retry) { + const isRefreshCall = original.url?.includes('/auth/refresh'); + const isAuthCheckCall = original.url?.includes('/auth/me'); + + if (error.response?.status === 401 && !original._retry && !isRefreshCall) { if (isRefreshing) { - return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { failedQueue.push({ resolve, reject }); - }).then((token) => { - if (original.headers) - original.headers.Authorization = `Bearer ${token}`; - return api(original); - }); + }).then(() => api(original)); } original._retry = true; isRefreshing = true; - const refreshToken = localStorage.getItem('refresh_token'); - if (!refreshToken) { - localStorage.removeItem('access_token'); - localStorage.removeItem('refresh_token'); - window.location.href = '/login'; - return Promise.reject(error); - } - try { - // Flask-JWT-Extended expects refresh token in Authorization header - const { data } = await axios.post( - (import.meta.env.VITE_API_URL || '') + '/api/v1/auth/refresh', - {}, - { headers: { Authorization: `Bearer ${refreshToken}` } }, - ); - const newToken = data.access_token; - localStorage.setItem('access_token', newToken); - if (original.headers) - original.headers.Authorization = `Bearer ${newToken}`; - processQueue(null, newToken); + // refresh_token cookie is sent automatically (scoped to + // /api/v1/auth); the request interceptor above attaches the CSRF + // header since this is a POST. + await api.post('/api/v1/auth/refresh', {}); + processQueue(null); return api(original); } catch (refreshError) { - processQueue(refreshError, null); - localStorage.removeItem('access_token'); - localStorage.removeItem('refresh_token'); - window.location.href = '/login'; + processQueue(refreshError); + // Don't hard-navigate for the silent "am I logged in" probe + // (useAuth.checkAuth -> /auth/me) -- a logged-out visitor on + // /login is expected to 401 here, and redirecting to the page + // it's already on would reload-loop. Its caller already handles + // the rejection by setting isAuthenticated=false. + if (!isAuthCheckCall) { + window.location.href = '/login'; + } return Promise.reject(refreshError); } finally { isRefreshing = false;