fix: harden auth with refresh token session binding, logging, and router state
- Add session_id to refresh tokens and enforce it during validation, preventing stolen refresh tokens from being usable without the originating browser session - Set router.isAuthenticated via auth:login event after successful login (previously only set at page load) - Add console.warn logging to WS message parse/handler errors - Improve _refreshPromise error handling in token refresh flow - Document rate limiter in-memory limitation and CSP connect-src same-origin requirement - Add 3 tests for session-bound refresh token validation
This commit is contained in:
@@ -155,10 +155,11 @@ def auth_refresh(_request: Any, body: Any) -> dict[str, Any]:
|
|||||||
"""Handle token refresh.
|
"""Handle token refresh.
|
||||||
|
|
||||||
Validates the refresh token, blacklists it, and issues a new access token.
|
Validates the refresh token, blacklists it, and issues a new access token.
|
||||||
|
Requires refresh_token and session_id in the request body for session binding.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
_request: Unused.
|
_request: Unused.
|
||||||
body: Dict with ``refresh_token``.
|
body: Dict with ``refresh_token`` and ``session_id``.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Dict with new access token, refresh token, user, and permissions.
|
Dict with new access token, refresh token, user, and permissions.
|
||||||
@@ -173,7 +174,8 @@ def auth_refresh(_request: Any, body: Any) -> dict[str, Any]:
|
|||||||
if not refresh_token:
|
if not refresh_token:
|
||||||
raise ValueError("refresh_token is required")
|
raise ValueError("refresh_token is required")
|
||||||
|
|
||||||
payload = validate_token(refresh_token, token_type="refresh")
|
session_id = body.get("session_id")
|
||||||
|
payload = validate_token(refresh_token, token_type="refresh", session_id=session_id)
|
||||||
if payload is None:
|
if payload is None:
|
||||||
raise ValueError("Invalid or expired refresh token")
|
raise ValueError("Invalid or expired refresh token")
|
||||||
|
|
||||||
|
|||||||
+11
-3
@@ -126,11 +126,15 @@ def generate_access_token(
|
|||||||
return jwt.encode(payload, secret, algorithm=algorithm)
|
return jwt.encode(payload, secret, algorithm=algorithm)
|
||||||
|
|
||||||
|
|
||||||
def generate_refresh_token(username: str) -> str:
|
def generate_refresh_token(username: str, session_id: str | None = None) -> str:
|
||||||
"""Generate a new refresh token for *username*.
|
"""Generate a new refresh token for *username*.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
username: The authenticated username.
|
username: The authenticated username.
|
||||||
|
session_id: Session binding ID included in the token payload.
|
||||||
|
When present, the refresh endpoint requires a matching session_id,
|
||||||
|
preventing a stolen refresh token from being usable without the
|
||||||
|
originating browser session.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
JWT refresh token string.
|
JWT refresh token string.
|
||||||
@@ -150,6 +154,8 @@ def generate_refresh_token(username: str) -> str:
|
|||||||
"jti": str(uuid.uuid4()),
|
"jti": str(uuid.uuid4()),
|
||||||
"type": "refresh",
|
"type": "refresh",
|
||||||
}
|
}
|
||||||
|
if session_id:
|
||||||
|
payload["session_id"] = session_id
|
||||||
return jwt.encode(payload, secret, algorithm=algorithm)
|
return jwt.encode(payload, secret, algorithm=algorithm)
|
||||||
|
|
||||||
|
|
||||||
@@ -165,7 +171,7 @@ def generate_tokens(username: str, permissions: dict[str, str]) -> dict[str, str
|
|||||||
"""
|
"""
|
||||||
session_id = secrets.token_urlsafe(16)
|
session_id = secrets.token_urlsafe(16)
|
||||||
access_token = generate_access_token(username, permissions, session_id)
|
access_token = generate_access_token(username, permissions, session_id)
|
||||||
refresh_token = generate_refresh_token(username)
|
refresh_token = generate_refresh_token(username, session_id)
|
||||||
_persist_refresh_token(username, refresh_token)
|
_persist_refresh_token(username, refresh_token)
|
||||||
return {
|
return {
|
||||||
"access_token": access_token,
|
"access_token": access_token,
|
||||||
@@ -397,7 +403,9 @@ class RateLimiter:
|
|||||||
del self.failures[key]
|
del self.failures[key]
|
||||||
|
|
||||||
|
|
||||||
# Global rate limiters
|
# Global rate limiters — in-memory only. Counts reset on daemon restart
|
||||||
|
# (SIGHUP reload, process restart). Acceptable for a single-user appliance
|
||||||
|
# where restarts are rare; brute-force windows briefly reset post-restart.
|
||||||
_login_limiter = RateLimiter(max_attempts=10, window_seconds=300)
|
_login_limiter = RateLimiter(max_attempts=10, window_seconds=300)
|
||||||
_webauthn_limiter = RateLimiter(max_attempts=5, window_seconds=600)
|
_webauthn_limiter = RateLimiter(max_attempts=5, window_seconds=600)
|
||||||
|
|
||||||
|
|||||||
@@ -811,6 +811,30 @@ class TestTokenValidationEdgeCases:
|
|||||||
payload = validate_token(token, "refresh")
|
payload = validate_token(token, "refresh")
|
||||||
assert payload is None
|
assert payload is None
|
||||||
|
|
||||||
|
def test_refresh_token_with_session_id_validates(self) -> None:
|
||||||
|
"""Refresh token with session_id validates when matching session_id provided."""
|
||||||
|
create_user("sessuser", "password123")
|
||||||
|
token = generate_refresh_token("sessuser", session_id="mysession")
|
||||||
|
payload = validate_token(token, "refresh", session_id="mysession")
|
||||||
|
assert payload is not None
|
||||||
|
assert payload["sub"] == "sessuser"
|
||||||
|
assert payload["session_id"] == "mysession"
|
||||||
|
|
||||||
|
def test_refresh_token_with_wrong_session_id_rejected(self) -> None:
|
||||||
|
"""Refresh token with session_id is rejected when wrong session_id provided."""
|
||||||
|
create_user("sessuser", "password123")
|
||||||
|
token = generate_refresh_token("sessuser", session_id="mysession")
|
||||||
|
payload = validate_token(token, "refresh", session_id="wrongsession")
|
||||||
|
assert payload is None
|
||||||
|
|
||||||
|
def test_refresh_token_generate_tokens_includes_session_id(self) -> None:
|
||||||
|
"""generate_tokens produces refresh token with session_id."""
|
||||||
|
create_user("tokuser", "password123")
|
||||||
|
tokens = generate_tokens("tokuser", {"firewall": "rw"})
|
||||||
|
refresh = decode_token(tokens["refresh_token"])
|
||||||
|
assert refresh is not None
|
||||||
|
assert refresh["session_id"] == tokens["session_id"]
|
||||||
|
|
||||||
|
|
||||||
# ═══════════════════════════════════════════════════════════════════════════
|
# ═══════════════════════════════════════════════════════════════════════════
|
||||||
# Multi-user tests (Phase 3)
|
# Multi-user tests (Phase 3)
|
||||||
|
|||||||
@@ -259,6 +259,9 @@ def _log_request_finish(response):
|
|||||||
)
|
)
|
||||||
|
|
||||||
# Content Security Policy — prevent inline script execution and XSS
|
# Content Security Policy — prevent inline script execution and XSS
|
||||||
|
# NOTE: connect-src 'self' is safe because all XHR/fetch/WS calls go through
|
||||||
|
# nginx on the same origin. If WS or API routing ever changes to use a
|
||||||
|
# different host/port directly, the CSP must be updated accordingly.
|
||||||
if "Content-Security-Policy" not in response.headers:
|
if "Content-Security-Policy" not in response.headers:
|
||||||
response.headers["Content-Security-Policy"] = (
|
response.headers["Content-Security-Policy"] = (
|
||||||
"default-src 'self'; "
|
"default-src 'self'; "
|
||||||
|
|||||||
@@ -263,6 +263,14 @@ export async function initApp() {
|
|||||||
render(mainEl, MainContent);
|
render(mainEl, MainContent);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Listen for login events to update router state after auth
|
||||||
|
window.addEventListener('auth:login', () => {
|
||||||
|
router.isAuthenticated = true;
|
||||||
|
if (!router.state.path.startsWith('/login')) {
|
||||||
|
fetchInitialData();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
// Check auth state before connecting WS
|
// Check auth state before connecting WS
|
||||||
const ok = await initAuth();
|
const ok = await initAuth();
|
||||||
if (ok) {
|
if (ok) {
|
||||||
|
|||||||
+34
-31
@@ -83,41 +83,44 @@ let _refreshPromise = null;
|
|||||||
*
|
*
|
||||||
* @returns {Promise<boolean>} ``true`` if refresh succeeded
|
* @returns {Promise<boolean>} ``true`` if refresh succeeded
|
||||||
*/
|
*/
|
||||||
async function tryRefreshToken() {
|
async function tryRefreshToken() {
|
||||||
_refreshPromise = _refreshPromise || (async () => {
|
if (!_refreshPromise) {
|
||||||
try {
|
_refreshPromise = (async () => {
|
||||||
const stored = getStoredAuth();
|
try {
|
||||||
if (!stored.refresh) return false;
|
const stored = getStoredAuth();
|
||||||
|
if (!stored.refresh) return false;
|
||||||
|
|
||||||
const res = await fetch('/api/auth/refresh', {
|
const res = await fetch('/api/auth/refresh', {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
|
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
|
||||||
body: JSON.stringify({ refresh_token: stored.refresh }),
|
body: JSON.stringify({ refresh_token: stored.refresh, session_id: stored.session_id }),
|
||||||
credentials: 'same-origin',
|
credentials: 'same-origin',
|
||||||
});
|
});
|
||||||
if (res.status !== 200) {
|
if (res.status !== 200) {
|
||||||
|
clearAuthTokens();
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const json = await res.json();
|
||||||
|
if (!json.ok || !json.data?.tokens) {
|
||||||
|
clearAuthTokens();
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const tokens = json.data.tokens;
|
||||||
|
setAuthToken(tokens.access_token);
|
||||||
|
sessionStorage.setItem('vw:refresh', tokens.refresh_token);
|
||||||
|
sessionStorage.setItem('vw:access_ttl', String((json.data.access_ttl || 300) * 1000));
|
||||||
|
sessionStorage.setItem('vw:session_id', tokens.session_id);
|
||||||
|
if (json.data.user) {
|
||||||
|
sessionStorage.setItem('vw:user', JSON.stringify(json.data.user));
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
} catch (err) {
|
||||||
|
console.warn('[Auth] Token refresh failed:', err);
|
||||||
clearAuthTokens();
|
clearAuthTokens();
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
const json = await res.json();
|
})();
|
||||||
if (!json.ok || !json.data?.tokens) {
|
}
|
||||||
clearAuthTokens();
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
const tokens = json.data.tokens;
|
|
||||||
setAuthToken(tokens.access_token);
|
|
||||||
sessionStorage.setItem('vw:refresh', tokens.refresh_token);
|
|
||||||
sessionStorage.setItem('vw:access_ttl', String((json.data.access_ttl || 300) * 1000));
|
|
||||||
sessionStorage.setItem('vw:session_id', tokens.session_id);
|
|
||||||
if (json.data.user) {
|
|
||||||
sessionStorage.setItem('vw:user', JSON.stringify(json.data.user));
|
|
||||||
}
|
|
||||||
return true;
|
|
||||||
} catch {
|
|
||||||
clearAuthTokens();
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
})();
|
|
||||||
return _refreshPromise.finally(() => { _refreshPromise = null; });
|
return _refreshPromise.finally(() => { _refreshPromise = null; });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -130,6 +130,13 @@ export function handleLoginSuccess(data, redirectPath = '/dashboard') {
|
|||||||
}
|
}
|
||||||
scheduleTokenRefresh();
|
scheduleTokenRefresh();
|
||||||
}
|
}
|
||||||
|
// Notify app.js that auth is established (used to set router.isAuthenticated)
|
||||||
|
window.dispatchEvent(new CustomEvent('auth:login', {
|
||||||
|
detail: {
|
||||||
|
permissions: sessionStorage.getItem('vw:permissions') ?
|
||||||
|
JSON.parse(sessionStorage.getItem('vw:permissions')) : {},
|
||||||
|
},
|
||||||
|
}));
|
||||||
window.location.hash = redirectPath;
|
window.location.hash = redirectPath;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -91,7 +91,9 @@ function _wsConnect() {
|
|||||||
try {
|
try {
|
||||||
const msg = typeof ev.data === 'string' ? JSON.parse(ev.data) : ev.data;
|
const msg = typeof ev.data === 'string' ? JSON.parse(ev.data) : ev.data;
|
||||||
handleMessage(msg);
|
handleMessage(msg);
|
||||||
} catch (_) {}
|
} catch (err) {
|
||||||
|
console.warn('[WS] Failed to parse message:', err);
|
||||||
|
}
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -124,7 +126,7 @@ function handleMessage(msg) {
|
|||||||
for (const h of _directHandlers) {
|
for (const h of _directHandlers) {
|
||||||
if (h.unsubscribed) continue;
|
if (h.unsubscribed) continue;
|
||||||
if (topics.some(t => h.topics.includes(t) || h.topics.includes('*'))) {
|
if (topics.some(t => h.topics.includes(t) || h.topics.includes('*'))) {
|
||||||
try { h.handler(msg); } catch (_) {}
|
try { h.handler(msg); } catch (err) { console.warn('[WS] Handler error:', err); }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user