Let a provider customize the HTTP client the OAuth flow uses
## Motivation and Context `MCP::Client::OAuth::Flow` takes an `http_client_factory:`, but `MCP::Client::HTTP` builds the flow itself and passes none, so an application using `oauth:` could not add middleware to the requests the flow makes: Protected Resource Metadata and authorization server metadata discovery, dynamic client registration, and every token request. The block that customizes the transport's Faraday connection does not help, because that connection is bound to the MCP server URL and carries the `headers:` meant for that server, and the authorization server is a different origin. The TypeScript SDK hands the transport's `fetch` to `auth()` and the Python SDK sends the provider's requests through the transport's httpx client, so both let an application instrument these requests; this SDK did not. (#558) Providers now take `http_client_customizer:`, a callable invoked with the Faraday connection the flow builds for its own requests, next to the other callables a provider already holds. It runs after the SDK's defaults (`Accept: application/json`, no redirect following) and between two middlewares of the SDK's own: `Flow::RequestedURLStamp`, registered before the customizer is invoked, records the URL the flow asked for on the request environment, and `Flow::RequestedOriginGuard`, registered last, sees what the customizer's middleware did to the request after that record. A request that would leave that origin, by following a redirect or rewriting the URL, or that arrives without the record, is refused with `Flow::DestinationMismatchError` before it reaches the adapter. The record lives on the environment rather than in `env.request.context`, which stays the application's to fill or replace. The error is an `ArgumentError` rather than an `AuthorizationError` so neither discovery nor the transport's refresh fallback can absorb a configuration mistake. The boundary resembles the one the Python SDK keeps for its own requests, which follows a redirect only within the origin; this flow follows none itself. `Flow.build_http_client` assembles that connection and `Flow.new(http_client_factory:)` keeps replacing it whole. Following done inside an adapter cannot be seen by any guard, so the documentation keeps that, `Accept-Encoding`, and `raise_error` off the list of things a customizer may add, and notes that a recording middleware sees client credentials and tokens. Fixes #558. ## How Has This Been Tested? New tests in `test/mcp/client/oauth/flow_test.rb`, `test/mcp/client/oauth/http_oauth_test.rb`, and the three provider test files. Against the library before this change, every one of them fails, on the unknown keyword or on the missing reader. ## Breaking Changes None. The customizer is opt-in, and the guard cannot fire on the SDK's own middleware-free connection.
This commit is contained in:
Koichi ITO
parent
f22ce86e97
commit
c9f381a3ee
5 changed files with 856 additions and 26 deletions
|
|
@ -49,6 +49,25 @@ module MCP
|
|||
assert_equal("client_secret_post", provider.client_information["token_endpoint_auth_method"])
|
||||
end
|
||||
|
||||
def test_initialize_keeps_the_http_client_customizer
|
||||
customizer = ->(_faraday) {}
|
||||
provider = ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", http_client_customizer: customizer)
|
||||
|
||||
assert_same(customizer, provider.http_client_customizer)
|
||||
assert_nil(ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret").http_client_customizer)
|
||||
end
|
||||
|
||||
def test_initialize_rejects_a_non_callable_http_client_customizer_before_writing_credentials
|
||||
storage = InMemoryStorage.new
|
||||
|
||||
error = assert_raises(ArgumentError) do
|
||||
ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", storage: storage, http_client_customizer: "recorder")
|
||||
end
|
||||
|
||||
assert_equal("http_client_customizer must respond to call (got String).", error.message)
|
||||
assert_nil(storage.client_information)
|
||||
end
|
||||
|
||||
def test_initialize_rejects_missing_client_id
|
||||
["", " ", nil].each do |value|
|
||||
assert_raises(ClientCredentialsProvider::InvalidCredentialsError, "should reject #{value.inspect}") do
|
||||
|
|
|
|||
|
|
@ -49,6 +49,36 @@ module MCP
|
|||
)
|
||||
end
|
||||
|
||||
def test_initialize_keeps_the_http_client_customizer
|
||||
customizer = ->(_faraday) {}
|
||||
provider = CrossAppAccessProvider.new(
|
||||
client_id: "xaa-client",
|
||||
client_secret: "xaa-secret",
|
||||
assertion_provider: ->(**) { "id-jag" },
|
||||
http_client_customizer: customizer,
|
||||
)
|
||||
|
||||
assert_same(customizer, provider.http_client_customizer)
|
||||
assert_nil(build_provider.http_client_customizer)
|
||||
end
|
||||
|
||||
def test_initialize_rejects_a_non_callable_http_client_customizer_before_writing_credentials
|
||||
storage = InMemoryStorage.new
|
||||
|
||||
error = assert_raises(ArgumentError) do
|
||||
CrossAppAccessProvider.new(
|
||||
client_id: "xaa-client",
|
||||
client_secret: "xaa-secret",
|
||||
assertion_provider: ->(**) { "id-jag" },
|
||||
storage: storage,
|
||||
http_client_customizer: "recorder",
|
||||
)
|
||||
end
|
||||
|
||||
assert_equal("http_client_customizer must respond to call (got String).", error.message)
|
||||
assert_nil(storage.client_information)
|
||||
end
|
||||
|
||||
def test_initialize_rejects_missing_client_id
|
||||
assert_raises(CrossAppAccessProvider::InvalidConfigurationError) do
|
||||
CrossAppAccessProvider.new(
|
||||
|
|
|
|||
|
|
@ -85,12 +85,13 @@ module MCP
|
|||
)
|
||||
end
|
||||
|
||||
def client_credentials_provider(token_endpoint_auth_method: "client_secret_basic", token_request_params: nil)
|
||||
def client_credentials_provider(token_endpoint_auth_method: "client_secret_basic", token_request_params: nil, http_client_customizer: nil)
|
||||
ClientCredentialsProvider.new(
|
||||
client_id: "cc-client",
|
||||
client_secret: "cc-secret",
|
||||
token_endpoint_auth_method: token_endpoint_auth_method,
|
||||
token_request_params: token_request_params,
|
||||
http_client_customizer: http_client_customizer,
|
||||
)
|
||||
end
|
||||
|
||||
|
|
@ -141,6 +142,118 @@ module MCP
|
|||
end
|
||||
end
|
||||
|
||||
# Records `[method, url]` for every request that passes through, standing in for the tracing middleware
|
||||
# an application adds through `http_client_customizer`.
|
||||
class RecordingMiddleware
|
||||
def initialize(app, log)
|
||||
@app = app
|
||||
@log = log
|
||||
end
|
||||
|
||||
def call(env)
|
||||
@log << [env.method, env.url.to_s]
|
||||
@app.call(env)
|
||||
end
|
||||
end
|
||||
|
||||
# Sends the request for `from` to `to` instead, the shape redirect-following middleware leaves behind:
|
||||
# the flow asked for one URL and the adapter is handed another.
|
||||
class URLRewritingMiddleware
|
||||
def initialize(app, from:, to:)
|
||||
@app = app
|
||||
@from = from
|
||||
@to = to
|
||||
end
|
||||
|
||||
def call(env)
|
||||
env.url = URI(@to) if env.url.to_s == @from
|
||||
@app.call(env)
|
||||
end
|
||||
end
|
||||
|
||||
# Replaces the per-request context the way instrumentation middleware may; the guard must not depend on it.
|
||||
class ContextReplacingMiddleware
|
||||
def initialize(app)
|
||||
@app = app
|
||||
end
|
||||
|
||||
def call(env)
|
||||
env.request.context = { tag: "instrumented" }
|
||||
@app.call(env)
|
||||
end
|
||||
end
|
||||
|
||||
# Hands the next middleware an environment rebuilt from Faraday's own members only, dropping whatever
|
||||
# an earlier middleware recorded on the original.
|
||||
class EnvRebuildingMiddleware
|
||||
def initialize(app)
|
||||
@app = app
|
||||
end
|
||||
|
||||
def call(env)
|
||||
@app.call(Faraday::Env.from(env.to_h))
|
||||
end
|
||||
end
|
||||
|
||||
# Follows a `3xx` the way `faraday-follow_redirects` does: duplicates the environment, points it at the `Location`,
|
||||
# and re-enters the stack from its own position.
|
||||
class RedirectFollowingMiddleware
|
||||
def initialize(app)
|
||||
@app = app
|
||||
end
|
||||
|
||||
def call(env)
|
||||
response = @app.call(env)
|
||||
location = response.headers["Location"]
|
||||
return response unless location && (300..399).cover?(response.status)
|
||||
|
||||
redirected = env.dup
|
||||
redirected.url = URI(location)
|
||||
redirected.response = nil
|
||||
@app.call(redirected)
|
||||
end
|
||||
end
|
||||
|
||||
# Replaces the request options wholesale, the way middleware that resets timeouts or contexts might.
|
||||
class RequestOptionsReplacingMiddleware
|
||||
def initialize(app)
|
||||
@app = app
|
||||
end
|
||||
|
||||
def call(env)
|
||||
env.request = Faraday::RequestOptions.new
|
||||
@app.call(env)
|
||||
end
|
||||
end
|
||||
|
||||
# Re-enters the stack once with the same environment after a `5xx`, the way retry middleware does.
|
||||
class RetryingMiddleware
|
||||
def initialize(app)
|
||||
@app = app
|
||||
end
|
||||
|
||||
def call(env)
|
||||
response = @app.call(env)
|
||||
return response unless response.status >= 500
|
||||
|
||||
env.response = nil
|
||||
@app.call(env)
|
||||
end
|
||||
end
|
||||
|
||||
# Records the application's `trace_id` from the per-request context, as a tracing middleware would.
|
||||
class ContextRecordingMiddleware
|
||||
def initialize(app, log)
|
||||
@app = app
|
||||
@log = log
|
||||
end
|
||||
|
||||
def call(env)
|
||||
@log << env.request.context&.dig(:trace_id)
|
||||
@app.call(env)
|
||||
end
|
||||
end
|
||||
|
||||
# Runs the full authorization flow and returns the `scope` query parameter
|
||||
# sent on the authorization request. The caller stubs the AS metadata;
|
||||
# this helper supplies a provider whose `grant_types` and optional pre-set
|
||||
|
|
@ -190,6 +303,278 @@ module MCP
|
|||
end
|
||||
end
|
||||
|
||||
def test_run_sends_every_authorization_server_request_through_the_provider_customizer
|
||||
log = []
|
||||
provider = client_credentials_provider(http_client_customizer: ->(faraday) { faraday.use(RecordingMiddleware, log) })
|
||||
|
||||
result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_equal(:authorized, result)
|
||||
assert_equal([[:get, @prm_url], [:get, @as_metadata_url], [:post, "#{@auth_base}/token"]], log)
|
||||
|
||||
# The SDK's defaults are applied before the customizer runs.
|
||||
assert_requested(:get, @prm_url, headers: { "Accept" => "application/json" })
|
||||
end
|
||||
|
||||
def test_run_refuses_a_customized_connection_that_sends_a_request_off_the_origin
|
||||
stub_request(:get, "https://other.example.com/prm.json").to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]),
|
||||
)
|
||||
provider = client_credentials_provider(
|
||||
http_client_customizer: ->(faraday) { faraday.use(URLRewritingMiddleware, from: @prm_url, to: "https://other.example.com/prm.json") },
|
||||
)
|
||||
|
||||
error = assert_raises(Flow::DestinationMismatchError) do
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
assert_equal(<<~MESSAGE, error.message)
|
||||
Request to \"#{@prm_url}\" would be sent to \"https://other.example.com/prm.json\", on a different origin; \
|
||||
middleware that follows redirects or rewrites URLs is refused.
|
||||
MESSAGE
|
||||
|
||||
# The guard sits before the adapter, so the request never leaves.
|
||||
assert_not_requested(:get, "https://other.example.com/prm.json")
|
||||
assert_not_requested(:post, "#{@auth_base}/token")
|
||||
end
|
||||
|
||||
def test_run_keeps_a_same_origin_rewrite
|
||||
moved_url = "https://srv.example.com/prm-moved.json"
|
||||
stub_request(:get, moved_url).to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]),
|
||||
)
|
||||
provider = client_credentials_provider(
|
||||
http_client_customizer: ->(faraday) { faraday.use(URLRewritingMiddleware, from: @prm_url, to: moved_url) },
|
||||
)
|
||||
|
||||
result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_equal(:authorized, result)
|
||||
assert_requested(:get, moved_url)
|
||||
end
|
||||
|
||||
def test_run_refuses_a_rewritten_request_even_when_middleware_replaces_the_request_context
|
||||
stub_request(:get, "https://other.example.com/prm.json").to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]),
|
||||
)
|
||||
provider = client_credentials_provider(
|
||||
http_client_customizer: ->(faraday) {
|
||||
faraday.use(ContextReplacingMiddleware)
|
||||
faraday.use(URLRewritingMiddleware, from: @prm_url, to: "https://other.example.com/prm.json")
|
||||
},
|
||||
)
|
||||
|
||||
assert_raises(Flow::DestinationMismatchError) do
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
assert_not_requested(:get, "https://other.example.com/prm.json")
|
||||
end
|
||||
|
||||
def test_run_refuses_a_redirect_off_the_origin_followed_by_middleware_placed_ahead_of_the_stamp
|
||||
# `builder.insert(0, ...)` is where middleware that wants to be outermost puts itself, so a follower can
|
||||
# land ahead of the SDK's stamp and re-enter the stack; the record of the first URL must survive that.
|
||||
stub_request(:get, @prm_url).to_return(status: 302, headers: { "Location" => "https://other.example.com/prm.json" })
|
||||
stub_request(:get, "https://other.example.com/prm.json").to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]),
|
||||
)
|
||||
provider = client_credentials_provider(
|
||||
http_client_customizer: ->(faraday) { faraday.builder.insert(0, RedirectFollowingMiddleware) },
|
||||
)
|
||||
|
||||
error = assert_raises(Flow::DestinationMismatchError) do
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
assert_match(/would be sent to "https:\/\/other\.example\.com\/prm\.json", on a different origin/, error.message)
|
||||
assert_not_requested(:get, "https://other.example.com/prm.json")
|
||||
end
|
||||
|
||||
def test_run_keeps_a_same_origin_redirect_followed_by_middleware_placed_ahead_of_the_stamp
|
||||
moved_url = "https://srv.example.com/prm-moved.json"
|
||||
stub_request(:get, @prm_url).to_return(status: 302, headers: { "Location" => moved_url })
|
||||
stub_request(:get, moved_url).to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]),
|
||||
)
|
||||
provider = client_credentials_provider(
|
||||
http_client_customizer: ->(faraday) { faraday.builder.insert(0, RedirectFollowingMiddleware) },
|
||||
)
|
||||
|
||||
result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_equal(:authorized, result)
|
||||
assert_requested(:get, moved_url)
|
||||
end
|
||||
|
||||
def test_run_refuses_a_request_whose_environment_was_rebuilt
|
||||
provider = client_credentials_provider(http_client_customizer: ->(faraday) { faraday.use(EnvRebuildingMiddleware) })
|
||||
|
||||
error = assert_raises(Flow::DestinationMismatchError) do
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
assert_equal(<<~MESSAGE, error.message)
|
||||
Request to \"#{@prm_url}\" carries no record of the URL the flow asked for; \
|
||||
middleware that rebuilds the request environment is refused.
|
||||
MESSAGE
|
||||
assert_not_requested(:get, @prm_url)
|
||||
end
|
||||
|
||||
def test_run_refuses_a_rewritten_token_request
|
||||
stub_request(:post, "https://other.example.com/token").to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(access_token: "stolen", token_type: "Bearer"),
|
||||
)
|
||||
provider = client_credentials_provider(
|
||||
http_client_customizer: ->(faraday) {
|
||||
faraday.use(URLRewritingMiddleware, from: "#{@auth_base}/token", to: "https://other.example.com/token")
|
||||
},
|
||||
)
|
||||
|
||||
assert_raises(Flow::DestinationMismatchError) do
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
assert_not_requested(:post, "https://other.example.com/token")
|
||||
assert_not_requested(:post, "#{@auth_base}/token")
|
||||
assert_nil(provider.access_token)
|
||||
end
|
||||
|
||||
def test_run_keeps_the_application_request_context_for_every_request
|
||||
trace_ids = []
|
||||
|
||||
result = run_authorization_flow(
|
||||
http_client_customizer: ->(faraday) {
|
||||
faraday.options.context = { trace_id: "incident-123" }
|
||||
faraday.use(ContextRecordingMiddleware, trace_ids)
|
||||
},
|
||||
)
|
||||
|
||||
assert_equal(:authorized, result)
|
||||
# Protected Resource Metadata and authorization server metadata (GET), registration (JSON POST),
|
||||
# and the token exchange (form POST) all carry it.
|
||||
assert_equal(["incident-123"] * 4, trace_ids)
|
||||
end
|
||||
|
||||
def test_run_keeps_the_application_request_context_on_a_factory_connection
|
||||
trace_ids = []
|
||||
factory = -> {
|
||||
Faraday.new do |faraday|
|
||||
faraday.options.context = { trace_id: "incident-123" }
|
||||
faraday.use(ContextRecordingMiddleware, trace_ids)
|
||||
end
|
||||
}
|
||||
|
||||
result = Flow.new(provider: client_credentials_provider, http_client_factory: factory).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_equal(:authorized, result)
|
||||
assert_equal(["incident-123"] * 3, trace_ids)
|
||||
end
|
||||
|
||||
def test_run_refuses_a_rewritten_request_even_when_middleware_replaces_the_request_options
|
||||
stub_request(:get, "https://other.example.com/prm.json").to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]),
|
||||
)
|
||||
provider = client_credentials_provider(
|
||||
http_client_customizer: ->(faraday) {
|
||||
faraday.use(RequestOptionsReplacingMiddleware)
|
||||
faraday.use(URLRewritingMiddleware, from: @prm_url, to: "https://other.example.com/prm.json")
|
||||
},
|
||||
)
|
||||
|
||||
error = assert_raises(Flow::DestinationMismatchError) do
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
# The mismatch, not a lost record: replacing the options must leave the record in place.
|
||||
assert_match(/would be sent to "https:\/\/other\.example\.com\/prm\.json", on a different origin/, error.message)
|
||||
assert_not_requested(:get, "https://other.example.com/prm.json")
|
||||
end
|
||||
|
||||
def test_run_refuses_a_redirect_off_the_origin_followed_by_middleware_placed_after_the_stamp
|
||||
stub_request(:get, @prm_url).to_return(status: 302, headers: { "Location" => "https://other.example.com/prm.json" })
|
||||
stub_request(:get, "https://other.example.com/prm.json").to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]),
|
||||
)
|
||||
provider = client_credentials_provider(http_client_customizer: ->(faraday) { faraday.use(RedirectFollowingMiddleware) })
|
||||
|
||||
error = assert_raises(Flow::DestinationMismatchError) do
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
# The mismatch, not a lost record: the duplicate the follower re-enters with shares the record.
|
||||
assert_match(/would be sent to "https:\/\/other\.example\.com\/prm\.json", on a different origin/, error.message)
|
||||
assert_not_requested(:get, "https://other.example.com/prm.json")
|
||||
end
|
||||
|
||||
def test_run_keeps_a_same_origin_redirect_followed_by_middleware_placed_after_the_stamp
|
||||
moved_url = "https://srv.example.com/prm-moved.json"
|
||||
stub_request(:get, @prm_url).to_return(status: 302, headers: { "Location" => moved_url })
|
||||
stub_request(:get, moved_url).to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]),
|
||||
)
|
||||
provider = client_credentials_provider(http_client_customizer: ->(faraday) { faraday.use(RedirectFollowingMiddleware) })
|
||||
|
||||
result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_equal(:authorized, result)
|
||||
assert_requested(:get, moved_url)
|
||||
end
|
||||
|
||||
def test_run_keeps_a_retry_by_middleware_placed_ahead_of_the_stamp
|
||||
# The same environment re-enters the stack with the same URL, so the first record still matches.
|
||||
stub_request(:get, @prm_url).to_return(
|
||||
{ status: 503 },
|
||||
{
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]),
|
||||
},
|
||||
)
|
||||
provider = client_credentials_provider(http_client_customizer: ->(faraday) { faraday.builder.insert(0, RetryingMiddleware) })
|
||||
|
||||
result = Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_equal(:authorized, result)
|
||||
assert_requested(:get, @prm_url, times: 2)
|
||||
end
|
||||
|
||||
def test_build_http_client_records_each_request_on_its_own
|
||||
# A caller using the SDK-built connection directly may address different origins from one request
|
||||
# to the next; each request gets its own record.
|
||||
stub_request(:get, "https://a.example.com/one").to_return(status: 200, body: "one")
|
||||
stub_request(:get, "https://b.example.com/two").to_return(status: 200, body: "two")
|
||||
connection = Flow.build_http_client
|
||||
|
||||
assert_equal("one", connection.get("https://a.example.com/one").body)
|
||||
assert_equal("two", connection.get("https://b.example.com/two").body)
|
||||
end
|
||||
|
||||
def test_run_prefers_an_explicit_http_client_factory_over_the_customizer
|
||||
provider = client_credentials_provider(http_client_customizer: ->(_faraday) { raise "the customizer must not run" })
|
||||
|
||||
result = Flow.new(provider: provider, http_client_factory: -> { Faraday.new }).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_equal(:authorized, result)
|
||||
end
|
||||
|
||||
def test_run_client_credentials_with_client_secret_post_sends_credentials_in_body
|
||||
# `client_secret_post` puts the credentials in the form body rather
|
||||
# than an HTTP Basic header (RFC 6749 Section 2.3.1).
|
||||
|
|
@ -586,7 +971,7 @@ module MCP
|
|||
# the Dynamic Client Registration request body. The default loopback redirect URI
|
||||
# exercises SEP-837's `"native"` inference; passing an HTTPS `redirect_uri` exercises
|
||||
# the `"web"` inference.
|
||||
def run_authorization_flow(redirect_uri: "http://localhost:0/callback", client_metadata_extra: {})
|
||||
def run_authorization_flow(redirect_uri: "http://localhost:0/callback", client_metadata_extra: {}, http_client_customizer: nil)
|
||||
state_holder = {}
|
||||
provider = Provider.new(
|
||||
client_metadata: {
|
||||
|
|
@ -598,6 +983,7 @@ module MCP
|
|||
redirect_uri: redirect_uri,
|
||||
redirect_handler: ->(url) { state_holder[:state] = URI.decode_www_form(url.query).to_h.fetch("state") },
|
||||
callback_handler: -> { ["test-auth-code", state_holder[:state]] },
|
||||
http_client_customizer: http_client_customizer,
|
||||
)
|
||||
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
|
|
|||
|
|
@ -11,6 +11,46 @@ module MCP
|
|||
class Client
|
||||
module OAuth
|
||||
class HTTPOAuthTest < Minitest::Test
|
||||
# Records `[method, url]` for every request that passes through, standing in for the tracing
|
||||
# middleware an application adds through `http_client_customizer`.
|
||||
class RecordingMiddleware
|
||||
def initialize(app, log)
|
||||
@app = app
|
||||
@log = log
|
||||
end
|
||||
|
||||
def call(env)
|
||||
@log << [env.method, env.url.to_s]
|
||||
@app.call(env)
|
||||
end
|
||||
end
|
||||
|
||||
# Sends the request for `from` to `to` instead, the shape redirect-following middleware leaves behind.
|
||||
class URLRewritingMiddleware
|
||||
def initialize(app, from:, to:)
|
||||
@app = app
|
||||
@from = from
|
||||
@to = to
|
||||
end
|
||||
|
||||
def call(env)
|
||||
env.url = URI(@to) if env.url.to_s == @from
|
||||
@app.call(env)
|
||||
end
|
||||
end
|
||||
|
||||
# Hands the next middleware an environment rebuilt from Faraday's own members only, dropping whatever
|
||||
# an earlier middleware recorded on the original.
|
||||
class EnvRebuildingMiddleware
|
||||
def initialize(app)
|
||||
@app = app
|
||||
end
|
||||
|
||||
def call(env)
|
||||
@app.call(Faraday::Env.from(env.to_h))
|
||||
end
|
||||
end
|
||||
|
||||
def setup
|
||||
WebMock.enable!
|
||||
@mcp_url = "https://srv.example.com/mcp"
|
||||
|
|
@ -101,6 +141,91 @@ module MCP
|
|||
assert_equal("test-token-after-flow", provider.access_token)
|
||||
end
|
||||
|
||||
def test_send_request_runs_the_oauth_flow_through_the_provider_customizer
|
||||
stub_request(:post, @mcp_url).with { |req|
|
||||
req.headers["Authorization"].nil?
|
||||
}.to_return(
|
||||
status: 401,
|
||||
headers: {
|
||||
"WWW-Authenticate" => %(Bearer error="invalid_token", resource_metadata="#{@prm_url}"),
|
||||
},
|
||||
body: "",
|
||||
)
|
||||
|
||||
stub_request(:post, @mcp_url).with(
|
||||
headers: { "Authorization" => "Bearer test-token-after-flow" }
|
||||
).to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(jsonrpc: "2.0", id: "1", result: { ok: true }),
|
||||
)
|
||||
|
||||
stub_request(:get, @prm_url).to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]),
|
||||
)
|
||||
|
||||
stub_request(:get, "#{@auth_base}/.well-known/oauth-authorization-server").to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(
|
||||
issuer: @auth_base,
|
||||
authorization_endpoint: "#{@auth_base}/authorize",
|
||||
token_endpoint: "#{@auth_base}/token",
|
||||
registration_endpoint: "#{@auth_base}/register",
|
||||
response_types_supported: ["code"],
|
||||
grant_types_supported: ["authorization_code"],
|
||||
code_challenge_methods_supported: ["S256"],
|
||||
token_endpoint_auth_methods_supported: ["none"],
|
||||
),
|
||||
)
|
||||
|
||||
stub_request(:post, "#{@auth_base}/register").to_return(
|
||||
status: 201,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(client_id: "test-client"),
|
||||
)
|
||||
|
||||
stub_request(:post, "#{@auth_base}/token").to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(access_token: "test-token-after-flow", token_type: "Bearer", expires_in: 3600),
|
||||
)
|
||||
|
||||
log = []
|
||||
state_holder = {}
|
||||
provider = Provider.new(
|
||||
client_metadata: {
|
||||
client_name: "ruby-sdk-test",
|
||||
redirect_uris: ["http://localhost:0/callback"],
|
||||
grant_types: ["authorization_code"],
|
||||
response_types: ["code"],
|
||||
token_endpoint_auth_method: "none",
|
||||
},
|
||||
redirect_uri: "http://localhost:0/callback",
|
||||
redirect_handler: ->(url) {
|
||||
state_holder[:state] = URI.decode_www_form(url.query).to_h.fetch("state")
|
||||
},
|
||||
callback_handler: -> { ["test-auth-code", state_holder[:state]] },
|
||||
http_client_customizer: ->(faraday) { faraday.use(RecordingMiddleware, log) },
|
||||
)
|
||||
|
||||
transport = HTTP.new(url: @mcp_url, oauth: provider)
|
||||
response = transport.send_request(request: { jsonrpc: "2.0", id: "1", method: "tools/list" })
|
||||
|
||||
assert_equal({ "ok" => true }, response["result"])
|
||||
assert_equal(
|
||||
[
|
||||
[:get, @prm_url],
|
||||
[:get, "#{@auth_base}/.well-known/oauth-authorization-server"],
|
||||
[:post, "#{@auth_base}/register"],
|
||||
[:post, "#{@auth_base}/token"],
|
||||
],
|
||||
log,
|
||||
)
|
||||
end
|
||||
|
||||
def test_send_request_does_not_follow_a_resource_metadata_challenge_off_the_server_origin
|
||||
# End to end over the transport, which is where the header is actually parsed:
|
||||
# a server that answers 401 must not be able to name an unrelated host in
|
||||
|
|
@ -678,6 +803,94 @@ module MCP
|
|||
assert_equal("saved-rt", provider.tokens["refresh_token"])
|
||||
end
|
||||
|
||||
def test_send_request_surfaces_a_refused_refresh_without_falling_back
|
||||
# A customizer middleware that would carry the refresh request off the origin is refused before
|
||||
# the request leaves, and that refusal is not one of the failures that start the interactive flow
|
||||
# or discard the stored tokens.
|
||||
stub_request(:post, @mcp_url).to_return(
|
||||
status: 401,
|
||||
headers: { "WWW-Authenticate" => %(Bearer error="invalid_token", resource_metadata="#{@prm_url}") },
|
||||
body: "",
|
||||
)
|
||||
|
||||
stub_request(:get, @prm_url).to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(resource: "https://srv.example.com/mcp", authorization_servers: [@auth_base]),
|
||||
)
|
||||
|
||||
stub_request(:get, "#{@auth_base}/.well-known/oauth-authorization-server").to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(
|
||||
issuer: @auth_base,
|
||||
authorization_endpoint: "#{@auth_base}/authorize",
|
||||
token_endpoint: "#{@auth_base}/token",
|
||||
registration_endpoint: "#{@auth_base}/register",
|
||||
code_challenge_methods_supported: ["S256"],
|
||||
token_endpoint_auth_methods_supported: ["none"],
|
||||
),
|
||||
)
|
||||
|
||||
stub_request(:post, "#{@auth_base}/register").to_raise(StandardError.new("DCR should not be called."))
|
||||
stub_request(:post, "https://other.example.com/token").to_raise(StandardError.new("the rewritten request must not leave."))
|
||||
|
||||
provider = build_provider(
|
||||
http_client_customizer: ->(faraday) {
|
||||
faraday.use(URLRewritingMiddleware, from: "#{@auth_base}/token", to: "https://other.example.com/token")
|
||||
},
|
||||
)
|
||||
provider.save_client_information("client_id" => "test-client")
|
||||
provider.save_tokens("access_token" => "stale-token", "refresh_token" => "saved-rt")
|
||||
|
||||
transport = HTTP.new(url: @mcp_url, oauth: provider)
|
||||
|
||||
assert_raises(MCP::Client::OAuth::Flow::DestinationMismatchError) do
|
||||
transport.send_request(request: { jsonrpc: "2.0", id: "1", method: "tools/list" })
|
||||
end
|
||||
|
||||
assert_not_requested(:post, "https://other.example.com/token")
|
||||
assert_not_requested(:post, "#{@auth_base}/token")
|
||||
assert_not_requested(:post, "#{@auth_base}/register")
|
||||
assert_equal("saved-rt", provider.tokens["refresh_token"])
|
||||
end
|
||||
|
||||
def test_send_request_surfaces_a_refresh_whose_environment_was_rebuilt_without_falling_back
|
||||
# The missing-record refusal takes the same route as the origin mismatch: out of `send_request`,
|
||||
# with no interactive fallback and the stored tokens intact.
|
||||
stub_request(:post, @mcp_url).to_return(
|
||||
status: 401,
|
||||
headers: { "WWW-Authenticate" => %(Bearer error="invalid_token", resource_metadata="#{@prm_url}") },
|
||||
body: "",
|
||||
)
|
||||
stub_request(:get, @prm_url).to_raise(StandardError.new("the rebuilt request must not leave."))
|
||||
stub_request(:post, "#{@auth_base}/register").to_raise(StandardError.new("DCR should not be called."))
|
||||
|
||||
interactive_flow_started = false
|
||||
provider = Provider.new(
|
||||
client_metadata: { redirect_uris: ["http://localhost:0/callback"] },
|
||||
redirect_uri: "http://localhost:0/callback",
|
||||
redirect_handler: ->(_url) { interactive_flow_started = true },
|
||||
callback_handler: -> { ["code", "state"] },
|
||||
http_client_customizer: ->(faraday) { faraday.use(EnvRebuildingMiddleware) },
|
||||
)
|
||||
provider.save_client_information("client_id" => "test-client")
|
||||
provider.save_tokens("access_token" => "stale-token", "refresh_token" => "saved-rt")
|
||||
|
||||
transport = HTTP.new(url: @mcp_url, oauth: provider)
|
||||
|
||||
error = assert_raises(MCP::Client::OAuth::Flow::DestinationMismatchError) do
|
||||
transport.send_request(request: { jsonrpc: "2.0", id: "1", method: "tools/list" })
|
||||
end
|
||||
|
||||
assert_match(/carries no record of the URL the flow asked for/, error.message)
|
||||
refute(interactive_flow_started, "the refusal must not fall back to the interactive flow")
|
||||
assert_not_requested(:get, @prm_url)
|
||||
assert_not_requested(:post, "#{@auth_base}/token")
|
||||
assert_not_requested(:post, "#{@auth_base}/register")
|
||||
assert_equal({ "access_token" => "stale-token", "refresh_token" => "saved-rt" }, provider.tokens)
|
||||
end
|
||||
|
||||
def test_send_request_preserves_refresh_token_when_refresh_hits_a_transient_failure
|
||||
# A 5xx (or any non-`invalid_grant`) from the token endpoint indicates
|
||||
# a transient AS outage, NOT that the refresh token is dead.
|
||||
|
|
@ -1656,12 +1869,13 @@ module MCP
|
|||
provider
|
||||
end
|
||||
|
||||
def build_provider
|
||||
def build_provider(http_client_customizer: nil)
|
||||
Provider.new(
|
||||
client_metadata: { redirect_uris: ["http://localhost:0/callback"] },
|
||||
redirect_uri: "http://localhost:0/callback",
|
||||
redirect_handler: ->(_url) {},
|
||||
callback_handler: -> { ["code", "state"] },
|
||||
http_client_customizer: http_client_customizer,
|
||||
)
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -38,6 +38,26 @@ module MCP
|
|||
end
|
||||
end
|
||||
|
||||
def test_initialize_defaults_http_client_customizer_to_nil
|
||||
assert_nil(Provider.new(**args_for("https://app.example.com/callback")).http_client_customizer)
|
||||
end
|
||||
|
||||
def test_initialize_keeps_the_http_client_customizer
|
||||
customizer = ->(_faraday) {}
|
||||
provider = Provider.new(**args_for("https://app.example.com/callback"), http_client_customizer: customizer)
|
||||
|
||||
assert_same(customizer, provider.http_client_customizer)
|
||||
end
|
||||
|
||||
def test_initialize_rejects_a_non_callable_http_client_customizer
|
||||
# A connection object in place of a callable is the likely mistake.
|
||||
error = assert_raises(ArgumentError) do
|
||||
Provider.new(**args_for("https://app.example.com/callback"), http_client_customizer: Object.new)
|
||||
end
|
||||
|
||||
assert_equal("http_client_customizer must respond to call (got Object).", error.message)
|
||||
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,
|
||||
|
|
|
|||
Loading…
Reference in a new issue