From 300abb36d2095682501daa5c3cb3e4393d9afc2b Mon Sep 17 00:00:00 2001 From: yetuge <182123929+yetuge@users.noreply.github.com> Date: Thu, 1 Oct 2026 07:43:16 +0800 Subject: [PATCH] fix(auth): reject a non-ASCII ID token nonce instead of raising (#6115) * fix(auth): reject a non-ASCII ID token nonce instead of raising The nonce comparison in validate_id_token still used the local secrets.compare_digest on str operands, which raises TypeError when either side holds non-ASCII characters. The nonce claim is provider-controlled text, so such a provider response turned the callback's sso_failed redirect (OIDCValidationError) into an unhandled 500. Route it through the shared constant_time_equals helper #6076 introduced for the same five compare sites, and drop the now-unused local helper. Signed-off-by: yetuge <2219677952@qq.com> * fix(auth): reject a non-string ID token nonce claim like a mismatch A provider returning a truthy non-string nonce claim (e.g. an int) slipped past the missing-claim guard and crashed the comparison helper with AttributeError, which the callback turned into a 500 instead of the sso_failed redirect. Guard the claim type before comparing so every malformed nonce takes the same OIDCValidationError path. Signed-off-by: yetuge <2219677952@qq.com> --------- Signed-off-by: yetuge <2219677952@qq.com> Co-authored-by: Willem Jiang --- backend/app/gateway/auth/oidc.py | 19 ++++++---- backend/tests/test_oidc_auth.py | 65 ++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 7 deletions(-) diff --git a/backend/app/gateway/auth/oidc.py b/backend/app/gateway/auth/oidc.py index 744c558f25..cd2cb5ff3d 100644 --- a/backend/app/gateway/auth/oidc.py +++ b/backend/app/gateway/auth/oidc.py @@ -7,7 +7,6 @@ generation, token exchange, ID token validation, and userinfo retrieval. from __future__ import annotations import logging -import secrets import time from dataclasses import dataclass from typing import Any @@ -17,6 +16,8 @@ import httpx import jwt from jwt import PyJWK +from app.gateway.utils import constant_time_equals + logger = logging.getLogger(__name__) # ── Data types ──────────────────────────────────────────────────────────── @@ -328,7 +329,16 @@ class OIDCService: token_nonce = claims.get("nonce") if not token_nonce: raise OIDCValidationError("ID token is missing the nonce claim") - if not _constant_time_compare(nonce, token_nonce): + # A non-string claim is malformed the same way: comparing it would + # raise AttributeError inside the shared helper, so reject it here + # to keep the sso_failed redirect contract. + if not isinstance(token_nonce, str): + raise OIDCValidationError("ID token nonce claim is not a string") + # The nonce claim is provider-controlled text; the shared comparison + # encodes before comparing so non-ASCII content rejects instead of + # raising TypeError (which the callback turns into a 500, not the + # sso_failed redirect an OIDCValidationError produces). + if not constant_time_equals(nonce, token_nonce): raise OIDCValidationError("ID token nonce does not match") return claims @@ -426,8 +436,3 @@ class OIDCService: name=merged.get("name"), claims=merged, ) - - -def _constant_time_compare(a: str, b: str) -> bool: - """Constant-time string comparison.""" - return secrets.compare_digest(a, b) diff --git a/backend/tests/test_oidc_auth.py b/backend/tests/test_oidc_auth.py index 1337977ff4..9d9f9021c8 100644 --- a/backend/tests/test_oidc_auth.py +++ b/backend/tests/test_oidc_auth.py @@ -1,6 +1,9 @@ +import time from unittest.mock import AsyncMock +import jwt import pytest +from cryptography.hazmat.primitives.asymmetric import rsa from fastapi import HTTPException from app.gateway.auth.models import User @@ -435,3 +438,65 @@ def test_oidc_callback_rejects_non_ascii_state_as_mismatch(monkeypatch): assert response.status_code == 403 assert response.json()["detail"] == "OIDC state mismatch" + + +def _metadata() -> OIDCMetadata: + return OIDCMetadata( + issuer="https://issuer.example.com", + authorization_endpoint="https://issuer.example.com/auth", + token_endpoint="https://issuer.example.com/token", + userinfo_endpoint=None, + jwks_uri="https://issuer.example.com/jwks", + ) + + +def _service_with_signing_key() -> tuple[OIDCService, object]: + service = OIDCService() + private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + service._load_jwks = AsyncMock(return_value={}) + service._resolve_signing_key = AsyncMock(return_value=private_key.public_key()) + return service, private_key + + +def _id_token(private_key, nonce: str) -> str: + now = int(time.time()) + return jwt.encode( + {"iss": "https://issuer.example.com", "aud": "deer-flow", "sub": "s1", "exp": now + 300, "iat": now, "nonce": nonce}, + private_key, + algorithm="RS256", + headers={"kid": "k1"}, + ) + + +@pytest.mark.asyncio +async def test_id_token_nonce_mismatch_with_non_ascii_is_rejected_not_a_type_error(): + """The nonce claim is provider-controlled text, so a mismatch must reject + with OIDCValidationError; the raw str comparison raised TypeError instead, + turning the callback's sso_failed redirect into a 500 (#6076 fixed the + same five compare sites and left this one behind).""" + service, private_key = _service_with_signing_key() + + with pytest.raises(OIDCValidationError, match="nonce does not match"): + await service.validate_id_token(metadata=_metadata(), client_id="deer-flow", id_token=_id_token(private_key, "nònce-é"), nonce="expected") + + +@pytest.mark.asyncio +async def test_id_token_nonce_match_with_non_ascii_is_accepted(): + """A matching non-ASCII nonce keeps its bytes: equality survives the comparison.""" + service, private_key = _service_with_signing_key() + + claims = await service.validate_id_token(metadata=_metadata(), client_id="deer-flow", id_token=_id_token(private_key, "nònce-é"), nonce="nònce-é") + + assert claims["nonce"] == "nònce-é" + + +@pytest.mark.asyncio +async def test_id_token_nonce_claim_of_non_string_type_is_rejected_not_an_attribute_error(): + """A provider returning a truthy non-string nonce claim (e.g. an int) must + reject with OIDCValidationError like every other malformed nonce; reaching + the comparison helper raised AttributeError on .encode() and turned the + callback's sso_failed redirect into a 500.""" + service, private_key = _service_with_signing_key() + + with pytest.raises(OIDCValidationError, match="nonce claim is not a string"): + await service.validate_id_token(metadata=_metadata(), client_id="deer-flow", id_token=_id_token(private_key, 12345), nonce="expected")