mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 21:26:39 +00:00
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 <willem.jiang@gmail.com>
This commit is contained in:
parent
69004bd52c
commit
300abb36d2
@ -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)
|
||||
|
||||
@ -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")
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user