Add OAuth 2.1 client support for MCP authorization flow
## Motivation and Context
The MCP Authorization spec (2025-11-25) requires clients to support
Protected Resource Metadata discovery (RFC 9728), Authorization
Server Metadata discovery (RFC 8414), Dynamic Client Registration,
and OAuth 2.1 Authorization Code grant with PKCE (S256). The Ruby SDK
previously had no client-side OAuth support: the HTTP transport raised
`RequestHandlerError` on a 401 with no recovery, and 21 `auth/*` scenarios
were listed as known failures in `conformance/expected_failures.yml`.
This aligns the Ruby SDK with the Python and TypeScript SDKs,
both of which expose a pluggable OAuth client provider that drives
the full discovery + DCR + authorization + token exchange flow.
`MCP::Client::OAuth::Provider` mirrors that surface: callers supply
`client_metadata`, a `redirect_uri`, and pluggable `redirect_handler`
/ `callback_handler` hooks plus a token/client information storage.
`MCP::Client::OAuth::InMemoryStorage` is provided as a default.
`MCP::Client::HTTP` accepts a new optional `oauth:` keyword. When present,
the transport attaches `Authorization: Bearer` to every request,
and on a 401 parses `WWW-Authenticate` for the `resource_metadata` URL
and `scope` challenge, drives the discovery + DCR + authorization code flow
through the provider, and retries the failed request once with the acquired token.
Spec compliance details:
- The `WWW-Authenticate` parser handles multiple challenges
(e.g. `Basic ..., Bearer ...` or `Bearer ..., DPoP ...`), extracting only
the parameters that belong to the Bearer challenge.
- The PRM `resource` value is required to cover the MCP server URL
(same scheme/host/port, with the PRM path as a prefix of the server path)
before the flow continues, preventing redirection of issued tokens to a different audience.
- Both authorization and token requests always include a `resource`
parameter (RFC 8707), defaulting to a canonicalized form of
the MCP server URL when PRM does not advertise one.
- `client_secret_basic`, `client_secret_post`, and `none` token endpoint
authentication methods are supported. When pre-registered client information
specifies no `token_endpoint_auth_method`, the transport defaults to
`client_secret_basic` per RFC 6749 §2.3.1 an dthe TS/Python SDK convention.
Pre-registered client credentials supplied via `Storage#save_client_information`
with either string or symbol keys are honored, skipping DCR.
- Scope resolution follows the spec order: the `WWW-Authenticate` challenge scope wins first,
then PRM `scopes_supported`, and `Provider#scope` is the last-resort fallback only
if neither is supplied (the provider-supplied scope must not pre-empt server-advertised scope).
- Token endpoint error responses (RFC 6749 §5.2) are parsed: a 4x with
`error: "invalid_grant"` raises `Flow::InvalidGrantError`,
so `MCP::Client::HTTP` can discard the stored refresh token only when
the AS truly says it is dead. Generic `AuthorizationError`
(5xx, network failures, other error codes) leaves the refresh token
intact, so a transient AS outage does not force interactive reauth
on the next attempt.
- The OAuth `state` parameter is compared via
`OpenSSL.fixed_length_secure_compare` (with a `bytesize` guard) to
prevent timing-based discovery of the expected value.
- The 401 retry guard is a request-scoped local variable, so an MCP server
that keeps returning 401 is surfaced as an error after one
retry rather than triggering an infinite re-authorization loop.
## How Has This Been Tested?
Unit tests cover PKCE generation, multi-challenge `WWW-Authenticate` parsing,
PRM and Authorization Server metadata URL builders, the full Flow orchestrator
(success path, state mismatch, missing PKCE support, skipping DCR for both string
and symbol-keyed preregistered client information, `client_secret_basic`
token endpoint authentication, PRM `resource` mismatch rejection, fallback to
the server URL when PRM omits `resource`, scope resolution preferring PRM
`scopes_supported` over `Provider#scope`, default `client_secret_basic` for
confidential clients without an explicit `token_endpoint_auth_method`,
and the new `InvalidGrantError` vs generic `AuthorizationError` split for
token endpoint rejections), and the HTTP transport behavior
(401 recovery + retry, no-provider fallthrough, no infinite loop on repeated 401,
no infinite loop on flow failure, refresh-token retention on transient 5xx from
the token endpoint).
Conformance: `bundle exec rake conformance` newly passes 16 `auth/*`
scenarios beyond the existing baseline, including
`auth/metadata-default`, `metadata-var{1,2,3}`,
`scope-from-www-authenticate`, `scope-from-scopes-supported`,
`scope-omitted-when-undefined`, `scope-retry-limit`,
`token-endpoint-auth-{basic,post,none}`, `pre-registration`,
`resource-mismatch`, and `offline-access-not-supported`. Remaining
`auth/*` scenarios stay in `expected_failures.yml`.
`bundle exec rake test rubocop` is clean.
## Breaking Changes
None. `MCP::Client::HTTP#initialize` accepts a new optional `oauth:`
keyword that defaults to `nil`, preserving existing behavior. When unset,
the transport raises `MCP::Client::RequestHandlerError` on a 401 exactly as before.
This commit is contained in:
parent
d6b5392831
commit
5766069923
5 changed files with 4600 additions and 70 deletions
467
test/mcp/client/oauth/discovery_test.rb
Normal file
467
test/mcp/client/oauth/discovery_test.rb
Normal file
|
|
@ -0,0 +1,467 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require "test_helper"
|
||||
require "mcp/client/oauth/discovery"
|
||||
|
||||
module MCP
|
||||
class Client
|
||||
module OAuth
|
||||
class DiscoveryTest < Minitest::Test
|
||||
def test_parse_www_authenticate_with_quoted_resource_metadata
|
||||
header = 'Bearer error="invalid_token", error_description="Missing", ' \
|
||||
'resource_metadata="https://srv.example.com/.well-known/oauth-protected-resource/mcp"'
|
||||
params = Discovery.parse_www_authenticate(header)
|
||||
|
||||
assert_equal("invalid_token", params["error"])
|
||||
assert_equal("Missing", params["error_description"])
|
||||
assert_equal("https://srv.example.com/.well-known/oauth-protected-resource/mcp", params["resource_metadata"])
|
||||
end
|
||||
|
||||
def test_parse_www_authenticate_returns_empty_for_other_schemes
|
||||
assert_empty(Discovery.parse_www_authenticate('Basic realm="x"'))
|
||||
end
|
||||
|
||||
def test_parse_www_authenticate_returns_empty_for_nil
|
||||
assert_empty(Discovery.parse_www_authenticate(nil))
|
||||
end
|
||||
|
||||
def test_parse_www_authenticate_finds_bearer_after_other_scheme
|
||||
header = 'Basic realm="x", Bearer error="invalid_token", resource_metadata="https://srv/x"'
|
||||
params = Discovery.parse_www_authenticate(header)
|
||||
|
||||
assert_equal("invalid_token", params["error"])
|
||||
assert_equal("https://srv/x", params["resource_metadata"])
|
||||
end
|
||||
|
||||
def test_parse_www_authenticate_stops_at_following_scheme
|
||||
header = 'Bearer error="invalid_token", resource_metadata="https://srv/x", DPoP algs="ES256"'
|
||||
params = Discovery.parse_www_authenticate(header)
|
||||
|
||||
assert_equal("invalid_token", params["error"])
|
||||
assert_equal("https://srv/x", params["resource_metadata"])
|
||||
refute(params.key?("algs"))
|
||||
end
|
||||
|
||||
def test_parse_www_authenticate_returns_empty_when_bearer_absent
|
||||
assert_empty(Discovery.parse_www_authenticate('DPoP algs="ES256"'))
|
||||
end
|
||||
|
||||
def test_parse_www_authenticate_handles_bare_bearer
|
||||
assert_empty(Discovery.parse_www_authenticate("Bearer"))
|
||||
end
|
||||
|
||||
def test_parse_www_authenticate_handles_tab_separator
|
||||
params = Discovery.parse_www_authenticate(%(Bearer\terror="invalid_token",\tresource_metadata="https://srv/x"))
|
||||
|
||||
assert_equal("invalid_token", params["error"])
|
||||
assert_equal("https://srv/x", params["resource_metadata"])
|
||||
end
|
||||
|
||||
def test_parse_www_authenticate_tolerates_trailing_comma
|
||||
params = Discovery.parse_www_authenticate('Bearer error="invalid_token", resource_metadata="https://srv/x",')
|
||||
|
||||
assert_equal("invalid_token", params["error"])
|
||||
assert_equal("https://srv/x", params["resource_metadata"])
|
||||
end
|
||||
|
||||
def test_parse_www_authenticate_handles_quoted_value_with_comma_and_spaces
|
||||
params = Discovery.parse_www_authenticate('Bearer error_description="missing, expired, or revoked token"')
|
||||
|
||||
assert_equal("missing, expired, or revoked token", params["error_description"])
|
||||
end
|
||||
|
||||
def test_parse_www_authenticate_unescapes_quoted_pair
|
||||
# Per RFC 7230 Section 3.2.6, a backslash inside a quoted-string escapes
|
||||
# the next character. The parser must surface the unescaped form.
|
||||
header = 'Bearer error_description="value with \"quoted\" word and a back\\\\slash"'
|
||||
params = Discovery.parse_www_authenticate(header)
|
||||
|
||||
assert_equal('value with "quoted" word and a back\\slash', params["error_description"])
|
||||
end
|
||||
|
||||
def test_protected_resource_metadata_urls_uses_explicit_url_first
|
||||
urls = Discovery.protected_resource_metadata_urls(
|
||||
server_url: "https://api.example.com/mcp",
|
||||
resource_metadata_url: "https://api.example.com/.well-known/oauth-protected-resource/mcp",
|
||||
)
|
||||
|
||||
assert_equal("https://api.example.com/.well-known/oauth-protected-resource/mcp", urls.first)
|
||||
assert_includes(urls, "https://api.example.com/.well-known/oauth-protected-resource")
|
||||
end
|
||||
|
||||
def test_protected_resource_metadata_urls_falls_back_to_path_then_root
|
||||
urls = Discovery.protected_resource_metadata_urls(server_url: "https://api.example.com/mcp")
|
||||
|
||||
assert_equal(
|
||||
[
|
||||
"https://api.example.com/.well-known/oauth-protected-resource/mcp",
|
||||
"https://api.example.com/.well-known/oauth-protected-resource",
|
||||
],
|
||||
urls,
|
||||
)
|
||||
end
|
||||
|
||||
def test_authorization_server_metadata_urls_for_root_issuer
|
||||
urls = Discovery.authorization_server_metadata_urls("https://auth.example.com")
|
||||
|
||||
assert_equal(
|
||||
[
|
||||
"https://auth.example.com/.well-known/oauth-authorization-server",
|
||||
"https://auth.example.com/.well-known/openid-configuration",
|
||||
],
|
||||
urls,
|
||||
)
|
||||
end
|
||||
|
||||
def test_authorization_server_metadata_urls_for_path_issuer
|
||||
urls = Discovery.authorization_server_metadata_urls("https://auth.example.com/tenant1")
|
||||
|
||||
assert_includes(urls, "https://auth.example.com/.well-known/oauth-authorization-server/tenant1")
|
||||
assert_includes(urls, "https://auth.example.com/.well-known/openid-configuration/tenant1")
|
||||
assert_includes(urls, "https://auth.example.com/tenant1/.well-known/openid-configuration")
|
||||
end
|
||||
|
||||
def test_canonicalize_url_normalizes_scheme_host_port_and_path
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp",
|
||||
Discovery.canonicalize_url("HTTPS://Srv.Example.COM:443/mcp#frag"),
|
||||
)
|
||||
assert_equal(
|
||||
"http://srv.example.com",
|
||||
Discovery.canonicalize_url("http://Srv.Example.COM:80/"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com:8443/mcp",
|
||||
Discovery.canonicalize_url("https://srv.example.com:8443/mcp"),
|
||||
)
|
||||
end
|
||||
|
||||
def test_canonicalize_url_resolves_dot_segments
|
||||
# Critical for audience binding: an attacker-supplied URL like
|
||||
# `/api/../mcp` must collapse to `/mcp` so it cannot pass a PRM check
|
||||
# whose `resource` is the more privileged `/api` path.
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp",
|
||||
Discovery.canonicalize_url("https://srv.example.com/api/../mcp"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/api/mcp",
|
||||
Discovery.canonicalize_url("https://srv.example.com/api/./mcp"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/bar",
|
||||
Discovery.canonicalize_url("https://srv.example.com/foo/../../bar"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com",
|
||||
Discovery.canonicalize_url("https://srv.example.com/api/.."),
|
||||
)
|
||||
end
|
||||
|
||||
def test_canonicalize_url_resolves_percent_encoded_dot_segments
|
||||
# `%2e` and `%2E` are percent-encoded forms of `.` (RFC 3986 Section 6.2.2.2)
|
||||
# and must be decoded *before* dot-segment resolution. Otherwise,
|
||||
# an attacker-supplied URL like `/api/%2e%2e/mcp` would slip past
|
||||
# `remove_dot_segments` and escape the PRM `resource=/api` audience
|
||||
# binding.
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp",
|
||||
Discovery.canonicalize_url("https://srv.example.com/api/%2e%2e/mcp"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp",
|
||||
Discovery.canonicalize_url("https://srv.example.com/api/%2E%2E/mcp"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/api/mcp",
|
||||
Discovery.canonicalize_url("https://srv.example.com/api/%2e/mcp"),
|
||||
)
|
||||
end
|
||||
|
||||
def test_canonicalize_url_drops_userinfo
|
||||
# The canonicalized URL is sent on the wire as the RFC 8707 `resource`
|
||||
# claim and is surfaced in error messages, so credentials in
|
||||
# the authority must never round-trip: dropping `user:pass@` keeps them
|
||||
# from leaking to the authorization server or to log destinations.
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp",
|
||||
Discovery.canonicalize_url("https://user:pass@Srv.Example.COM:443/api/../mcp"),
|
||||
)
|
||||
end
|
||||
|
||||
def test_canonicalize_url_normalizes_query_to_match_faraday
|
||||
# Faraday rewrites `env.url` before sending a request: it sorts
|
||||
# parameters by name, uppercases percent-encoded hex, and drops
|
||||
# a bare `?` with no parameters. The canonical form must apply
|
||||
# the same transformation so the URL guard does not raise a false
|
||||
# positive on every valid request with a query string.
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?a=1&b=2",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?b=2&a=1"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?x=%2F",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?x=%2f"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?"),
|
||||
)
|
||||
end
|
||||
|
||||
def test_canonicalize_url_collapses_same_name_query_parameters
|
||||
# Faraday's internal query encoder uses a Hash, so repeated keys
|
||||
# collapse to the last value. The canonical form must apply
|
||||
# the same collapse, otherwise the URL guard false-positives on URLs
|
||||
# like `?a=1&a=2` (Faraday rewrites to `?a=2`).
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?a=2",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?a=1&a=2"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?a=3&b=2",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?a=1&b=2&a=3"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?x=%2F",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?x=%2f&x=%2F"),
|
||||
)
|
||||
end
|
||||
|
||||
def test_canonicalize_url_preserves_valueless_vs_empty_value_distinction
|
||||
# Faraday preserves the difference between `?key` (no `=`) and
|
||||
# `?key=` (empty value) exactly as the caller passed it. A naive
|
||||
# `URI.decode_www_form` round-trip would collapse both to `?key=`
|
||||
# and false-positive the URL guard / RFC 8707 `resource` check for
|
||||
# any URL that uses the value-less form.
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?tenant",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?tenant"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?tenant=",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?tenant="),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?a=1&b",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?a=1&b"),
|
||||
)
|
||||
# Collapse with last-write-wins also keeps the `=` form of
|
||||
# the surviving segment: `flag=1&flag` -> last is `flag` (no `=`), so
|
||||
# the canonical form is `?flag`, matching Faraday.
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?flag",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?flag=1&flag"),
|
||||
)
|
||||
end
|
||||
|
||||
def test_canonicalize_url_preserves_array_form_repeated_keys
|
||||
# Keys ending in `[]` are Faraday's array notation: the encoder
|
||||
# preserves every occurrence in input order instead of collapsing
|
||||
# them. The canonical form must do the same; otherwise a hijacked
|
||||
# middleware that drops one entry from `?tenant[]=victim&tenant[]=...`
|
||||
# would slip past the URL guard, and a legitimate
|
||||
# `?roles[]=a&roles[]=b` URL would false-positive on the first
|
||||
# request.
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?a%5B%5D=1&a%5B%5D=2",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?a[]=1&a[]=2"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?tenant%5B%5D=evil&tenant%5B%5D=victim",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?tenant[]=evil&tenant[]=victim"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?a%5B%5D=1&a%5B%5D=2&b=3",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?a[]=1&a[]=2&b=3"),
|
||||
)
|
||||
# Nested keys like `a[b]` are not array notation. They collapse with
|
||||
# last-write-wins, matching Faraday's `NestedParamsEncoder`.
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?a%5Bb%5D=2",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?a[b]=1&a[b]=2"),
|
||||
)
|
||||
end
|
||||
|
||||
def test_canonicalize_url_drops_empty_name_and_blank_query_segments
|
||||
# Faraday's encoder treats pairs with an empty name and blank `&&`
|
||||
# separators as no parameter at all. The canonical form must do
|
||||
# the same; otherwise URLs like `?=v` or `?&&a=1&&` produce a snapshot
|
||||
# that disagrees with the Faraday-rewritten effective URL and
|
||||
# the request-time guard false-positives.
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?=v"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?a=1",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?&&a=1&&"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?a=2",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?a=1&&a=2"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp?a=1",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?=&=&a=1"),
|
||||
)
|
||||
assert_equal(
|
||||
"https://srv.example.com/mcp",
|
||||
Discovery.canonicalize_url("https://srv.example.com/mcp?&"),
|
||||
)
|
||||
end
|
||||
|
||||
def test_resource_covers_blocks_dot_segment_bypass_after_canonicalization
|
||||
server = Discovery.canonicalize_url("https://srv.example.com/api/../mcp")
|
||||
prm = Discovery.canonicalize_url("https://srv.example.com/api")
|
||||
|
||||
refute(
|
||||
Discovery.resource_covers?(prm: prm, server: server),
|
||||
"PRM `resource=/api` must not cover `/api/../mcp` after canonicalization",
|
||||
)
|
||||
end
|
||||
|
||||
def test_secure_url_accepts_https
|
||||
assert(Discovery.secure_url?("https://srv.example.com/mcp"))
|
||||
assert(Discovery.secure_url?("HTTPS://Srv.Example.COM/mcp"))
|
||||
end
|
||||
|
||||
def test_secure_url_accepts_loopback_http
|
||||
assert(Discovery.secure_url?("http://localhost/x"))
|
||||
assert(Discovery.secure_url?("http://localhost:8080/x"))
|
||||
assert(Discovery.secure_url?("http://127.0.0.1/x"))
|
||||
assert(Discovery.secure_url?("http://127.42.0.5/x"))
|
||||
assert(Discovery.secure_url?("http://127.255.255.255/x"))
|
||||
assert(Discovery.secure_url?("http://[::1]/x"))
|
||||
end
|
||||
|
||||
def test_secure_url_rejects_non_loopback_http
|
||||
refute(Discovery.secure_url?("http://srv.example.com/mcp"))
|
||||
refute(Discovery.secure_url?("http://192.168.1.1/x"))
|
||||
refute(Discovery.secure_url?("http://10.0.0.1/x"))
|
||||
end
|
||||
|
||||
def test_secure_url_rejects_hostname_tricks_that_resemble_loopback
|
||||
# Naive `start_with?("127.")` would let these slip through; IPAddr-based
|
||||
# checks correctly classify them as non-loopback hostnames.
|
||||
refute(Discovery.secure_url?("http://127.attacker.com/x"))
|
||||
refute(Discovery.secure_url?("http://127.0.0.1.evil.com/x"))
|
||||
refute(Discovery.secure_url?("http://127./x"))
|
||||
# `localhost` MUST be matched exactly - `foo.localhost` is just a regular
|
||||
# hostname that happens to share the suffix.
|
||||
refute(Discovery.secure_url?("http://foo.localhost/x"))
|
||||
end
|
||||
|
||||
def test_secure_url_rejects_non_http_schemes
|
||||
refute(Discovery.secure_url?("ftp://srv.example.com/mcp"))
|
||||
refute(Discovery.secure_url?("file:///etc/passwd"))
|
||||
refute(Discovery.secure_url?("javascript:alert(1)"))
|
||||
refute(Discovery.secure_url?(""))
|
||||
refute(Discovery.secure_url?(nil))
|
||||
end
|
||||
|
||||
def test_secure_url_rejects_malformed_or_hostless_urls
|
||||
refute(Discovery.secure_url?("https:/missing-host"))
|
||||
refute(Discovery.secure_url?("https:///missing-host"))
|
||||
refute(Discovery.secure_url?("https://"))
|
||||
refute(Discovery.secure_url?("not a url"))
|
||||
end
|
||||
|
||||
def test_resource_covers_blocks_percent_encoded_dot_segment_bypass
|
||||
server = Discovery.canonicalize_url("https://srv.example.com/api/%2e%2e/mcp")
|
||||
prm = Discovery.canonicalize_url("https://srv.example.com/api")
|
||||
|
||||
refute(
|
||||
Discovery.resource_covers?(prm: prm, server: server),
|
||||
"PRM `resource=/api` must not cover `/api/%2e%2e/mcp` after canonicalization",
|
||||
)
|
||||
end
|
||||
|
||||
def test_resource_covers_blocks_cross_tenant_query_bypass
|
||||
# An attacker-controlled PRM that advertises a different tenant's
|
||||
# resource URI MUST NOT be treated as covering the server URL. If it
|
||||
# were, the client would mint a token for `tenant=evil` while
|
||||
# actually talking to `tenant=victim`, defeating audience binding.
|
||||
server = Discovery.canonicalize_url("https://srv.example.com/mcp?tenant=victim")
|
||||
prm_evil = Discovery.canonicalize_url("https://srv.example.com/mcp?tenant=evil")
|
||||
|
||||
refute(
|
||||
Discovery.resource_covers?(prm: prm_evil, server: server),
|
||||
"PRM `resource=?tenant=evil` must not cover server `?tenant=victim`",
|
||||
)
|
||||
end
|
||||
|
||||
def test_resource_covers_allows_matching_query
|
||||
server = Discovery.canonicalize_url("https://srv.example.com/mcp?tenant=victim")
|
||||
prm = Discovery.canonicalize_url("https://srv.example.com/mcp?tenant=victim")
|
||||
|
||||
assert(Discovery.resource_covers?(prm: prm, server: server))
|
||||
end
|
||||
|
||||
def test_resource_covers_allows_prm_without_query_to_cover_server_with_query
|
||||
# A query-less PRM acts as a generic identifier over the origin +
|
||||
# path prefix and is allowed to cover server URLs that scope by query.
|
||||
server = Discovery.canonicalize_url("https://srv.example.com/mcp?tenant=1")
|
||||
prm = Discovery.canonicalize_url("https://srv.example.com/mcp")
|
||||
|
||||
assert(Discovery.resource_covers?(prm: prm, server: server))
|
||||
end
|
||||
|
||||
def test_resource_covers_blocks_prm_with_query_when_server_has_none
|
||||
# PRM that names a specific tenant cannot cover an unscoped server
|
||||
# URL; otherwise a hijacked PRM could re-bind the token to a tenant
|
||||
# the user did not select.
|
||||
server = Discovery.canonicalize_url("https://srv.example.com/mcp")
|
||||
prm = Discovery.canonicalize_url("https://srv.example.com/mcp?tenant=evil")
|
||||
|
||||
refute(Discovery.resource_covers?(prm: prm, server: server))
|
||||
end
|
||||
|
||||
def test_resource_covers_does_not_treat_empty_query_as_wildcard
|
||||
# `https://srv.example.com/mcp?` parses with `URI#query == ""`, which
|
||||
# is distinct from "no query at all" (`URI#query == nil`). A naive
|
||||
# `prm_query.nil? || prm_query.empty?` would let an attacker-supplied
|
||||
# PRM with a trailing `?` slip past the cross-tenant audience check.
|
||||
server = Discovery.canonicalize_url("https://srv.example.com/mcp?tenant=victim")
|
||||
prm_empty_query = "https://srv.example.com/mcp?"
|
||||
|
||||
refute(
|
||||
Discovery.resource_covers?(prm: prm_empty_query, server: server),
|
||||
"PRM with a literal empty query (`?`) must not act as a wildcard cover",
|
||||
)
|
||||
end
|
||||
|
||||
def test_resource_covers_returns_true_for_same_origin_prefix
|
||||
assert(Discovery.resource_covers?(
|
||||
prm: "https://srv.example.com",
|
||||
server: "https://srv.example.com/mcp",
|
||||
))
|
||||
assert(Discovery.resource_covers?(
|
||||
prm: "https://srv.example.com/mcp",
|
||||
server: "https://srv.example.com/mcp",
|
||||
))
|
||||
assert(Discovery.resource_covers?(
|
||||
prm: "https://srv.example.com/api",
|
||||
server: "https://srv.example.com/api/v2/mcp",
|
||||
))
|
||||
end
|
||||
|
||||
def test_resource_covers_returns_false_for_different_origin_or_path_mismatch
|
||||
refute(Discovery.resource_covers?(
|
||||
prm: "https://evil.example.com/mcp",
|
||||
server: "https://srv.example.com/mcp",
|
||||
))
|
||||
refute(Discovery.resource_covers?(
|
||||
prm: "https://srv.example.com:8443/mcp",
|
||||
server: "https://srv.example.com/mcp",
|
||||
))
|
||||
# Path "api" must not be considered a prefix of "/apiv2".
|
||||
refute(Discovery.resource_covers?(
|
||||
prm: "https://srv.example.com/api",
|
||||
server: "https://srv.example.com/apiv2",
|
||||
))
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
1318
test/mcp/client/oauth/flow_test.rb
Normal file
1318
test/mcp/client/oauth/flow_test.rb
Normal file
File diff suppressed because it is too large
Load diff
1086
test/mcp/client/oauth/http_oauth_test.rb
Normal file
1086
test/mcp/client/oauth/http_oauth_test.rb
Normal file
File diff suppressed because it is too large
Load diff
34
test/mcp/client/oauth/pkce_test.rb
Normal file
34
test/mcp/client/oauth/pkce_test.rb
Normal file
|
|
@ -0,0 +1,34 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require "test_helper"
|
||||
require "base64"
|
||||
require "digest"
|
||||
require "mcp/client/oauth/pkce"
|
||||
|
||||
module MCP
|
||||
class Client
|
||||
module OAuth
|
||||
class PKCETest < Minitest::Test
|
||||
def test_generate_returns_verifier_and_s256_challenge
|
||||
result = PKCE.generate
|
||||
|
||||
assert_kind_of(String, result[:code_verifier])
|
||||
assert_operator(result[:code_verifier].length, :>=, 43)
|
||||
assert_operator(result[:code_verifier].length, :<=, 128)
|
||||
assert_equal("S256", result[:code_challenge_method])
|
||||
|
||||
expected = Base64.urlsafe_encode64(Digest::SHA256.digest(result[:code_verifier]), padding: false)
|
||||
assert_equal(expected, result[:code_challenge])
|
||||
end
|
||||
|
||||
def test_generate_produces_distinct_values
|
||||
a = PKCE.generate
|
||||
b = PKCE.generate
|
||||
|
||||
refute_equal(a[:code_verifier], b[:code_verifier])
|
||||
refute_equal(a[:code_challenge], b[:code_challenge])
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
89
test/mcp/client/oauth/provider_test.rb
Normal file
89
test/mcp/client/oauth/provider_test.rb
Normal file
|
|
@ -0,0 +1,89 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require "test_helper"
|
||||
require "mcp/client/oauth"
|
||||
|
||||
module MCP
|
||||
class Client
|
||||
module OAuth
|
||||
class ProviderTest < Minitest::Test
|
||||
# Builds a complete Provider argument hash whose `client_metadata`
|
||||
# registers the same `redirect_uri` that is being asserted on.
|
||||
# The registered-URI check in `Provider#initialize` requires the two to
|
||||
# match, so accept-path tests need them paired.
|
||||
def args_for(redirect_uri)
|
||||
{
|
||||
client_metadata: {
|
||||
redirect_uris: [redirect_uri],
|
||||
grant_types: ["authorization_code"],
|
||||
response_types: ["code"],
|
||||
token_endpoint_auth_method: "none",
|
||||
},
|
||||
redirect_uri: redirect_uri,
|
||||
redirect_handler: ->(_url) {},
|
||||
callback_handler: -> { ["code", "state"] },
|
||||
}
|
||||
end
|
||||
|
||||
def test_initialize_accepts_https_redirect_uri
|
||||
provider = Provider.new(**args_for("https://app.example.com/callback"))
|
||||
|
||||
assert_equal("https://app.example.com/callback", provider.redirect_uri)
|
||||
end
|
||||
|
||||
def test_initialize_accepts_loopback_http_redirect_uri
|
||||
["http://localhost/callback", "http://127.0.0.1:3000/cb", "http://[::1]/cb"].each do |uri|
|
||||
provider = Provider.new(**args_for(uri))
|
||||
assert_equal(uri, provider.redirect_uri)
|
||||
end
|
||||
end
|
||||
|
||||
def test_initialize_rejects_non_loopback_http_redirect_uri
|
||||
# Communication Security: a non-loopback `http://` redirect URI would
|
||||
# let an attacker steal the authorization code from a network sniffer,
|
||||
# so the provider must refuse to construct.
|
||||
assert_raises(Provider::InsecureRedirectURIError) do
|
||||
Provider.new(**args_for("http://app.example.com/callback"))
|
||||
end
|
||||
end
|
||||
|
||||
def test_initialize_rejects_non_http_scheme_redirect_uri
|
||||
assert_raises(Provider::InsecureRedirectURIError) do
|
||||
Provider.new(**args_for("ftp://app.example.com/callback"))
|
||||
end
|
||||
end
|
||||
|
||||
def test_initialize_rejects_hostname_tricks_that_resemble_loopback
|
||||
["http://127.attacker.com/cb", "http://127.0.0.1.evil.com/cb", "http://foo.localhost/cb"].each do |uri|
|
||||
assert_raises(Provider::InsecureRedirectURIError, "should reject #{uri}") do
|
||||
Provider.new(**args_for(uri))
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def test_initialize_rejects_redirect_uri_not_listed_in_client_metadata
|
||||
# Per the OAuth Dynamic Client Registration response, the AS will
|
||||
# bind the client_id to a specific set of redirect_uris and reject any
|
||||
# authorization request that uses something else. Detect that mismatch
|
||||
# at Provider construction so the failure surfaces immediately.
|
||||
args = args_for("http://localhost:0/callback")
|
||||
args[:redirect_uri] = "http://localhost:0/different"
|
||||
|
||||
assert_raises(Provider::UnregisteredRedirectURIError) do
|
||||
Provider.new(**args)
|
||||
end
|
||||
end
|
||||
|
||||
def test_initialize_accepts_redirect_uri_with_string_keys_in_client_metadata
|
||||
# `client_metadata` is hand-built by callers, so accept either symbol
|
||||
# or string keys for `redirect_uris`.
|
||||
args = args_for("http://localhost:0/callback")
|
||||
args[:client_metadata] = { "redirect_uris" => ["http://localhost:0/callback"] }
|
||||
|
||||
provider = Provider.new(**args)
|
||||
assert_equal("http://localhost:0/callback", provider.redirect_uri)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
Loading…
Reference in a new issue