Let a provider add parameters to the token requests it makes
## Motivation and Context `ClientCredentialsProvider` builds a fixed `client_credentials` token request: `grant_type`, `scope`, `resource`, and the client authentication that `Flow#post_to_token_endpoint` adds. An authorization server that requires further parameters, such as Auth0's `audience`, cannot be used through `oauth:` at all, and giving up `oauth:` also gives up the bearer header and the 401-driven retry that the transport provides. RFC 6749 Section 8.2 allows extension parameters on the token request, and the TypeScript SDK lets a provider shape its token request through `prepareTokenRequest`. `ClientCredentialsProvider.new` now takes `token_request_params:`, a Hash of String keys and values added to every token request the provider makes, and `Flow#post_to_token_endpoint` reads the same name by duck typing, so a `Provider` or `CrossAppAccessProvider` subclass that defines the method gets the same treatment on its authorization code exchange, refresh, or `jwt-bearer` request. `Provider.new` and `CrossAppAccessProvider.new` take the same keyword, held by `StorageBackedProvider` like `authorization_request_validator`, so every bundled provider refuses a bad value before anything is stored. The parameters go underneath the flow's own, so the SDK's values always win, and a key the SDK sets itself (`Flow::RESERVED_TOKEN_REQUEST_PARAMS`) is refused rather than silently overridden: with `ArgumentError` from the constructor, before any `client_information` is written, and again with `ArgumentError` from the flow, before the token request is sent. Only Strings are accepted because `URI.encode_www_form` encodes other scalars and Arrays in ways the caller did not write, and the provider keeps a frozen copy so a later change to the caller's Hash cannot alter what is sent. A Hash comparing keys by identity is refused too, since two equal keys are two entries there, sent twice by a provider method or silently collapsed into one by the copy. The flow raises `ArgumentError` rather than `AuthorizationError` because `MCP::Client::HTTP` treats a failed refresh as a reason to run the interactive flow, which would then fail the same way after the user signed in. Both refusals raise `Flow::InvalidTokenRequestParamsError`, a subclass of `ArgumentError`, so a caller can tell them from an unknown keyword, and the copy duplicates keys as well as values, since `Hash` copies only keys whose class is exactly `String`. Fixes #554. ## How Has This Been Tested? New tests cover every token request the parameters can ride (the `client_credentials` grant under all three client authentication methods, the authorization code exchange, refresh, and `jwt-bearer`), both refusal boundaries (the constructors of all three providers, and the flow for a provider defining the method), and the transport surfacing a refused hook from a refresh attempt without starting the interactive flow. All of them fail against the previous library. ## Breaking Changes None. The keyword is optional and the hook is opt-in; a provider that does not define `token_request_params` sends the same requests as before.
This commit is contained in:
parent
41fbf05d26
commit
8ad80dd0b4
5 changed files with 583 additions and 24 deletions
|
|
@ -37,11 +37,15 @@ module MCP
|
|||
# also take the algorithm as an explicit option).
|
||||
# - `scope` - String of space-separated scopes to request when the server's
|
||||
# `WWW-Authenticate` and the Protected Resource Metadata do not specify one.
|
||||
# - `storage` - Object responding to `tokens`, `save_tokens(tokens)`,
|
||||
# `client_information`, and `save_client_information(info)`. Defaults to
|
||||
# an `InMemoryStorage`. The `client_id` / `client_secret` are written
|
||||
# into it so the token exchange reads them through the same path as
|
||||
# a pre-registered authorization-code client.
|
||||
# - `storage` - Object responding to `tokens`, `save_tokens(tokens)`, `client_information`,
|
||||
# and `save_client_information(info)`. Defaults to an `InMemoryStorage`.
|
||||
# The `client_id` / `client_secret` are written into it so the token exchange reads
|
||||
# them through the same path as a pre-registered authorization-code client.
|
||||
# - `token_request_params` - Hash of String keys and values added to every
|
||||
# token request this provider makes, for parameters the authorization
|
||||
# server requires beyond the grant itself (Auth0's `audience`, for example).
|
||||
# A key in `Flow::RESERVED_TOKEN_REQUEST_PARAMS` raises `Flow::InvalidTokenRequestParamsError`.
|
||||
# The Hash is copied and frozen. See `StorageBackedProvider#token_request_params`.
|
||||
class ClientCredentialsProvider
|
||||
include StorageBackedProvider
|
||||
|
||||
|
|
@ -61,7 +65,8 @@ module MCP
|
|||
signing_algorithm: nil,
|
||||
scope: nil,
|
||||
storage: nil,
|
||||
authorization_request_validator: nil
|
||||
authorization_request_validator: nil,
|
||||
token_request_params: nil
|
||||
)
|
||||
if blank?(client_id)
|
||||
raise InvalidCredentialsError, "client_id is required for the client_credentials grant."
|
||||
|
|
@ -105,6 +110,7 @@ module MCP
|
|||
@scope = scope
|
||||
@storage = storage || InMemoryStorage.new
|
||||
@authorization_request_validator = authorization_request_validator
|
||||
@token_request_params = frozen_token_request_params(token_request_params)
|
||||
@storage.save_client_information(client_information)
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -25,6 +25,10 @@ module MCP
|
|||
# the Protected Resource Metadata do not specify one.
|
||||
# - `storage` - Object responding to `tokens`, `save_tokens(tokens)`, `client_information`, and `save_client_information(info)`.
|
||||
# Defaults to an `InMemoryStorage`.
|
||||
# - `token_request_params` - Hash of String keys and values added to the `jwt-bearer` token request, for parameters
|
||||
# the authorization server requires beyond the grant itself. A key in `Flow::RESERVED_TOKEN_REQUEST_PARAMS` raises
|
||||
# `Flow::InvalidTokenRequestParamsError`. The Hash is copied and frozen.
|
||||
# See `StorageBackedProvider#token_request_params`.
|
||||
#
|
||||
# https://github.com/modelcontextprotocol/modelcontextprotocol/issues/990
|
||||
class CrossAppAccessProvider
|
||||
|
|
@ -35,7 +39,15 @@ module MCP
|
|||
|
||||
attr_reader :scope, :storage
|
||||
|
||||
def initialize(client_id:, client_secret:, assertion_provider:, scope: nil, storage: nil, authorization_request_validator: nil)
|
||||
def initialize(
|
||||
client_id:,
|
||||
client_secret:,
|
||||
assertion_provider:,
|
||||
scope: nil,
|
||||
storage: nil,
|
||||
authorization_request_validator: nil,
|
||||
token_request_params: nil
|
||||
)
|
||||
if blank?(client_id)
|
||||
raise InvalidConfigurationError, "client_id is required for the jwt-bearer grant."
|
||||
end
|
||||
|
|
@ -52,6 +64,7 @@ module MCP
|
|||
@scope = scope
|
||||
@storage = storage || InMemoryStorage.new
|
||||
@authorization_request_validator = authorization_request_validator
|
||||
@token_request_params = frozen_token_request_params(token_request_params)
|
||||
@storage.save_client_information(
|
||||
"client_id" => client_id,
|
||||
"client_secret" => client_secret,
|
||||
|
|
|
|||
|
|
@ -17,6 +17,23 @@ module MCP
|
|||
TOKEN_ENDPOINT_ERROR_MAX_LENGTH = 128
|
||||
TOKEN_ENDPOINT_ERROR_DESCRIPTION_MAX_LENGTH = 512
|
||||
|
||||
# Token request parameters the flow sets itself. Its values win over a provider's `token_request_params`,
|
||||
# so a provider naming one of these is refused rather than left believing its value was sent.
|
||||
RESERVED_TOKEN_REQUEST_PARAMS = [
|
||||
"grant_type",
|
||||
"client_id",
|
||||
"client_secret",
|
||||
"client_assertion",
|
||||
"client_assertion_type",
|
||||
"scope",
|
||||
"resource",
|
||||
"code",
|
||||
"code_verifier",
|
||||
"redirect_uri",
|
||||
"refresh_token",
|
||||
"assertion",
|
||||
].freeze
|
||||
|
||||
class AuthorizationError < StandardError
|
||||
attr_reader :http_status, :error, :error_description
|
||||
|
||||
|
|
@ -40,6 +57,33 @@ module MCP
|
|||
# or authorization server metadata failure by rescuing a class rather than by matching the message text.
|
||||
class AuthorizationRefusedError < AuthorizationError; end
|
||||
|
||||
# Raised for a `token_request_params` value the SDK refuses: a reserved key, a Hash comparing keys by identity,
|
||||
# or anything but a Hash of Strings. An `ArgumentError` because the value is a configuration mistake,
|
||||
# not a failed authorization, and deliberately outside `AuthorizationError`, which `MCP::Client::HTTP` treats on
|
||||
# a failed refresh as a reason to run the interactive flow.
|
||||
class InvalidTokenRequestParamsError < ArgumentError; end
|
||||
|
||||
class << self
|
||||
# Returns why `params` cannot ride a token request as `token_request_params`, or `nil` when it can.
|
||||
# Shared by the provider constructors and the flow, which both refuse the value with `InvalidTokenRequestParamsError`,
|
||||
# so the same problem reads the same wherever it surfaces.
|
||||
def token_request_params_problem(params)
|
||||
return "must be a Hash (got #{params.class})." unless params.is_a?(Hash)
|
||||
|
||||
# Two equal keys are two entries here, which would be sent twice from a provider method
|
||||
# or silently collapse into one when the constructors copy the Hash.
|
||||
return "must not compare keys by identity." if params.compare_by_identity?
|
||||
|
||||
params.each do |key, value|
|
||||
return "keys must be Strings (got #{key.class})." unless key.is_a?(String)
|
||||
return "values must be Strings (got #{value.class} for #{key.inspect})." unless value.is_a?(String)
|
||||
return "must not set #{key.inspect}, which the SDK sets itself." if RESERVED_TOKEN_REQUEST_PARAMS.include?(key)
|
||||
end
|
||||
|
||||
nil
|
||||
end
|
||||
end
|
||||
|
||||
def initialize(provider:, http_client_factory: nil)
|
||||
@provider = provider
|
||||
@http_client_factory = http_client_factory || -> { default_http_client }
|
||||
|
|
@ -960,6 +1004,22 @@ module MCP
|
|||
@provider.authorization_flow
|
||||
end
|
||||
|
||||
# Parameters the provider adds to every token request it makes (RFC 6749 Section 8.2 leaves room for them;
|
||||
# Auth0's `audience` is the usual one). Duck-typed like `authorization_flow`, so a provider without the method,
|
||||
# or one returning `nil`, adds nothing.
|
||||
# A bad value raises `InvalidTokenRequestParamsError`, as the provider constructors do.
|
||||
def provider_token_request_params
|
||||
return {} unless @provider.respond_to?(:token_request_params)
|
||||
|
||||
params = @provider.token_request_params
|
||||
return {} if params.nil?
|
||||
|
||||
problem = self.class.token_request_params_problem(params)
|
||||
raise InvalidTokenRequestParamsError, "The provider's token_request_params #{problem}" if problem
|
||||
|
||||
params
|
||||
end
|
||||
|
||||
def build_authorization_url(as_metadata:, client_id:, scope:, state:, code_challenge:, resource:)
|
||||
authorization_endpoint = as_metadata["authorization_endpoint"]
|
||||
unless authorization_endpoint
|
||||
|
|
@ -1012,6 +1072,9 @@ module MCP
|
|||
# Submits a form-encoded token request using the authentication method
|
||||
# stored in `client_information`. The method determines whether client
|
||||
# credentials belong in the form body, a Basic header, or a JWT assertion.
|
||||
# A provider's `token_request_params` go underneath the flow's own parameters,
|
||||
# which therefore win, and are refused before the request is sent when they name
|
||||
# a reserved parameter or are not a Hash of Strings.
|
||||
def post_to_token_endpoint(as_metadata:, client_info:, form:)
|
||||
client_id = client_info_required_value(client_info, "client_id")
|
||||
unless client_id
|
||||
|
|
@ -1021,6 +1084,7 @@ module MCP
|
|||
|
||||
client_secret = client_info_required_value(client_info, "client_secret")
|
||||
token_endpoint_auth_method = client_info_value(client_info, "token_endpoint_auth_method")
|
||||
form = provider_token_request_params.merge(form)
|
||||
|
||||
# Apply one client authentication method per request (RFC 6749 Section 2.3).
|
||||
headers = {}
|
||||
|
|
|
|||
|
|
@ -48,6 +48,11 @@ module MCP
|
|||
# The document served at the URL is a separate JSON artifact from the `client_metadata` keyword:
|
||||
# DCR `client_metadata` MUST NOT include `client_id`, while the CIMD document MUST include `client_id` set
|
||||
# to the URL, `client_name`, and `redirect_uris` covering `redirect_uri`.
|
||||
# - `token_request_params` - Hash of String keys and values added to every token request this provider makes
|
||||
# (the authorization code exchange and refresh), for parameters the authorization server requires beyond
|
||||
# the grant itself. The authorization request is not affected. A key in `Flow::RESERVED_TOKEN_REQUEST_PARAMS`
|
||||
# raises `Flow::InvalidTokenRequestParamsError`.
|
||||
# The Hash is copied and frozen. See `StorageBackedProvider#token_request_params`.
|
||||
class Provider
|
||||
include StorageBackedProvider
|
||||
|
||||
|
|
@ -87,7 +92,8 @@ module MCP
|
|||
scope: nil,
|
||||
storage: nil,
|
||||
client_id_metadata_document_url: nil,
|
||||
authorization_request_validator: nil
|
||||
authorization_request_validator: nil,
|
||||
token_request_params: nil
|
||||
)
|
||||
unless Discovery.secure_url?(redirect_uri)
|
||||
raise InsecureRedirectURIError,
|
||||
|
|
@ -117,6 +123,7 @@ module MCP
|
|||
@storage = storage || InMemoryStorage.new
|
||||
@client_id_metadata_document_url = client_id_metadata_document_url
|
||||
@authorization_request_validator = authorization_request_validator
|
||||
@token_request_params = frozen_token_request_params(token_request_params)
|
||||
end
|
||||
|
||||
# Identifies the OAuth flow this provider drives.
|
||||
|
|
|
|||
|
|
@ -4,11 +4,11 @@ module MCP
|
|||
class Client
|
||||
module OAuth
|
||||
# Shared token/credential persistence for the OAuth provider classes
|
||||
# (`Provider` for the authorization-code flow and `ClientCredentialsProvider`
|
||||
# for the client_credentials flow). The two grants differ in how they authenticate,
|
||||
# but both read and write the same two pieces of state through a `storage` object:
|
||||
# the token response and the client information. This module supplies that delegation
|
||||
# so the `Flow` orchestrator can treat any provider uniformly.
|
||||
# (`Provider` for the authorization-code flow, `ClientCredentialsProvider`
|
||||
# for the client_credentials flow, and `CrossAppAccessProvider` for the jwt-bearer flow).
|
||||
# The grants differ in how they authenticate, but all read and write the same two pieces of state
|
||||
# through a `storage` object: the token response and the client information. This module supplies
|
||||
# that delegation so the `Flow` orchestrator can treat any provider uniformly.
|
||||
#
|
||||
# Including classes must set `@storage` to an object responding to `tokens`,
|
||||
# `save_tokens(tokens)`, `client_information`, and `save_client_information(info)`
|
||||
|
|
@ -21,6 +21,13 @@ module MCP
|
|||
# MCP SDK has today.
|
||||
attr_reader :authorization_request_validator
|
||||
|
||||
# Optional Hash of String keys and values added to every token request the provider makes, for parameters
|
||||
# the authorization server requires beyond the grant itself (RFC 6749 Section 8.2 leaves room for them;
|
||||
# Auth0's `audience` is the usual one). `nil` (the default) adds nothing.
|
||||
# Set through `frozen_token_request_params`, which refuses a key `Flow` sets itself or a wrongly shaped value
|
||||
# with `Flow::InvalidTokenRequestParamsError`.
|
||||
attr_reader :token_request_params
|
||||
|
||||
def access_token
|
||||
tokens&.dig("access_token") || tokens&.dig(:access_token)
|
||||
end
|
||||
|
|
@ -44,6 +51,21 @@ module MCP
|
|||
def clear_tokens!
|
||||
@storage.save_tokens(nil)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
# Constructors call this before writing to `storage`, so a rejected provider leaves it untouched.
|
||||
# The copy has its own frozen keys and values, so a later change to the caller's Hash, to a key,
|
||||
# or to a value cannot alter what is sent to the token endpoint. Keys are copied explicitly
|
||||
# because `Hash` copies and freezes only keys whose class is exactly `String`.
|
||||
def frozen_token_request_params(params)
|
||||
return if params.nil?
|
||||
|
||||
problem = Flow.token_request_params_problem(params)
|
||||
raise Flow::InvalidTokenRequestParamsError, "token_request_params #{problem}" if problem
|
||||
|
||||
params.each_with_object({}) { |(key, value), copy| copy[key.dup.freeze] = value.dup.freeze }.freeze
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in a new issue