Skip to content

Fix OAuth discovery rejecting a trailing-slash issuer or resource - #162

Merged
patvice merged 2 commits into
patvice:mainfrom
lukasedw:oauth-discovery-empty-path-slash-normalization
Oct 8, 2026
Merged

patvice merged 2 commits into
patvice:mainfrom
lukasedw:oauth-discovery-empty-path-slash-normalization

Conversation

@lukasedw

Copy link
Copy Markdown

An MCP server that delegates to Google lists authorization_servers: ["https://accounts.google.com/"], while Google's /.well-known/oauth-authorization-server answers issuer: "https://accounts.google.com", so the exact comparison in validate_server_metadata! rejects it.

On the resource side, OAuthProvider.normalize_url (oauth_provider.rb:18) strips a root / from server_url, so the expected resource is always https://host, and a server that answers resource: "https://host/" is rejected by validate_resource_metadata! the same way.

Both methods now treat an empty path and / as equal (RFC 3986 6.2.3), and nothing else: a different host, port or non-root path is still rejected.

Part of #133 (the issuer trailing slash). The resource prefix match is left out.

@lukasedw

lukasedw commented Oct 1, 2026

Copy link
Copy Markdown
Author

what do you think about this @NielsKSchjoedt @newstler?

Replace the regex-based empty-path rewrite with Ruby's RFC 3986
scheme-based normalization. This also accepts scheme/host case and
explicit default port differences, matching how OAuthProvider already
normalizes the server URL. Non-root trailing slashes are still rejected.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@patvice

patvice commented Oct 8, 2026

Copy link
Copy Markdown
Owner

Thanks @lukasedw, thanks for this change! I pushed a small follow-up commit (85fb5c3) to simplify the comparison.

Ruby's URI#normalize already does scheme-based normalization, so uris_match? is now just URI.parse(a).normalize == URI.parse(b).normalize.

As a bonus, it also accepts scheme/host case and explicit default-port differences (e.g. https://host:443). OAuthProvider.normalize_url already strips those from the expected resource, so they could still have caused false mismatches. Non-root trailing slashes (/tenant1/ vs /tenant1) are still rejected as before. I also added a few unit specs for uris_match?. Your original tests all pass unchanged.

Going to merge this now

@patvice
patvice merged commit 0d39bba into patvice:main Oct 8, 2026
6 checks passed
@lukasedw

lukasedw commented Oct 8, 2026

Copy link
Copy Markdown
Author

@patvice awesome! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants