Let the embedding application refuse an authorization request
## Motivation and Context An MCP server chooses the authorization server its users sign in to. It names one in `authorization_servers` in its Protected Resource Metadata, and states the scopes it wants in `scopes_supported` or in the `WWW-Authenticate` challenge. Nothing binds either choice to the server's own identity. No origin rule can settle this, because an authorization server is legitimately a different origin from the resource it protects. Validating that a token was issued for the intended audience is a responsibility the specification places on MCP servers rather than on clients, and there is no portable way for a client to perform it either: access tokens are frequently opaque, and an authorization server is not required to honor the `resource` parameter or to report an audience in the token response. Two things follow from that, and they are separate. The first is a judgement, and it belongs to whoever embeds this SDK, because an application often knows which providers its users deal with. `authorization_request_validator` is handed an `AuthorizationRequest` describing the authorization server, the scopes, the MCP server, and the RFC 8707 resource, and can refuse. It is one object rather than keyword arguments so that later revisions of the specification can add to it without changing the shape implementers wrote. It runs on all three grants, and on the authorization-code grant it runs before dynamic client registration, which is why scope resolution moved above `ensure_client_registered`: a refusal must not leave this client registered at an authorization server the host just rejected. The second is not a judgement and is enforced here rather than asked about. `refresh!` rediscovers the authorization server on every call, so the server named for a session can differ from the one the stored tokens came from. SEP-2352 already binds client credentials to their issuer, but that check passes when the issuer was never recorded and is deliberately skipped for a Client ID Metadata Document URL, which is portable across authorization servers. The issuer that minted the tokens is now recorded with them, and `refresh!` refuses to present them to a different one. The transport answers that refusal by running a full authorization, which brings the new authorization server to the validator above. Tokens stored before this carry no issuer and keep refreshing. The hook decides whether to proceed, not what to ask for. The scopes reach the authorization server unchanged either way, because the specification requires a client to treat the scopes in the challenge as authoritative for the operation, and narrowing them silently would break that. ## How Has This Been Tested? New tests in `test/mcp/client/oauth/flow_test.rb` assert what the validator receives, that a refusal raises before any registration request is sent and before the redirect handler runs, and that the `client_credentials` and `jwt-bearer` grants consult it too. Others cover the issuer recorded on authorization, the refusal to refresh against a different one, and that tokens stored without an issuer still refresh. A test in `test/mcp/client/oauth/http_oauth_test.rb` drives the two halves together: a stale issuer makes the refresh fail, the transport reauthorizes, and the validator sees the newly named authorization server while no refresh token is ever posted to it. Each of them fails with its half of this change removed. `bundle exec rake` (tests, RuboCop, and conformance baseline) passes. ## Breaking Changes Refreshing now fails when the authorization server named for a session differs from the one that issued the stored tokens, and the caller reauthorizes instead. Tokens stored before this change carry no recorded issuer and are unaffected until their next authorization. The validator itself is additive: it defaults to `nil`, and a provider that does not offer it authorizes as before.
This commit is contained in:
Koichi ITO
parent
da4ce54075
commit
09d4bf41aa
2 changed files with 478 additions and 17 deletions
|
|
@ -363,6 +363,107 @@ module MCP
|
|||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
# Runs the authorization-code flow with an `authorization_request_validator` that records what it
|
||||
# was handed and answers `approve`.
|
||||
private def run_flow_with_validator(approve:, recorder: [])
|
||||
state_holder = {}
|
||||
provider = Provider.new(
|
||||
client_metadata: {
|
||||
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) {
|
||||
recorder << :redirected
|
||||
state_holder[:state] = URI.decode_www_form(url.query).to_h.fetch("state")
|
||||
},
|
||||
callback_handler: -> { ["test-auth-code", state_holder[:state]] },
|
||||
authorization_request_validator: ->(request) {
|
||||
recorder << request
|
||||
approve
|
||||
},
|
||||
)
|
||||
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
def test_run_hands_the_authorization_server_and_scopes_to_the_validator
|
||||
stub_request(:get, @prm_url).to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(
|
||||
resource: @server_url,
|
||||
authorization_servers: [@auth_base],
|
||||
scopes_supported: ["Mail.Read", "Files.ReadWrite.All"],
|
||||
),
|
||||
)
|
||||
|
||||
recorder = []
|
||||
run_flow_with_validator(approve: true, recorder: recorder)
|
||||
|
||||
request = recorder.first
|
||||
|
||||
assert_equal(@auth_base, request.authorization_server)
|
||||
assert_equal(["Mail.Read", "Files.ReadWrite.All"], request.scopes)
|
||||
assert_equal(@server_url, request.server_url)
|
||||
assert_equal("https://srv.example.com/mcp", request.resource)
|
||||
end
|
||||
|
||||
def test_run_refuses_the_flow_and_registers_nothing_when_the_validator_declines
|
||||
recorder = []
|
||||
|
||||
error = assert_raises(Flow::AuthorizationRefusedError) do
|
||||
run_flow_with_validator(approve: false, recorder: recorder)
|
||||
end
|
||||
|
||||
assert_match(/refused by `authorization_request_validator`/, error.message)
|
||||
|
||||
# Refused before registration, so nothing of this client is left at the authorization server
|
||||
# the host just rejected, and no browser was opened.
|
||||
assert_not_requested(:post, "#{@auth_base}/register")
|
||||
refute_includes(recorder, :redirected)
|
||||
end
|
||||
|
||||
def test_run_client_credentials_consults_the_validator
|
||||
provider = ClientCredentialsProvider.new(
|
||||
client_id: "cc-client",
|
||||
client_secret: "cc-secret",
|
||||
authorization_request_validator: ->(_request) { false },
|
||||
)
|
||||
|
||||
error = assert_raises(Flow::AuthorizationRefusedError) do
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
assert_match(/refused by `authorization_request_validator`/, error.message)
|
||||
assert_not_requested(:post, "#{@auth_base}/token")
|
||||
end
|
||||
|
||||
def test_run_jwt_bearer_consults_the_validator
|
||||
minted = false
|
||||
provider = CrossAppAccessProvider.new(
|
||||
client_id: "xaa-client",
|
||||
client_secret: "xaa-secret",
|
||||
assertion_provider: ->(audience:, resource:) {
|
||||
minted = true
|
||||
"id-jag-assertion"
|
||||
},
|
||||
authorization_request_validator: ->(_request) { false },
|
||||
)
|
||||
|
||||
error = assert_raises(Flow::AuthorizationRefusedError) do
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
assert_match(/refused by `authorization_request_validator`/, error.message)
|
||||
assert_not_requested(:post, "#{@auth_base}/token")
|
||||
|
||||
# Refused before the identity provider was asked for an assertion carrying this authorization server as its audience.
|
||||
refute(minted, "the ID-JAG assertion must not be minted for a refused authorization server")
|
||||
end
|
||||
|
||||
def test_run_registers_native_application_type_for_loopback_redirect_uri
|
||||
run_authorization_flow
|
||||
|
||||
|
|
@ -2264,6 +2365,64 @@ module MCP
|
|||
assert_equal("saved-rt", provider.tokens["refresh_token"])
|
||||
end
|
||||
|
||||
def test_run_records_the_issuer_that_minted_the_tokens
|
||||
state_holder = {}
|
||||
provider = Provider.new(
|
||||
client_metadata: {
|
||||
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]] },
|
||||
)
|
||||
|
||||
Flow.new(provider: provider).run!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_equal(@auth_base, provider.tokens["issuer"])
|
||||
end
|
||||
|
||||
def test_refresh_refuses_an_authorization_server_that_did_not_issue_the_tokens
|
||||
# The stored tokens came from a different authorization server, so this refresh token must not reach the one named now.
|
||||
stub_request(:post, "#{@auth_base}/token").to_raise(StandardError.new("must not reach the token endpoint."))
|
||||
|
||||
provider = refresh_only_provider
|
||||
provider.save_tokens(
|
||||
"access_token" => "stale-at",
|
||||
"refresh_token" => "saved-rt",
|
||||
"issuer" => "https://issued-by.example.com",
|
||||
)
|
||||
|
||||
error = assert_raises(Flow::AuthorizationError) do
|
||||
Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
end
|
||||
|
||||
assert_match(/issued by a different authorization server/, error.message)
|
||||
|
||||
# Left in place: it is still good at the server that issued it, and the caller answers this by running the full authorization flow.
|
||||
assert_equal("saved-rt", provider.tokens["refresh_token"])
|
||||
end
|
||||
|
||||
def test_refresh_proceeds_for_tokens_stored_before_the_issuer_was_recorded
|
||||
stub_request(:post, "#{@auth_base}/token").to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(access_token: "fresh-at", token_type: "Bearer"),
|
||||
)
|
||||
|
||||
provider = refresh_only_provider
|
||||
provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt")
|
||||
|
||||
result = Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url)
|
||||
|
||||
assert_equal(:refreshed, result)
|
||||
|
||||
# Recorded on the way out, so the binding takes effect from here on.
|
||||
assert_equal(@auth_base, provider.tokens["issuer"])
|
||||
end
|
||||
|
||||
def test_refresh_succeeds_when_stored_issuer_matches
|
||||
stub_request(:post, "#{@auth_base}/token").with(
|
||||
body: hash_including("grant_type" => "refresh_token", "refresh_token" => "saved-rt"),
|
||||
|
|
@ -2913,6 +3072,20 @@ module MCP
|
|||
URI.decode_www_form(req.body).to_h["resource"] == "https://srv.example.com/mcp"
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def refresh_only_provider
|
||||
provider = Provider.new(
|
||||
client_metadata: { redirect_uris: ["http://localhost:0/callback"] },
|
||||
redirect_uri: "http://localhost:0/callback",
|
||||
redirect_handler: ->(_url) {},
|
||||
callback_handler: -> { [nil, nil] },
|
||||
)
|
||||
provider.save_client_information("client_id" => "test-client")
|
||||
|
||||
provider
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -1433,6 +1433,94 @@ module MCP
|
|||
assert_requested(:post, "#{@auth_base}/token", times: 1)
|
||||
end
|
||||
|
||||
def test_send_request_reauthorizes_when_the_authorization_server_did_not_issue_the_stored_tokens
|
||||
# The refusal in `refresh!` is only half the answer; this is the other half.
|
||||
# A renamed authorization server makes the refresh fail, the transport falls through to a full authorization,
|
||||
# and that is where the embedding application is asked about the new one.
|
||||
stub_request(:post, @mcp_url).with { |req|
|
||||
req.headers["Authorization"] != "Bearer fresh-at"
|
||||
}.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 fresh-at" }
|
||||
).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: @mcp_url, 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: "reauthorized-client"),
|
||||
)
|
||||
stub_request(:post, "#{@auth_base}/token").to_return(
|
||||
status: 200,
|
||||
headers: { "Content-Type" => "application/json" },
|
||||
body: JSON.generate(access_token: "fresh-at", token_type: "Bearer"),
|
||||
)
|
||||
|
||||
seen = []
|
||||
state_holder = {}
|
||||
provider = Provider.new(
|
||||
client_metadata: {
|
||||
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]] },
|
||||
authorization_request_validator: ->(request) {
|
||||
seen << request.authorization_server
|
||||
true
|
||||
},
|
||||
)
|
||||
|
||||
# Registered and refreshable, so the refresh path is genuinely reachable and the issuer binding is
|
||||
# what stops it rather than a missing client identity.
|
||||
provider.save_client_information("client_id" => "test-client")
|
||||
provider.save_tokens(
|
||||
"access_token" => "stale-at",
|
||||
"refresh_token" => "saved-rt",
|
||||
"issuer" => "https://issued-by.example.com",
|
||||
)
|
||||
|
||||
HTTP.new(url: @mcp_url, oauth: provider).send_request(
|
||||
request: { jsonrpc: "2.0", id: "1", method: "tools/call", params: {} },
|
||||
)
|
||||
|
||||
# The refresh token was never presented to the authorization server now being named.
|
||||
assert_not_requested(:post, "#{@auth_base}/token") do |req|
|
||||
URI.decode_www_form(req.body).to_h["grant_type"] == "refresh_token"
|
||||
end
|
||||
assert_equal([@auth_base], seen)
|
||||
assert_equal(@auth_base, provider.tokens["issuer"])
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
# Stubs PRM, AS metadata, /register, and /token so a full authorization
|
||||
|
|
|
|||
Loading…
Reference in a new issue