WIP: SEP-990 conformance test improvements (review feedback + validation)#146
Closed
WIP: SEP-990 conformance test improvements (review feedback + validation)#146
Conversation
- Delete unused separate token-exchange and jwt-bearer scenarios, keeping only the complete e2e flow (review comment) - Add missing required token exchange params per SEP-990 spec: requested_token_type, audience, resource (review comment) - Use ctx.idp_client_id for token exchange client_id instead of AS client_id (review comment) - Client discovers resource and auth server via PRM metadata instead of receiving auth_server_url via context (review comment) - Server IdP handler verifies all required token exchange params with detailed error messages (review comment) - Add resource, client_id, jti claims to ID-JAG per SEP-990 spec - Verify ID-JAG typ header (oauth-id-jag+jwt) in JWT bearer handler - Remove auth_server_url from context schema
Server-side (AS) now verifies: - client_secret_basic authentication on JWT bearer grant - ID-JAG typ header is oauth-id-jag+jwt - ID-JAG client_id claim matches the authenticating client (Section 5.1) - ID-JAG resource claim matches the MCP server resource identifier - Client credentials provided via context (client_secret) Server-side (IdP) now: - Sets ID-JAG client_id to the MCP Client's AS client_id (not the IdP client_id), per Section 6.1 Example client now: - Authenticates to AS via client_secret_basic (Authorization: Basic) instead of sending client_id in body - Checks AS metadata grant_types_supported includes jwt-bearer before attempting the flow
- Add shared MockTokenVerifier between AS and MCP server so the MCP server only accepts tokens actually issued by the auth server, matching the pattern used by all other auth scenarios - Remove private_key_jwt from tokenEndpointAuthMethodsSupported since the handler only implements client_secret_basic
commit: |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft PR for reviewing the diff of changes on top of #110.
Changes on top of PR #110
Address PR review comments (
daf77b8)requested_token_type,audience,resourceidp_client_idfor token exchange, not ASclient_idauth_server_urlvia contextAdd client auth and ID-JAG validation (
f1778ee)client_secret_basic(Authorization: Basic) per Section 5typheader isoauth-id-jag+jwtclient_idmatches authenticating client (Section 5.1)resourcematches MCP server resourcegrant_types_supportedincludesjwt-bearerclient_idto AS client_id (not IdP client_id) per Section 6.1Bug fixes (
406ee27)MockTokenVerifierbetween AS and MCP server (matches all other auth scenarios)private_key_jwtfrom advertised auth methods (onlyclient_secret_basicis implemented)Related: #110