From b560ee260cd4f20607d78f65dd645d3a8d32ba0d Mon Sep 17 00:00:00 2001 From: Scarriffle Date: Wed, 9 Sep 2026 18:33:19 +0200 Subject: [PATCH] fix(oidc): Nonce-Pruefung selbst kontrollieren statt Authlib zu ueberlassen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Authlib wertet "Aufrufer hat eine Nonce uebergeben, das Token traegt keine" als Mismatch. Das ist fuer den erneuerten Token der mobilen Clients falsch: ein per Refresh-Token geholtes ID-Token laesst die nonce-Claim regulaer weg, womit der Hintergrund-Refresh immer an einem 401 gescheitert waere. Die Nonce wird daher nicht mehr an Authlib durchgereicht, sondern hier geprueft: vorhandene Nonce muss passen, und der Browser-Flow verlangt sie zusaetzlich zwingend (require_nonce) — dort erzeugen wir sie selbst, also muss der Provider sie zurueckspiegeln. Signatur und uebrige Claims bleiben unveraendert bei Authlib. Zwei Tests dazu: erneuertes Token ohne Nonce wird angenommen, Browser-Flow ohne Nonce abgelehnt. Co-Authored-By: Claude Opus 5 --- backend/oidc_client.py | 18 ++++++++++++++++-- backend/routers/oidc_router.py | 2 ++ backend/tests/test_oidc.py | 23 +++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/backend/oidc_client.py b/backend/oidc_client.py index d45d589..6e496ce 100644 --- a/backend/oidc_client.py +++ b/backend/oidc_client.py @@ -208,9 +208,17 @@ def _verify_at_hash(claims, access_token: str) -> None: def validate_id_token(provider: OIDCProvider, id_token: str, *, expected_audience: str, nonce: Optional[str] = None, + require_nonce: bool = False, access_token: Optional[str] = None) -> dict: """Verify an ID token's signature and claims. Returns the claim dict. + ``nonce`` is the value the client bound into the flow. When + ``require_nonce`` is set the token must carry a matching one — that is the + browser flow, where we generated the nonce ourselves and the provider is + obliged to echo it. Native clients also pass a nonce, but a token they + renewed with a refresh token legitimately no longer carries the claim, so + there the check applies only if the claim is present. + Raises :class:`OIDCError` with a stable slug on any failure. """ issuers = accepted_issuers(provider) @@ -225,7 +233,10 @@ def validate_id_token(provider: OIDCProvider, id_token: str, *, "exp": {"essential": True}, "sub": {"essential": True}, }, - claims_params={"nonce": nonce, "client_id": expected_audience, + # Nonce deliberately omitted: Authlib treats "caller supplied a + # nonce but the token has none" as a mismatch, which breaks + # refreshed tokens. The policy is enforced explicitly below. + claims_params={"client_id": expected_audience, "access_token": access_token}, ) @@ -268,7 +279,10 @@ def validate_id_token(provider: OIDCProvider, id_token: str, *, if not claims.get("sub"): raise OIDCError("oidc_no_subject") - if nonce is not None and claims.get("nonce") != nonce: + token_nonce = claims.get("nonce") + if require_nonce and not token_nonce: + raise OIDCError("oidc_nonce_missing") + if nonce is not None and token_nonce is not None and token_nonce != nonce: raise OIDCError("oidc_nonce_mismatch") if access_token: diff --git a/backend/routers/oidc_router.py b/backend/routers/oidc_router.py index 722d2c7..292a92a 100644 --- a/backend/routers/oidc_router.py +++ b/backend/routers/oidc_router.py @@ -208,6 +208,8 @@ def oidc_callback(provider_key: str, request: Request, provider, tokens["id_token"], expected_audience=provider.client_id, nonce=payload.get("n"), + # We generated this nonce; the provider must echo it back. + require_nonce=True, access_token=tokens.get("access_token"), ) if not claims.get("email"): diff --git a/backend/tests/test_oidc.py b/backend/tests/test_oidc.py index 9e2611f..551e466 100644 --- a/backend/tests/test_oidc.py +++ b/backend/tests/test_oidc.py @@ -374,6 +374,29 @@ def test_exchange_rejects_nonce_mismatch(client, authentik): assert r.status_code == 401 +def test_refreshed_token_without_nonce_is_accepted(client, authentik): + """A token renewed via the refresh token drops the nonce claim; the app + still sends the nonce it remembers, and that must not be a mismatch.""" + _seed_identity(client) + r = exchange(client, nonce="original-nonce", + id_token=make_id_token(aud=MOBILE_CLIENT, nonce=None)) + assert r.status_code == 200, r.text + + +def test_browser_flow_requires_a_nonce_in_the_token(client, authentik): + """We generate the nonce for the browser flow, so the provider must echo + it — a token without one is refused there even though /exchange allows it.""" + import oidc_config + from oidc_client import OIDCError + + provider = oidc_config.get_provider("authentik") + with pytest.raises(OIDCError) as e: + oidc_client.validate_id_token( + provider, make_id_token(nonce=None), + expected_audience=WEB_CLIENT, nonce="expected", require_nonce=True) + assert e.value.slug == "oidc_nonce_missing" + + def test_exchange_requires_nonce_when_token_carries_one(client, authentik): _seed_identity(client) r = exchange(client, id_token=make_id_token(aud=MOBILE_CLIENT, nonce="from-appauth"))