fix(oidc): Nonce-Pruefung selbst kontrollieren statt Authlib zu ueberlassen
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
07235fd4c9
commit
b560ee260c
+16
-2
@@ -208,9 +208,17 @@ def _verify_at_hash(claims, access_token: str) -> None:
|
|||||||
def validate_id_token(provider: OIDCProvider, id_token: str, *,
|
def validate_id_token(provider: OIDCProvider, id_token: str, *,
|
||||||
expected_audience: str,
|
expected_audience: str,
|
||||||
nonce: Optional[str] = None,
|
nonce: Optional[str] = None,
|
||||||
|
require_nonce: bool = False,
|
||||||
access_token: Optional[str] = None) -> dict:
|
access_token: Optional[str] = None) -> dict:
|
||||||
"""Verify an ID token's signature and claims. Returns the claim 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.
|
Raises :class:`OIDCError` with a stable slug on any failure.
|
||||||
"""
|
"""
|
||||||
issuers = accepted_issuers(provider)
|
issuers = accepted_issuers(provider)
|
||||||
@@ -225,7 +233,10 @@ def validate_id_token(provider: OIDCProvider, id_token: str, *,
|
|||||||
"exp": {"essential": True},
|
"exp": {"essential": True},
|
||||||
"sub": {"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},
|
"access_token": access_token},
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -268,7 +279,10 @@ def validate_id_token(provider: OIDCProvider, id_token: str, *,
|
|||||||
if not claims.get("sub"):
|
if not claims.get("sub"):
|
||||||
raise OIDCError("oidc_no_subject")
|
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")
|
raise OIDCError("oidc_nonce_mismatch")
|
||||||
|
|
||||||
if access_token:
|
if access_token:
|
||||||
|
|||||||
@@ -208,6 +208,8 @@ def oidc_callback(provider_key: str, request: Request,
|
|||||||
provider, tokens["id_token"],
|
provider, tokens["id_token"],
|
||||||
expected_audience=provider.client_id,
|
expected_audience=provider.client_id,
|
||||||
nonce=payload.get("n"),
|
nonce=payload.get("n"),
|
||||||
|
# We generated this nonce; the provider must echo it back.
|
||||||
|
require_nonce=True,
|
||||||
access_token=tokens.get("access_token"),
|
access_token=tokens.get("access_token"),
|
||||||
)
|
)
|
||||||
if not claims.get("email"):
|
if not claims.get("email"):
|
||||||
|
|||||||
@@ -374,6 +374,29 @@ def test_exchange_rejects_nonce_mismatch(client, authentik):
|
|||||||
assert r.status_code == 401
|
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):
|
def test_exchange_requires_nonce_when_token_carries_one(client, authentik):
|
||||||
_seed_identity(client)
|
_seed_identity(client)
|
||||||
r = exchange(client, id_token=make_id_token(aud=MOBILE_CLIENT, nonce="from-appauth"))
|
r = exchange(client, id_token=make_id_token(aud=MOBILE_CLIENT, nonce="from-appauth"))
|
||||||
|
|||||||
Reference in New Issue
Block a user