Validate ID tokens against discovered issuer and refreshed tokens

OIDCClient.validate_id_token compared the iss claim against
OIDCConfig.issuer (the discovery URL) instead of the issuer advertised by
the provider metadata. This broke Microsoft Entra ID, whose v2 metadata
advertises the templated issuer
https://login.microsoftonline.com/{tenantid}/v2.0 while tokens carry the
concrete tenant GUID, and it also affected tenant-domain issuers.

- Validate iss against the discovered metadata issuer and substitute
  {tenantid} with the token's tid claim.
- Catch jwt.PyJWTError in the OIDC callback so invalid tokens return 400
  instead of 500 (InvalidIssuerError is not a ValueError).
- Re-enable aud/exp/iat/nbf checks for unsigned (alg: none) ID tokens.
- Validate ID tokens returned by the refresh grant before storing them,
  including nonce continuity when present and sub continuity with the
  session user.
- Document Entra ID support and update tests.

packages/kaya-oidc/tests/test_oidc.py

@@ -2,7 +2,7 @@ import base64
 import unittest
 from dataclasses import replace
 from time import time
-from typing import Mapping, Optional
+from typing import Any, Mapping, Optional
 from urllib.parse import parse_qs, urlparse
 
 import httpx
@@ -24,6 +24,7 @@ class MockOIDCProvider:
         issuer: Optional[str] = None,
         sub: str = 'user123',
         email: str = 'user@example.com',
+        metadata_issuer: Optional[str] = None,
         supports_par: bool = False,
         require_par: bool = False,
         par_error: bool = False,
@@ -32,14 +33,19 @@ class MockOIDCProvider:
         self.public_key = self.private_key.public_key()
         self.kid = 'mock-key'
         self.issuer = issuer or f'https://{name}-oidc.local'
+        self.metadata_issuer = metadata_issuer if metadata_issuer is not None else self.issuer
         self.sub = sub
         self.email = email
         self.nonce = f'{name}-nonce-value'
+        self.token_issuer: Optional[str] = None
+        self.token_tid: Optional[str] = None
+        self.token_sub: Optional[str] = None
+        self.token_expired = False
         self.par_error = par_error
         self.par_requests: list[dict[str, list[str]]] = []
         self.par_auth_headers: list[Optional[str]] = []
         self.discovery = {
-            'issuer': self.issuer,
+            'issuer': self.metadata_issuer,
             'authorization_endpoint': f'{self.issuer}/auth',
             'token_endpoint': f'{self.issuer}/token',
             'userinfo_endpoint': f'{self.issuer}/userinfo',
@@ -64,17 +70,29 @@ class MockOIDCProvider:
             'alg': 'RS256',
         }
 
-    def issue_id_token(self, nonce: str, audience: str, expired: bool = False, wrong_nonce: bool = False) -> str:
+    def issue_id_token(
+        self,
+        nonce: Optional[str],
+        audience: str,
+        expired: bool = False,
+        wrong_nonce: bool = False,
+        issuer: Optional[str] = None,
+        tid: Optional[str] = None,
+        sub: Optional[str] = None,
+    ) -> str:
         now = time()
         exp = now - 3600 if expired else now + 3600
-        payload = {
-            'sub': self.sub,
-            'iss': self.issuer,
+        payload: dict[str, Any] = {
+            'sub': sub if sub is not None else self.sub,
+            'iss': issuer if issuer is not None else self.issuer,
             'aud': audience,
             'iat': now,
             'exp': exp,
-            'nonce': 'wrong-nonce' if wrong_nonce else nonce,
         }
+        if nonce is not None:
+            payload['nonce'] = 'wrong-nonce' if wrong_nonce else nonce
+        if tid is not None:
+            payload['tid'] = tid
         return jwt.encode(
             payload,
             self.private_key,
@@ -82,13 +100,39 @@ class MockOIDCProvider:
             headers={'kid': self.kid},
         )
 
+    def issue_unsigned_id_token(
+        self,
+        nonce: str,
+        audience: str,
+        issuer: Optional[str] = None,
+    ) -> str:
+        payload: dict[str, Any] = {
+            'sub': self.sub,
+            'iss': issuer if issuer is not None else self.issuer,
+            'aud': audience,
+            'iat': time(),
+            'exp': time() + 3600,
+            'nonce': nonce,
+        }
+        return jwt.encode(payload, None, algorithm='none')
+
+    def _issue_configured_token(self, audience: str) -> str:
+        return self.issue_id_token(
+            self.nonce,
+            audience,
+            expired=self.token_expired,
+            issuer=self.token_issuer,
+            tid=self.token_tid,
+            sub=self.token_sub if self.token_sub is not None else self.sub,
+        )
+
     def handle_request(self, request: httpx.Request) -> httpx.Response:
         url = str(request.url)
         path = urlparse(url).path
         # Serve discovery and JWKS for any host so that issuer-mismatch tests can still fetch keys.
-        if path == '/.well-known/openid-configuration':
+        if path.endswith('/.well-known/openid-configuration'):
             return httpx.Response(200, json=self.discovery)
-        if path == '/jwks':
+        if path.endswith('/jwks'):
             return httpx.Response(200, json={'keys': [self._public_key_to_jwk()]})
         if path == '/par':
             if request.method != 'POST':
@@ -111,13 +155,13 @@ class MockOIDCProvider:
                 return httpx.Response(200, json={
                     'access_token': 'new-access-token',
                     'refresh_token': 'new-refresh-token',
-                    'id_token': self.issue_id_token('refreshed-nonce', data.get('client_id', 'client')),
+                    'id_token': self._issue_configured_token(data.get('client_id', 'client')),
                     'token_type': 'Bearer',
                 })
             return httpx.Response(200, json={
                 'access_token': 'mock-access-token',
                 'refresh_token': 'mock-refresh-token',
-                'id_token': self.issue_id_token(self.nonce, data.get('client_id', 'client')),
+                'id_token': self._issue_configured_token(data.get('client_id', 'client')),
                 'token_type': 'Bearer',
             })
         if path == '/userinfo':
@@ -185,17 +229,110 @@ class OIDCClientTest(unittest.TestCase):
 
     @async_test
     async def test_validate_id_token_wrong_issuer(self) -> None:
-        token = self.provider.issue_id_token('nonce-value', 'client')
+        provider = MockOIDCProvider(metadata_issuer='https://other-issuer.local')
+        token = provider.issue_id_token('nonce-value', 'client')
         config = OIDCConfig(
-            issuer='https://wrong-issuer.local',
+            issuer=provider.issuer,
             client_id='client',
             redirect_uri='http://localhost:8000/auth/callback',
-            http_client=self.http_client,
+            http_client=httpx.AsyncClient(transport=MockTransport(provider)),
         )
         client = OIDCClient(config)
         with self.assertRaises(jwt.InvalidIssuerError):
             await client.validate_id_token(token, 'nonce-value')
 
+    @async_test
+    async def test_validate_id_token_uses_discovered_issuer(self) -> None:
+        # The configured issuer is only used as the discovery URL; the ID token
+        # is validated against the issuer advertised by the discovery document.
+        provider = MockOIDCProvider(metadata_issuer='https://canonical-issuer.local')
+        token = provider.issue_id_token('nonce-value', 'client', issuer='https://canonical-issuer.local')
+        config = OIDCConfig(
+            issuer=provider.issuer,
+            client_id='client',
+            redirect_uri='http://localhost:8000/auth/callback',
+            http_client=httpx.AsyncClient(transport=MockTransport(provider)),
+        )
+        client = OIDCClient(config)
+        claims = await client.validate_id_token(token, 'nonce-value')
+        self.assertEqual('user123', claims['sub'])
+
+    @async_test
+    async def test_validate_id_token_entra_style_issuer_template(self) -> None:
+        # Microsoft Entra ID multi-tenant metadata advertises
+        # https://login.microsoftonline.com/{tenantid}/v2.0 and tokens carry
+        # the concrete tenant id in both iss and tid.
+        tenant = '11111111-2222-3333-4444-555555555555'
+        provider = MockOIDCProvider(metadata_issuer='https://mock-oidc.local/{tenantid}/v2.0')
+        token = provider.issue_id_token(
+            'nonce-value',
+            'client',
+            issuer=f'https://mock-oidc.local/{tenant}/v2.0',
+            tid=tenant,
+        )
+        config = OIDCConfig(
+            issuer=f'{provider.issuer}/common/v2.0',
+            client_id='client',
+            redirect_uri='http://localhost:8000/auth/callback',
+            http_client=httpx.AsyncClient(transport=MockTransport(provider)),
+        )
+        client = OIDCClient(config)
+        claims = await client.validate_id_token(token, 'nonce-value')
+        self.assertEqual(tenant, claims['tid'])
+
+    @async_test
+    async def test_validate_id_token_template_with_wrong_tid(self) -> None:
+        tenant = '11111111-2222-3333-4444-555555555555'
+        other_tenant = '99999999-8888-7777-6666-555555555555'
+        provider = MockOIDCProvider(metadata_issuer='https://mock-oidc.local/{tenantid}/v2.0')
+        token = provider.issue_id_token(
+            'nonce-value',
+            'client',
+            issuer=f'https://mock-oidc.local/{tenant}/v2.0',
+            tid=other_tenant,
+        )
+        config = OIDCConfig(
+            issuer=f'{provider.issuer}/common/v2.0',
+            client_id='client',
+            redirect_uri='http://localhost:8000/auth/callback',
+            http_client=httpx.AsyncClient(transport=MockTransport(provider)),
+        )
+        client = OIDCClient(config)
+        with self.assertRaises(jwt.InvalidIssuerError):
+            await client.validate_id_token(token, 'nonce-value')
+
+    @async_test
+    async def test_validate_id_token_template_without_tid(self) -> None:
+        tenant = '11111111-2222-3333-4444-555555555555'
+        provider = MockOIDCProvider(metadata_issuer='https://mock-oidc.local/{tenantid}/v2.0')
+        token = provider.issue_id_token(
+            'nonce-value',
+            'client',
+            issuer=f'https://mock-oidc.local/{tenant}/v2.0',
+        )
+        config = OIDCConfig(
+            issuer=f'{provider.issuer}/common/v2.0',
+            client_id='client',
+            redirect_uri='http://localhost:8000/auth/callback',
+            http_client=httpx.AsyncClient(transport=MockTransport(provider)),
+        )
+        client = OIDCClient(config)
+        with self.assertRaises(jwt.InvalidIssuerError):
+            await client.validate_id_token(token, 'nonce-value')
+
+    @async_test
+    async def test_validate_refreshed_id_token_allows_missing_nonce(self) -> None:
+        token = self.provider.issue_id_token(None, 'client')
+        claims = await self.client.validate_refreshed_id_token(token, 'nonce-value')
+        self.assertEqual('user123', claims['sub'])
+
+    @async_test
+    async def test_validate_refreshed_id_token_wrong_nonce(self) -> None:
+        token = self.provider.issue_id_token('other-nonce', 'client')
+        with self.assertRaises(ValueError) as ctx:
+            await self.client.validate_refreshed_id_token(token, 'nonce-value')
+        self.assertIn('nonce', str(ctx.exception))
+
     @async_test
     async def test_validate_id_token_disallowed_algorithm(self) -> None:
         token = self.provider.issue_id_token('nonce-value', 'client')
@@ -211,6 +348,55 @@ class OIDCClientTest(unittest.TestCase):
             await client.validate_id_token(token, 'nonce-value')
         self.assertIn('RS256', str(ctx.exception))
 
+    @async_test
+    async def test_validate_id_token_unsigned_allowed(self) -> None:
+        token = self.provider.issue_unsigned_id_token('nonce-value', 'client')
+        config = OIDCConfig(
+            issuer=self.provider.issuer,
+            client_id='client',
+            redirect_uri='http://localhost:8000/auth/callback',
+            http_client=self.http_client,
+            allow_unsigned_id_tokens=True,
+        )
+        client = OIDCClient(config)
+        claims = await client.validate_id_token(token, 'nonce-value')
+        self.assertEqual('user123', claims['sub'])
+
+    @async_test
+    async def test_validate_id_token_unsigned_rejected_by_default(self) -> None:
+        token = self.provider.issue_unsigned_id_token('nonce-value', 'client')
+        with self.assertRaises(ValueError) as ctx:
+            await self.client.validate_id_token(token, 'nonce-value')
+        self.assertIn('Unsigned', str(ctx.exception))
+
+    @async_test
+    async def test_validate_id_token_unsigned_wrong_audience(self) -> None:
+        token = self.provider.issue_unsigned_id_token('nonce-value', 'other-client')
+        config = OIDCConfig(
+            issuer=self.provider.issuer,
+            client_id='client',
+            redirect_uri='http://localhost:8000/auth/callback',
+            http_client=self.http_client,
+            allow_unsigned_id_tokens=True,
+        )
+        client = OIDCClient(config)
+        with self.assertRaises(jwt.InvalidAudienceError):
+            await client.validate_id_token(token, 'nonce-value')
+
+    @async_test
+    async def test_validate_id_token_unsigned_wrong_issuer(self) -> None:
+        token = self.provider.issue_unsigned_id_token('nonce-value', 'client', issuer='https://evil-issuer.local')
+        config = OIDCConfig(
+            issuer=self.provider.issuer,
+            client_id='client',
+            redirect_uri='http://localhost:8000/auth/callback',
+            http_client=self.http_client,
+            allow_unsigned_id_tokens=True,
+        )
+        client = OIDCClient(config)
+        with self.assertRaises(jwt.InvalidIssuerError):
+            await client.validate_id_token(token, 'nonce-value')
+
     @async_test
     async def test_fetch_userinfo(self) -> None:
         userinfo = await self.client.fetch_userinfo('access-token')
@@ -498,6 +684,24 @@ class OIDCMixinTest(unittest.TestCase):
             new_token = await oidc.refresh_access_token(ctx.session)
             await ctx.send_str(200, new_token or 'no-token')
 
+        @app.GET('/id-token')
+        @oidc.require_auth
+        async def id_token(ctx: HttpContext) -> None:
+            stored = ctx.session.get(oidc._key('id_token'))
+            await ctx.send_str(200, stored if isinstance(stored, str) else 'none')
+
+    async def _login(self, client: httpx.AsyncClient, provider: MockOIDCProvider) -> None:
+        r = await client.get('/auth/login', follow_redirects=False)
+        self.assertEqual(302, r.status_code)
+        query = parse_qs(urlparse(r.headers['Location']).query)
+        provider.nonce = query['nonce'][0]
+        r = await client.get(
+            '/auth/callback',
+            params={'code': 'mock-code', 'state': query['state'][0]},
+            follow_redirects=False,
+        )
+        self.assertEqual(302, r.status_code)
+
     @async_test
     async def test_login_redirect(self) -> None:
         app, oidc, provider, store = self._build_app()
@@ -543,6 +747,42 @@ class OIDCMixinTest(unittest.TestCase):
             r = await client.get('/auth/callback', params={'code': 'mock-code', 'state': 'wrong'}, follow_redirects=False)
             self.assertEqual(400, r.status_code)
 
+    @async_test
+    async def test_callback_invalid_issuer_returns_400(self) -> None:
+        app, oidc, provider, store = self._build_app()
+        self._setup_routes(app, oidc)
+        transport = httpx.ASGITransport(app=app)
+        async with httpx.AsyncClient(transport=transport, base_url='http://127.0.0.1:80') as client:
+            r = await client.get('/auth/login', follow_redirects=False)
+            query = parse_qs(urlparse(r.headers['Location']).query)
+            provider.nonce = query['nonce'][0]
+            provider.token_issuer = 'https://evil-issuer.local'
+
+            r = await client.get(
+                '/auth/callback',
+                params={'code': 'mock-code', 'state': query['state'][0]},
+                follow_redirects=False,
+            )
+            self.assertEqual(400, r.status_code)
+
+    @async_test
+    async def test_callback_expired_id_token_returns_400(self) -> None:
+        app, oidc, provider, store = self._build_app()
+        self._setup_routes(app, oidc)
+        transport = httpx.ASGITransport(app=app)
+        async with httpx.AsyncClient(transport=transport, base_url='http://127.0.0.1:80') as client:
+            r = await client.get('/auth/login', follow_redirects=False)
+            query = parse_qs(urlparse(r.headers['Location']).query)
+            provider.nonce = query['nonce'][0]
+            provider.token_expired = True
+
+            r = await client.get(
+                '/auth/callback',
+                params={'code': 'mock-code', 'state': query['state'][0]},
+                follow_redirects=False,
+            )
+            self.assertEqual(400, r.status_code)
+
     @async_test
     async def test_logout(self) -> None:
         app, oidc, provider, store = self._build_app()
@@ -580,17 +820,68 @@ class OIDCMixinTest(unittest.TestCase):
         self._setup_routes(app, oidc)
         transport = httpx.ASGITransport(app=app)
         async with httpx.AsyncClient(transport=transport, base_url='http://127.0.0.1:80') as client:
-            r = await client.get('/auth/login', follow_redirects=False)
-            parsed = urlparse(r.headers['Location'])
-            query = parse_qs(parsed.query)
-            state = query['state'][0]
-            provider.nonce = query['nonce'][0]
-            await client.get('/auth/callback', params={'code': 'mock-code', 'state': state}, follow_redirects=False)
+            await self._login(client, provider)
 
             r = await client.get('/refresh')
             self.assertEqual(200, r.status_code)
             self.assertEqual('new-access-token', r.text)
 
+    @async_test
+    async def test_refresh_access_token_rejects_invalid_issuer(self) -> None:
+        app, oidc, provider, store = self._build_app()
+        self._setup_routes(app, oidc)
+        transport = httpx.ASGITransport(app=app)
+        async with httpx.AsyncClient(transport=transport, base_url='http://127.0.0.1:80') as client:
+            await self._login(client, provider)
+            original_id_token = (await client.get('/id-token')).text
+
+            provider.token_issuer = 'https://evil-issuer.local'
+            r = await client.get('/refresh')
+            self.assertEqual(200, r.status_code)
+            self.assertEqual('no-token', r.text)
+
+            # The refreshed ID token was not validated, so nothing was stored.
+            self.assertEqual(original_id_token, (await client.get('/id-token')).text)
+
+    @async_test
+    async def test_refresh_access_token_rejects_wrong_nonce(self) -> None:
+        app, oidc, provider, store = self._build_app()
+        self._setup_routes(app, oidc)
+        transport = httpx.ASGITransport(app=app)
+        async with httpx.AsyncClient(transport=transport, base_url='http://127.0.0.1:80') as client:
+            await self._login(client, provider)
+
+            provider.nonce = 'tampered-nonce'
+            r = await client.get('/refresh')
+            self.assertEqual(200, r.status_code)
+            self.assertEqual('no-token', r.text)
+
+    @async_test
+    async def test_refresh_access_token_rejects_wrong_subject(self) -> None:
+        app, oidc, provider, store = self._build_app()
+        self._setup_routes(app, oidc)
+        transport = httpx.ASGITransport(app=app)
+        async with httpx.AsyncClient(transport=transport, base_url='http://127.0.0.1:80') as client:
+            await self._login(client, provider)
+
+            provider.token_sub = 'other-user'
+            r = await client.get('/refresh')
+            self.assertEqual(200, r.status_code)
+            self.assertEqual('no-token', r.text)
+
+    @async_test
+    async def test_refresh_access_token_expired_id_token(self) -> None:
+        app, oidc, provider, store = self._build_app()
+        self._setup_routes(app, oidc)
+        transport = httpx.ASGITransport(app=app)
+        async with httpx.AsyncClient(transport=transport, base_url='http://127.0.0.1:80') as client:
+            await self._login(client, provider)
+
+            provider.token_expired = True
+            r = await client.get('/refresh')
+            self.assertEqual(200, r.status_code)
+            self.assertEqual('no-token', r.text)
+
     @async_test
     async def test_dependency_applied_automatically(self) -> None:
         # Only pass oidc to KayaApp; SessionMixin should be applied via dependencies.