fix(oidc): Add state parameter and validate nonce against login CSRF
The OIDC implicit-flow authorize request carried no state parameter and the nonce was generated with weak Math.random() and never validated client- or server-side, enabling login CSRF / forced authentication / replay (AUTHZ-VULN-05). Generate state and nonce with the Web Crypto CSPRNG, send state in the authorize request, and validate both state and the id_token nonce claim in the callback before storing tokens. Forward the nonce to /validate- token so the backend also binds the id_token to the login flow. Fixes the forged-callback fragment attack where a planted id_token with a mismatched nonce was accepted, stored, and used by the SPA.
This commit is contained in:
@@ -38,6 +38,7 @@ AUTHORIZED_USERS = {
|
|||||||
class TokenValidationRequest(BaseModel):
|
class TokenValidationRequest(BaseModel):
|
||||||
id_token: str
|
id_token: str
|
||||||
access_token: Optional[str] = None
|
access_token: Optional[str] = None
|
||||||
|
nonce: Optional[str] = None
|
||||||
|
|
||||||
class TokenRefreshRequest(BaseModel):
|
class TokenRefreshRequest(BaseModel):
|
||||||
refresh_token: str
|
refresh_token: str
|
||||||
@@ -204,8 +205,13 @@ def refresh_token(refresh_token: str):
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
raise ValueError(f"Token refresh failed: {str(e)}")
|
raise ValueError(f"Token refresh failed: {str(e)}")
|
||||||
|
|
||||||
def verify_token(id_token: str):
|
def verify_token(id_token: str, expected_nonce: Optional[str] = None):
|
||||||
"""Verify JWT token and return payload if valid"""
|
"""Verify JWT token and return payload if valid
|
||||||
|
|
||||||
|
When expected_nonce is provided, the id_token's nonce claim must match it.
|
||||||
|
This binds the token to the OIDC login flow that minted it (anti-replay /
|
||||||
|
anti-forced-authentication), complementing the client-side state/nonce checks.
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
# Validate input
|
# Validate input
|
||||||
if not id_token or not isinstance(id_token, str):
|
if not id_token or not isinstance(id_token, str):
|
||||||
@@ -243,6 +249,14 @@ def verify_token(id_token: str):
|
|||||||
issuer=ISSUER
|
issuer=ISSUER
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Validate nonce binding when an expected nonce is supplied
|
||||||
|
if expected_nonce is not None:
|
||||||
|
token_nonce = payload.get("nonce")
|
||||||
|
if not token_nonce:
|
||||||
|
raise ValueError("Token missing nonce claim")
|
||||||
|
if token_nonce != expected_nonce:
|
||||||
|
raise ValueError("Nonce mismatch: token is not bound to this login")
|
||||||
|
|
||||||
return payload
|
return payload
|
||||||
except jwt.ExpiredSignatureError:
|
except jwt.ExpiredSignatureError:
|
||||||
raise ValueError("Token has expired")
|
raise ValueError("Token has expired")
|
||||||
@@ -261,7 +275,7 @@ async def validate_token(request: TokenValidationRequest):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
# Verify token
|
# Verify token
|
||||||
payload = verify_token(request.id_token)
|
payload = verify_token(request.id_token, request.nonce)
|
||||||
|
|
||||||
# Extract user email
|
# Extract user email
|
||||||
user_email = payload.get("email")
|
user_email = payload.get("email")
|
||||||
|
|||||||
+58
-7
@@ -131,11 +131,26 @@
|
|||||||
},
|
},
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Generate a nonce for security
|
* Generate a cryptographically secure nonce for OIDC anti-replay
|
||||||
* @returns {string} Nonce string
|
* @returns {string} Nonce string
|
||||||
*/
|
*/
|
||||||
generateNonce() {
|
generateNonce() {
|
||||||
return Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15);
|
return utils.generateRandomString(32);
|
||||||
|
},
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Decode a JWT payload without verification (client-side nonce check only)
|
||||||
|
* @param {string} token - JWT token
|
||||||
|
* @returns {Object|null} Decoded payload or null if malformed
|
||||||
|
*/
|
||||||
|
decodeJwtPayload(token) {
|
||||||
|
try {
|
||||||
|
const part = token.split('.')[1];
|
||||||
|
const json = atob(part.replace(/-/g, '+').replace(/_/g, '/'));
|
||||||
|
return JSON.parse(json);
|
||||||
|
} catch (e) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
},
|
},
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -197,9 +212,11 @@
|
|||||||
*/
|
*/
|
||||||
async login() {
|
async login() {
|
||||||
const nonce = utils.generateNonce();
|
const nonce = utils.generateNonce();
|
||||||
|
const state = utils.generateRandomString(32);
|
||||||
|
|
||||||
// Store nonce for later use
|
// Store nonce and state for later validation (anti-replay / anti-CSRF)
|
||||||
localStorage.setItem('nonce', nonce);
|
localStorage.setItem('nonce', nonce);
|
||||||
|
localStorage.setItem('oidc_state', state);
|
||||||
|
|
||||||
// Build authorization URL for implicit flow
|
// Build authorization URL for implicit flow
|
||||||
const authUrl = new URL(CONFIG.issuer + '/authorize');
|
const authUrl = new URL(CONFIG.issuer + '/authorize');
|
||||||
@@ -208,6 +225,7 @@
|
|||||||
authUrl.searchParams.append('response_type', 'id_token token');
|
authUrl.searchParams.append('response_type', 'id_token token');
|
||||||
authUrl.searchParams.append('scope', CONFIG.scopes);
|
authUrl.searchParams.append('scope', CONFIG.scopes);
|
||||||
authUrl.searchParams.append('nonce', nonce);
|
authUrl.searchParams.append('nonce', nonce);
|
||||||
|
authUrl.searchParams.append('state', state);
|
||||||
|
|
||||||
// Redirect to authorization server
|
// Redirect to authorization server
|
||||||
window.location.href = authUrl.toString();
|
window.location.href = authUrl.toString();
|
||||||
@@ -224,6 +242,7 @@
|
|||||||
const accessToken = urlParams.get('access_token');
|
const accessToken = urlParams.get('access_token');
|
||||||
const idToken = urlParams.get('id_token');
|
const idToken = urlParams.get('id_token');
|
||||||
const refreshToken = urlParams.get('refresh_token');
|
const refreshToken = urlParams.get('refresh_token');
|
||||||
|
const state = urlParams.get('state');
|
||||||
const error = urlParams.get('error');
|
const error = urlParams.get('error');
|
||||||
|
|
||||||
if (error) {
|
if (error) {
|
||||||
@@ -232,7 +251,29 @@
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Anti-CSRF: validate state binds this callback to a user-initiated login
|
||||||
|
const storedState = localStorage.getItem('oidc_state');
|
||||||
|
if (!state || !storedState || state !== storedState) {
|
||||||
|
alert('Security error: invalid or missing state parameter. Login aborted.');
|
||||||
|
localStorage.removeItem('oidc_state');
|
||||||
|
localStorage.removeItem('nonce');
|
||||||
|
window.history.replaceState({}, document.title, window.location.pathname);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (accessToken && idToken) {
|
if (accessToken && idToken) {
|
||||||
|
// Anti-replay / anti-forced-auth: validate the id_token nonce matches the one we sent
|
||||||
|
const storedNonce = localStorage.getItem('nonce');
|
||||||
|
const payload = utils.decodeJwtPayload(idToken);
|
||||||
|
const tokenNonce = payload ? payload.nonce : null;
|
||||||
|
if (!storedNonce || !tokenNonce || tokenNonce !== storedNonce) {
|
||||||
|
alert('Security error: nonce mismatch. Login aborted to prevent session fixation.');
|
||||||
|
localStorage.removeItem('oidc_state');
|
||||||
|
localStorage.removeItem('nonce');
|
||||||
|
window.history.replaceState({}, document.title, window.location.pathname);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// Clear URL fragment
|
// Clear URL fragment
|
||||||
window.history.replaceState({}, document.title, window.location.pathname);
|
window.history.replaceState({}, document.title, window.location.pathname);
|
||||||
|
|
||||||
@@ -244,8 +285,9 @@
|
|||||||
localStorage.setItem('refresh_token', refreshToken);
|
localStorage.setItem('refresh_token', refreshToken);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Clean up
|
// Keep nonce in storage so it can be sent to /validate-token
|
||||||
localStorage.removeItem('nonce');
|
// (cleared after validation completes in handleValidationResponse)
|
||||||
|
localStorage.removeItem('oidc_state');
|
||||||
|
|
||||||
// Get user info from backend after a short delay
|
// Get user info from backend after a short delay
|
||||||
// Only call getSecretPhrase if we don't already have a recent cached secret
|
// Only call getSecretPhrase if we don't already have a recent cached secret
|
||||||
@@ -351,6 +393,7 @@
|
|||||||
localStorage.removeItem('refresh_token');
|
localStorage.removeItem('refresh_token');
|
||||||
localStorage.removeItem('user_data');
|
localStorage.removeItem('user_data');
|
||||||
localStorage.removeItem('nonce');
|
localStorage.removeItem('nonce');
|
||||||
|
localStorage.removeItem('oidc_state');
|
||||||
localStorage.removeItem('secret_phrase');
|
localStorage.removeItem('secret_phrase');
|
||||||
localStorage.removeItem('secret_last_fetch');
|
localStorage.removeItem('secret_last_fetch');
|
||||||
|
|
||||||
@@ -448,6 +491,7 @@
|
|||||||
|
|
||||||
const idToken = localStorage.getItem('id_token');
|
const idToken = localStorage.getItem('id_token');
|
||||||
const accessToken = localStorage.getItem('access_token');
|
const accessToken = localStorage.getItem('access_token');
|
||||||
|
const nonce = localStorage.getItem('nonce');
|
||||||
|
|
||||||
if (!idToken) {
|
if (!idToken) {
|
||||||
utils.showAlert(elements.secretResult, 'No ID token found. Please log in first.', 'danger');
|
utils.showAlert(elements.secretResult, 'No ID token found. Please log in first.', 'danger');
|
||||||
@@ -464,7 +508,8 @@
|
|||||||
},
|
},
|
||||||
body: JSON.stringify({
|
body: JSON.stringify({
|
||||||
id_token: idToken,
|
id_token: idToken,
|
||||||
access_token: accessToken
|
access_token: accessToken,
|
||||||
|
nonce: nonce || undefined
|
||||||
})
|
})
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -476,6 +521,7 @@
|
|||||||
// Retry the request with refreshed tokens
|
// Retry the request with refreshed tokens
|
||||||
const refreshedIdToken = localStorage.getItem('id_token');
|
const refreshedIdToken = localStorage.getItem('id_token');
|
||||||
const refreshedAccessToken = localStorage.getItem('access_token');
|
const refreshedAccessToken = localStorage.getItem('access_token');
|
||||||
|
const refreshedNonce = localStorage.getItem('nonce');
|
||||||
|
|
||||||
const retryResponse = await fetch('/validate-token', {
|
const retryResponse = await fetch('/validate-token', {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
@@ -484,7 +530,8 @@
|
|||||||
},
|
},
|
||||||
body: JSON.stringify({
|
body: JSON.stringify({
|
||||||
id_token: refreshedIdToken,
|
id_token: refreshedIdToken,
|
||||||
access_token: refreshedAccessToken
|
access_token: refreshedAccessToken,
|
||||||
|
nonce: refreshedNonce || undefined
|
||||||
})
|
})
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -518,6 +565,10 @@
|
|||||||
* @param {Object} data - Response data from validation endpoint
|
* @param {Object} data - Response data from validation endpoint
|
||||||
*/
|
*/
|
||||||
async handleValidationResponse(data) {
|
async handleValidationResponse(data) {
|
||||||
|
// Nonce is single-use for the login flow; clear it now that validation is done
|
||||||
|
localStorage.removeItem('nonce');
|
||||||
|
localStorage.removeItem('oidc_state');
|
||||||
|
|
||||||
if (data.valid) {
|
if (data.valid) {
|
||||||
// Store user info
|
// Store user info
|
||||||
if (data.user) {
|
if (data.user) {
|
||||||
|
|||||||
@@ -106,6 +106,41 @@ def test_validate_token_invalid(mock_verify_token):
|
|||||||
assert not data["valid"]
|
assert not data["valid"]
|
||||||
assert data["error"] == "Invalid token"
|
assert data["error"] == "Invalid token"
|
||||||
|
|
||||||
|
@patch('main.verify_token')
|
||||||
|
def test_validate_token_passes_nonce_to_verify(mock_verify_token):
|
||||||
|
"""The endpoint must forward the supplied nonce to verify_token (anti-replay binding)."""
|
||||||
|
mock_verify_token.return_value = {
|
||||||
|
"email": "rene.luria@infomaniak.com",
|
||||||
|
"iss": "https://login.infomaniak.com",
|
||||||
|
"aud": "test-client-id",
|
||||||
|
"nonce": "abc123",
|
||||||
|
}
|
||||||
|
|
||||||
|
response = client.post("/validate-token", json={
|
||||||
|
"id_token": "valid.token.here",
|
||||||
|
"nonce": "abc123"
|
||||||
|
})
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.json()["valid"]
|
||||||
|
# verify_token must have been called with the nonce as the second positional arg
|
||||||
|
assert mock_verify_token.call_args.args[1] == "abc123"
|
||||||
|
|
||||||
|
@patch('main.verify_token')
|
||||||
|
def test_validate_token_rejects_nonce_mismatch(mock_verify_token):
|
||||||
|
"""A mismatched nonce (forced-auth / replay token) is rejected by the backend."""
|
||||||
|
mock_verify_token.side_effect = ValueError("Nonce mismatch: token is not bound to this login")
|
||||||
|
|
||||||
|
response = client.post("/validate-token", json={
|
||||||
|
"id_token": "forged.token.here",
|
||||||
|
"nonce": "abc123"
|
||||||
|
})
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
data = response.json()
|
||||||
|
assert not data["valid"]
|
||||||
|
assert "Nonce mismatch" in data["error"]
|
||||||
|
|
||||||
def test_validate_token_empty_request():
|
def test_validate_token_empty_request():
|
||||||
"""Test validation with empty token."""
|
"""Test validation with empty token."""
|
||||||
response = client.post("/validate-token", json={
|
response = client.post("/validate-token", json={
|
||||||
|
|||||||
@@ -58,6 +58,99 @@ def test_verify_token_no_matching_key(mock_get_header, mock_get_jwks):
|
|||||||
with pytest.raises(ValueError, match="Unable to find matching key in JWKS"):
|
with pytest.raises(ValueError, match="Unable to find matching key in JWKS"):
|
||||||
verify_token("some.token.string")
|
verify_token("some.token.string")
|
||||||
|
|
||||||
|
|
||||||
|
def _mock_jwks_and_key():
|
||||||
|
"""Helper: return (jwks, header) fixtures for reaching the jwt.decode step."""
|
||||||
|
jwks = {
|
||||||
|
"keys": [
|
||||||
|
{"kid": "test-key-id", "kty": "RSA", "n": "modulus", "e": "AQAB"}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
header = {"kid": "test-key-id", "alg": "RS256"}
|
||||||
|
return jwks, header
|
||||||
|
|
||||||
|
|
||||||
|
@patch('main.jwt.decode')
|
||||||
|
@patch('main.jwt.algorithms.RSAAlgorithm.from_jwk')
|
||||||
|
@patch('main.jwt.get_unverified_header')
|
||||||
|
@patch('main.get_jwks')
|
||||||
|
def test_verify_token_nonce_match(mock_get_jwks, mock_get_header, mock_from_jwk, mock_decode):
|
||||||
|
"""verify_token returns payload when expected_nonce matches the token nonce."""
|
||||||
|
jwks, header = _mock_jwks_and_key()
|
||||||
|
mock_get_jwks.return_value = jwks
|
||||||
|
mock_get_header.return_value = header
|
||||||
|
mock_from_jwk.return_value = "mock-key"
|
||||||
|
mock_decode.return_value = {
|
||||||
|
"iss": "https://login.infomaniak.com",
|
||||||
|
"aud": "test-client-id",
|
||||||
|
"email": "rene.luria@infomaniak.com",
|
||||||
|
"nonce": "abc123",
|
||||||
|
}
|
||||||
|
|
||||||
|
payload = verify_token("some.token.string", expected_nonce="abc123")
|
||||||
|
assert payload["nonce"] == "abc123"
|
||||||
|
|
||||||
|
|
||||||
|
@patch('main.jwt.decode')
|
||||||
|
@patch('main.jwt.algorithms.RSAAlgorithm.from_jwk')
|
||||||
|
@patch('main.jwt.get_unverified_header')
|
||||||
|
@patch('main.get_jwks')
|
||||||
|
def test_verify_token_nonce_mismatch(mock_get_jwks, mock_get_header, mock_from_jwk, mock_decode):
|
||||||
|
"""verify_token rejects an id_token whose nonce does not match the expected one."""
|
||||||
|
jwks, header = _mock_jwks_and_key()
|
||||||
|
mock_get_jwks.return_value = jwks
|
||||||
|
mock_get_header.return_value = header
|
||||||
|
mock_from_jwk.return_value = "mock-key"
|
||||||
|
mock_decode.return_value = {
|
||||||
|
"iss": "https://login.infomaniak.com",
|
||||||
|
"aud": "test-client-id",
|
||||||
|
"email": "attacker@evil.com",
|
||||||
|
"nonce": "VERYWRONG_NONCE",
|
||||||
|
}
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="Nonce mismatch"):
|
||||||
|
verify_token("some.token.string", expected_nonce="abc123")
|
||||||
|
|
||||||
|
|
||||||
|
@patch('main.jwt.decode')
|
||||||
|
@patch('main.jwt.algorithms.RSAAlgorithm.from_jwk')
|
||||||
|
@patch('main.jwt.get_unverified_header')
|
||||||
|
@patch('main.get_jwks')
|
||||||
|
def test_verify_token_nonce_missing_in_token(mock_get_jwks, mock_get_header, mock_from_jwk, mock_decode):
|
||||||
|
"""verify_token rejects a token lacking a nonce claim when an expected nonce is supplied."""
|
||||||
|
jwks, header = _mock_jwks_and_key()
|
||||||
|
mock_get_jwks.return_value = jwks
|
||||||
|
mock_get_header.return_value = header
|
||||||
|
mock_from_jwk.return_value = "mock-key"
|
||||||
|
mock_decode.return_value = {
|
||||||
|
"iss": "https://login.infomaniak.com",
|
||||||
|
"aud": "test-client-id",
|
||||||
|
"email": "rene.luria@infomaniak.com",
|
||||||
|
}
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="Token missing nonce claim"):
|
||||||
|
verify_token("some.token.string", expected_nonce="abc123")
|
||||||
|
|
||||||
|
|
||||||
|
@patch('main.jwt.decode')
|
||||||
|
@patch('main.jwt.algorithms.RSAAlgorithm.from_jwk')
|
||||||
|
@patch('main.jwt.get_unverified_header')
|
||||||
|
@patch('main.get_jwks')
|
||||||
|
def test_verify_token_nonce_not_checked_when_none(mock_get_jwks, mock_get_header, mock_from_jwk, mock_decode):
|
||||||
|
"""verify_token skips the nonce check when no expected_nonce is provided (returning visits / refresh)."""
|
||||||
|
jwks, header = _mock_jwks_and_key()
|
||||||
|
mock_get_jwks.return_value = jwks
|
||||||
|
mock_get_header.return_value = header
|
||||||
|
mock_from_jwk.return_value = "mock-key"
|
||||||
|
mock_decode.return_value = {
|
||||||
|
"iss": "https://login.infomaniak.com",
|
||||||
|
"aud": "test-client-id",
|
||||||
|
"email": "rene.luria@infomaniak.com",
|
||||||
|
}
|
||||||
|
|
||||||
|
payload = verify_token("some.token.string")
|
||||||
|
assert payload["email"] == "rene.luria@infomaniak.com"
|
||||||
|
|
||||||
@patch('main.requests.get')
|
@patch('main.requests.get')
|
||||||
def test_get_well_known_config_success(mock_get):
|
def test_get_well_known_config_success(mock_get):
|
||||||
"""Test successful retrieval of well-known configuration."""
|
"""Test successful retrieval of well-known configuration."""
|
||||||
|
|||||||
Reference in New Issue
Block a user