fix(oidc): require azp == client_id on multi-audience ID tokens (#385)

[EnjoyBacon7]

Summary

services/auth/oidc_client.py only checked client_id in aud when aud was a JSON array. Per OIDC Core 1.0 §3.1.3.7, a token whose aud lists multiple audiences must also contain an azp claim equal to the RP's client_id. Without that check, a token issued by the same IdP for a sibling client that happened to include this client in its audience list was accepted, letting one tenant's tokens authenticate against another tenant's sessions.

This PR adds the azp validation branch in the ID token verifier. (The logout-token verifier is intentionally left alone for now: back-channel logout already requires a matching session id and is a different threat model — happy to apply the same change there if reviewers prefer.)

Test plan

• Single-audience tokens (the common case) continue to validate
• Multi-audience token with azp == client_id validates
• Multi-audience token with missing or mismatched azp is rejected

Fixes #385

Summary by CodeRabbit

• Bug Fixes

  • Strengthened OpenID Connect multi-audience ID token validation by requiring the authorized party claim to match the client ID when multiple audiences are present.

• Tests

  • Added unit tests validating multi-audience ID token rejection and acceptance based on authorized party claim verification.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements OIDC specification compliance by adding multi-audience ID token validation. When an ID token's aud claim contains multiple audiences, the azp claim must now be present and match the client ID, preventing cross-tenant token confusion. The implementation adds validation logic and includes test coverage for both rejection and acceptance scenarios.

Changes

Multi-Audience ID Token Validation

Layer / File(s)	Summary
azp claim validation for multi-audience ID tokens openrag/services/auth/oidc_client.py	_verify_id_token enforces that when aud is a list with more than one entry, the azp claim must exist and match client_id, raising ValueError otherwise.
Multi-audience token validation test cases openrag/components/auth/test_oidc_client.py	Added test_multi_aud_requires_matching_azp to verify rejection of multi-audience tokens when azp does not match client_id, and test_multi_aud_with_correct_azp_passes to verify acceptance when azp correctly matches.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A security fix, both tidy and tight,
Multi-audience tokens now must match just right!
azp claims align with client_id true,
No cross-tenant confusion for me and for you. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding enforcement of azp == client_id validation for multi-audience ID tokens in OIDC, directly addressing issue #385.
Linked Issues check	✅ Passed	The implementation adds azp validation in _verify_id_token for multi-audience tokens and includes comprehensive tests covering rejection and acceptance cases, directly meeting issue #385 requirements.
Out of Scope Changes check	✅ Passed	All changes are scoped to the OIDC ID token verification logic and corresponding tests; no unrelated modifications are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/385-oidc-azp-multi-aud-check

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

openrag/components/auth/test_oidc_client.py (1)

256-279: ⚡ Quick win

Add an explicit missing-azp regression case.

Current negative coverage only verifies mismatched azp. On Line 264’s stated intent, add a variant with multi-audience aud and no azp claim to lock the full requirement.

Suggested test refinement

-    async def test_multi_aud_requires_matching_azp(self, client):
+    `@pytest.mark.parametrize`(
+        "extra_claims",
+        [
+            {"aud": [CLIENT_ID, "other-client"]},  # missing azp
+            {"aud": [CLIENT_ID, "other-client"], "azp": "other-client"},  # mismatched azp
+        ],
+    )
+    async def test_multi_aud_requires_matching_azp(self, client, extra_claims):
         _setup_discovery(client._mock_router)
         _setup_jwks(client._mock_router)

         nonce = "n-azp"
-        # Multi-aud token with the wrong (or missing) azp must be rejected
-        id_token = _sign_jwt(
-            _id_token_payload(nonce, extra={"aud": [CLIENT_ID, "other-client"], "azp": "other-client"})
-        )
+        id_token = _sign_jwt(_id_token_payload(nonce, extra=extra_claims))

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openrag/components/auth/test_oidc_client.py` around lines 256 - 279, Add a
second negative variant to test_multi_aud_requires_matching_azp that covers the
missing azp case: create an id_token via _sign_jwt(_id_token_payload(nonce,
extra={"aud": [CLIENT_ID, "other-client"]})) (omit "azp"), mock the token
endpoint return_value with that token_response as in the existing case, then
await client.exchange_code(code="code", code_verifier="v", expected_nonce=nonce)
inside a pytest.raises(ValueError, match="multi-aud") to assert the
multi-audience-without-azp regression is rejected; reuse the same mocking
pattern and variables (client._mock_router.post(...).mock(...), token_response,
nonce) used in the existing test.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@openrag/components/auth/test_oidc_client.py`:
- Around line 256-279: Add a second negative variant to
test_multi_aud_requires_matching_azp that covers the missing azp case: create an
id_token via _sign_jwt(_id_token_payload(nonce, extra={"aud": [CLIENT_ID,
"other-client"]})) (omit "azp"), mock the token endpoint return_value with that
token_response as in the existing case, then await
client.exchange_code(code="code", code_verifier="v", expected_nonce=nonce)
inside a pytest.raises(ValueError, match="multi-aud") to assert the
multi-audience-without-azp regression is rejected; reuse the same mocking
pattern and variables (client._mock_router.post(...).mock(...), token_response,
nonce) used in the existing test.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5bf19cd0-0ec8-4a04-a13a-a86f86d86069

📥 Commits

Reviewing files that changed from the base of the PR and between 470c30e and 8a73176.

📒 Files selected for processing (2)

• openrag/components/auth/test_oidc_client.py
• openrag/services/auth/oidc_client.py