diff --git a/main.py b/main.py index 3421587..2c46d08 100644 --- a/main.py +++ b/main.py @@ -38,6 +38,7 @@ AUTHORIZED_USERS = { class TokenValidationRequest(BaseModel): id_token: str access_token: Optional[str] = None + nonce: Optional[str] = None class TokenRefreshRequest(BaseModel): refresh_token: str @@ -204,8 +205,13 @@ def refresh_token(refresh_token: str): except Exception as e: raise ValueError(f"Token refresh failed: {str(e)}") -def verify_token(id_token: str): - """Verify JWT token and return payload if valid""" +def verify_token(id_token: str, expected_nonce: Optional[str] = None): + """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: # Validate input if not id_token or not isinstance(id_token, str): @@ -243,6 +249,14 @@ def verify_token(id_token: str): 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 except jwt.ExpiredSignatureError: raise ValueError("Token has expired") @@ -261,7 +275,7 @@ async def validate_token(request: TokenValidationRequest): try: # Verify token - payload = verify_token(request.id_token) + payload = verify_token(request.id_token, request.nonce) # Extract user email user_email = payload.get("email") diff --git a/templates/index.html b/templates/index.html index dc6f14c..102e0e4 100644 --- a/templates/index.html +++ b/templates/index.html @@ -131,11 +131,26 @@ }, /** - * Generate a nonce for security + * Generate a cryptographically secure nonce for OIDC anti-replay * @returns {string} Nonce string */ 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() { 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('oidc_state', state); // Build authorization URL for implicit flow const authUrl = new URL(CONFIG.issuer + '/authorize'); @@ -208,6 +225,7 @@ authUrl.searchParams.append('response_type', 'id_token token'); authUrl.searchParams.append('scope', CONFIG.scopes); authUrl.searchParams.append('nonce', nonce); + authUrl.searchParams.append('state', state); // Redirect to authorization server window.location.href = authUrl.toString(); @@ -224,6 +242,7 @@ const accessToken = urlParams.get('access_token'); const idToken = urlParams.get('id_token'); const refreshToken = urlParams.get('refresh_token'); + const state = urlParams.get('state'); const error = urlParams.get('error'); if (error) { @@ -232,7 +251,29 @@ 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) { + // 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 window.history.replaceState({}, document.title, window.location.pathname); @@ -244,8 +285,9 @@ localStorage.setItem('refresh_token', refreshToken); } - // Clean up - localStorage.removeItem('nonce'); + // Keep nonce in storage so it can be sent to /validate-token + // (cleared after validation completes in handleValidationResponse) + localStorage.removeItem('oidc_state'); // Get user info from backend after a short delay // Only call getSecretPhrase if we don't already have a recent cached secret @@ -351,6 +393,7 @@ localStorage.removeItem('refresh_token'); localStorage.removeItem('user_data'); localStorage.removeItem('nonce'); + localStorage.removeItem('oidc_state'); localStorage.removeItem('secret_phrase'); localStorage.removeItem('secret_last_fetch'); @@ -448,6 +491,7 @@ const idToken = localStorage.getItem('id_token'); const accessToken = localStorage.getItem('access_token'); + const nonce = localStorage.getItem('nonce'); if (!idToken) { utils.showAlert(elements.secretResult, 'No ID token found. Please log in first.', 'danger'); @@ -464,7 +508,8 @@ }, body: JSON.stringify({ id_token: idToken, - access_token: accessToken + access_token: accessToken, + nonce: nonce || undefined }) }); @@ -476,6 +521,7 @@ // Retry the request with refreshed tokens const refreshedIdToken = localStorage.getItem('id_token'); const refreshedAccessToken = localStorage.getItem('access_token'); + const refreshedNonce = localStorage.getItem('nonce'); const retryResponse = await fetch('/validate-token', { method: 'POST', @@ -484,7 +530,8 @@ }, body: JSON.stringify({ id_token: refreshedIdToken, - access_token: refreshedAccessToken + access_token: refreshedAccessToken, + nonce: refreshedNonce || undefined }) }); @@ -518,6 +565,10 @@ * @param {Object} data - Response data from validation endpoint */ 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) { // Store user info if (data.user) { diff --git a/tests/test_main.py b/tests/test_main.py index ab31eed..5d1a9ee 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -106,6 +106,41 @@ def test_validate_token_invalid(mock_verify_token): assert not data["valid"] 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(): """Test validation with empty token.""" response = client.post("/validate-token", json={ diff --git a/tests/test_token_validation.py b/tests/test_token_validation.py index c6443b2..91fb933 100644 --- a/tests/test_token_validation.py +++ b/tests/test_token_validation.py @@ -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"): 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') def test_get_well_known_config_success(mock_get): """Test successful retrieval of well-known configuration."""