diff --git a/CHANGELOG.md b/CHANGELOG.md index 41880459..ff38a6ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Let a web application finish authorization in the request that receives the redirect (#573) + ## [1.6.0] - 2026-09-21 This release lets an application configure, on the OAuth provider, the requests the flow makes to diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index 49542fb4..72a9ade0 100644 --- a/docs/_client/authorization.md +++ b/docs/_client/authorization.md @@ -90,12 +90,14 @@ Required keyword arguments to `Provider.new`: an explicit value always wins. - `redirect_uri`: String. Must use HTTPS or be a loopback URL (`localhost`, `127.0.0.0/8`, `::1`); other values raise `Provider::InsecureRedirectURIError`. - `redirect_handler`: Callable invoked with the fully-built authorization `URI`. Typically opens the user's browser. -- `callback_handler`: Callable that returns `[code, state]` or `[code, state, iss]` after the user is redirected back to `redirect_uri`. Returning the 3-element form - (with `iss` set to the RFC 9207 `iss` parameter from the redirect, or `nil` when absent) opts into SEP-2468 issuer validation: a present `iss` must match - the authorization server's issuer, and a missing one is rejected when the server advertises `authorization_response_iss_parameter_supported`. Optional keyword arguments: +- `callback_handler`: Callable that returns `[code, state]` or `[code, state, iss]` after the user is redirected back to `redirect_uri`. Returning the 3-element form + (with `iss` set to the RFC 9207 `iss` parameter from the redirect, or `nil` when absent) opts into SEP-2468 issuer validation: a present `iss` must match + the authorization server's issuer, and a missing one is rejected when the server advertises `authorization_response_iss_parameter_supported`. + Omit it when the redirect arrives in a later request, as it does in a web application; see [Authorization in Web Applications](#authorization-in-web-applications). +- `pending_authorization_max_age`: Integer seconds a pending authorization stays redeemable after the redirect when `callback_handler` is omitted. Defaults to 600. - `scope`: Space-separated scopes to request when the server's `WWW-Authenticate` does not specify one. - `authorization_request_validator`: Callable invoked with an `MCP::Client::OAuth::AuthorizationRequest` before any authorization request is built. Returning a falsy value abandons the flow with `Flow::AuthorizationRefusedError`. See [Reviewing the authorization request](#reviewing-the-authorization-request). @@ -106,7 +108,8 @@ Optional keyword arguments: issued it (SEP-2352): when the server's authorization server changes, the SDK discards the stale registration and its tokens and re-registers automatically (portable CIMD `client_id`s are kept). Saved `tokens` carry an `"issuer"` member of their own, recording the authorization server that minted them, which is what lets a later refresh refuse a server the MCP server has since renamed. Treat both hashes as opaque and persist them as-is; a storage that writes out selected members - instead drops these bindings with no error. + instead drops these bindings with no error. Without `callback_handler`, it must also hold pending authorizations; see + [Authorization in Web Applications](#authorization-in-web-applications). - `client_id_metadata_document_url`: URL where you publish a Client ID Metadata Document (`draft-ietf-oauth-client-id-metadata-document` and the MCP authorization specification). When the authorization server advertises `client_id_metadata_document_supported: true`, @@ -173,6 +176,75 @@ provider = MCP::Client::OAuth::Provider.new( ) ``` +### Authorization in Web Applications + +`callback_handler` keeps the flow open until the code comes back, so the process that sent the user to the authorization server stays blocked for as long as +the user takes to sign in and consent. That suits CLI and desktop clients. In a web application the redirect arrives as a separate HTTP request, often served +by a different process, and relaying the code to a request held open for that long is impractical. Omit `callback_handler` and the authorization spans the two requests: + +1. When the transport meets a `401`, or a `403` step-up challenge, the flow runs discovery and registration as usual, saves a pending authorization in `storage` + keyed by the `state` it generated, hands the authorization URL to `redirect_handler`, and raises `MCP::Client::OAuth::Flow::AuthorizationPendingError` + instead of retrying. The error's `authorization_url` reader returns the same URL, so the application can send the user there from wherever is convenient. +2. The request that receives the redirect calls `MCP::Client::OAuth::Flow#finish!` with the redirect's whole query. Requests made afterwards use the stored tokens. + +```ruby +def mcp_oauth_provider(user) + MCP::Client::OAuth::Provider.new( + client_metadata: { + client_name: "My MCP App", + redirect_uris: ["https://app.example.com/oauth/mcp/callback"], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }, + redirect_uri: "https://app.example.com/oauth/mcp/callback", + redirect_handler: ->(_authorization_url) {}, + storage: McpCredentialStorage.new(user), # per-user storage, including pending authorizations + ) +end + +# In a request that talks to the MCP server: +transport = MCP::Client::HTTP.new(url: server_url, oauth: mcp_oauth_provider(current_user)) +begin + tools = MCP::Client.new(transport: transport).tools +rescue MCP::Client::OAuth::Flow::AuthorizationPendingError => e + redirect_to(e.authorization_url.to_s, allow_other_host: true) +end + +# In the action serving `redirect_uri`: +MCP::Client::OAuth::Flow.new(provider: mcp_oauth_provider(current_user)).finish!( + server_url: server_url, + callback_params: request.query_parameters, +) +``` + +`run!` can also start an authorization without a request to the MCP server: `MCP::Client::OAuth::Flow.new(provider: provider).run!(server_url: server_url)` returns `:redirect`, +and the flow's `authorization_url` reader returns the URL it handed to `redirect_handler`. + +The storage must also respond to `save_pending_authorization(state, pending)`, `pending_authorization(state)`, and `delete_pending_authorization(state)`; +`Provider.new` raises `Provider::PendingAuthorizationStorageError` when it does not. A pending authorization is a Hash of JSON-compatible values that includes +the PKCE verifier, so keep it where you keep credentials, persist it as-is, and share it between the processes that can receive the redirect. +`delete_pending_authorization` must remove the entry and return it in one atomic step, such as `GETDEL` in Redis or `DELETE ... RETURNING` in SQL, +and return `nil` when there was none. `InMemoryStorage` implements the methods for a single process. + +`finish!` redeems the code the way the authorization began, and refuses anything else with `Flow::AuthorizationError`: + +- The pending authorization is looked up by `state` before any request is made. An unknown, already used, or malformed one is refused, + and one older than `pending_authorization_max_age` is discarded and refused. +- `server_url` must name the MCP server the authorization began with. +- The RFC 9207 `iss` parameter is validated against the recorded issuer before the pending authorization is consumed, so a forged callback carrying a valid `state` + cannot discard the verifier the legitimate callback needs. Because `finish!` sees the whole query, a missing `iss` is refused whenever the authorization server + advertises `authorization_response_iss_parameter_supported`. +- The pending authorization is then consumed through `delete_pending_authorization`, and only a callback that gets the entry back proceeds, so of two callbacks + racing on the same `state`, such as a retried redirect, at most one redeems the code. An `error` response is raised with its `error` and `error_description`, + bounded as [token endpoint errors](#token-endpoint-errors) are; it is read only after the `iss` check, since in a mix-up those parameters are the attacker's. +- The code is redeemed at the recorded token endpoint, with the client registration, `resource`, and `redirect_uri` used when the authorization began, + without running discovery again (SEP-2352). A registration replaced in the meantime is refused. + +{: .important } +> `state` proves that this SDK started the authorization, not which user did. Binding the callback to the user who started it is the application's responsibility: +> scope `storage` to that user, as in the example, so that a callback delivered to another user's session finds no pending authorization. + ### Token Endpoint Errors When a token exchange or refresh fails, `MCP::Client::OAuth::Flow::AuthorizationError` includes the HTTP status and diff --git a/lib/mcp/client/http.rb b/lib/mcp/client/http.rb index 6969b07e..3d3f23b8 100644 --- a/lib/mcp/client/http.rb +++ b/lib/mcp/client/http.rb @@ -902,15 +902,25 @@ def parse_www_authenticate_from_error(error) MCP::Client::OAuth::Discovery.parse_www_authenticate(header) end + # A provider without a `callback_handler` finishes the authorization in the request that receives the redirect, + # not here, so there is nothing to retry with yet: the pending authorization surfaces as + # `Flow::AuthorizationPendingError`, and requests made after `Flow#finish!` pick up the stored tokens. def run_full_authorization_flow!(flow:, params:) # Use the URL snapshotted at `initialize` time so a post-construction # mutation of `@url` cannot redirect PRM/AS discovery and the authorize # URL to an attacker-controlled host. - flow.run!( + result = flow.run!( server_url: @oauth_server_url, resource_metadata_url: params["resource_metadata"], scope: params["scope"], ) + return unless result == :redirect + + raise MCP::Client::OAuth::Flow::AuthorizationPendingError.new( + "Authorization is pending: the user was sent to the authorization server, and the request can be retried " \ + "once `MCP::Client::OAuth::Flow#finish!` completes the authorization with the redirect's query.", + authorization_url: flow.authorization_url, + ) end # Tries to swap a saved `refresh_token` for a fresh access token. Returns truthy diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index 07164df5..13298e5d 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -83,6 +83,27 @@ class InvalidTokenRequestParamsError < ArgumentError; end # and `MCP::Client::HTTP` treats on a failed refresh as a reason to run the interactive flow. class DestinationMismatchError < ArgumentError; end + # Raised by `MCP::Client::HTTP` when a provider without a `callback_handler` has sent the user to the authorization server: + # the request cannot be retried until the application finishes the authorization with `finish!` in the request that + # receives the redirect. `authorization_url` is the URL handed to `redirect_handler`; it is kept out of the message, + # which may reach logs, because it carries the `state` that `finish!` looks the pending authorization up by. + # Deliberately outside `AuthorizationError`, which `MCP::Client::HTTP` treats on a failed refresh as a reason to run + # the interactive flow: this is the interactive flow waiting on the user, not a failure. + class AuthorizationPendingError < StandardError + attr_reader :authorization_url + + def initialize(message = nil, authorization_url: nil) + super(message) + @authorization_url = authorization_url + end + end + + # A `state` the SDK generates is 43 characters; a callback carrying a much longer one is refused before storage is asked. + CALLBACK_STATE_MAX_LENGTH = 128 + + UNKNOWN_PENDING_AUTHORIZATION_MESSAGE = + "Authorization callback `state` matches no pending authorization; it is unknown, already used, or expired." + # Faraday middleware registered on the connection `build_http_client` assembles before the customizer # is invoked, so with the usual `use` it sits ahead of the customizer's middleware and sees the URL exactly # as the flow requested it, which it records on the request environment for `RequestedOriginGuard`. @@ -189,13 +210,20 @@ def build_http_client(customizer = nil) end end + # The authorization URL the last `run!` handed to `redirect_handler` before returning `:redirect`, or `nil`. + attr_reader :authorization_url + def initialize(provider:, http_client_factory: nil) @provider = provider @http_client_factory = http_client_factory || -> { default_http_client } + @authorization_url = nil end # Runs the full discovery, registration, authorization, and token exchange flow. # On success, persists tokens via the provider and returns `:authorized`. + # A provider without a `callback_handler` stops after the redirect instead: the flow saves + # a pending authorization in the provider's storage, keyed by `state`, and returns `:redirect`, leaving the code + # exchange to `finish!` in the request that receives the redirect. def run!(server_url:, resource_metadata_url: nil, scope: nil) # The `resource_metadata` URL ships in `WWW-Authenticate` and is the very # first thing we contact in the OAuth flow, so it has to clear the same @@ -257,6 +285,20 @@ def run!(server_url:, resource_metadata_url: nil, scope: nil) resource: resource, ) + if @provider.callback_handler.nil? + save_pending_authorization( + state: state, + code_verifier: pkce[:code_verifier], + server_url: server_url, + resource: resource, + client_id: client_info_required_value(client_info, "client_id"), + as_metadata: as_metadata, + ) + @authorization_url = authorization_url + @provider.redirect_handler.call(authorization_url) + return :redirect + end + @provider.redirect_handler.call(authorization_url) callback_result = Array(@provider.callback_handler.call) code, returned_state, returned_iss = callback_result @@ -449,6 +491,82 @@ def refresh!(server_url:, resource_metadata_url: nil) :refreshed end + # Finishes an authorization that `run!` left pending, in the request that receives the redirect to `redirect_uri`, + # which may run in another process. `callback_params` is that redirect's whole query as a Hash (`code`, `state`, and, + # when present, `iss`, `error`, and `error_description`); passing all of it, rather than picking values out, is what + # lets the flow tell an absent `iss` from one the caller did not look for. + # + # The pending authorization is looked up by `state` before any request is made, and it binds the rest of the exchange: + # the code is redeemed at the token endpoint recorded when the authorization began, with the client registration, + # `resource`, and `redirect_uri` used then, and without discovery running again, so the code reaches the authorization + # server the user was sent to (SEP-2352). The RFC 9207 `iss` is validated against the recorded issuer before the pending + # authorization is consumed, so a forged callback carrying a valid `state` cannot discard the verifier the legitimate + # callback needs, and before the callback's `error` is read, since in a mix-up those parameters are the attacker's. + # Past that check the pending authorization is consumed with `delete_pending_authorization`, which returns the entry + # it removed; only a callback that gets the entry back redeems the code, so of callbacks racing on the same `state` + # (a retried redirect, say), at most one does. + # + # Binding the callback to the user who started the authorization stays with the application: scoping `storage` + # to that user means a callback delivered to another user's session finds no pending authorization. + # + # On success, persists tokens via the provider and returns `:authorized`. + def finish!(server_url:, callback_params:) + unless provider_authorization_flow == :authorization_code && @provider.callback_handler.nil? + raise ArgumentError, + "finish! completes an authorization started by a provider without a callback_handler; " \ + "this provider finishes its authorizations in `run!`." + end + + state = callback_param(callback_params, "state") + raise AuthorizationError, "Authorization callback carried no `state`." unless state + + pending = @provider.pending_authorization(state) if state.bytesize <= CALLBACK_STATE_MAX_LENGTH + raise AuthorizationError, UNKNOWN_PENDING_AUTHORIZATION_MESSAGE unless valid_pending_authorization?(pending) + + if pending_authorization_expired?(pending) + @provider.delete_pending_authorization(state) + raise AuthorizationError, "The pending authorization has expired; start a new authorization." + end + + unless safe_canonicalize_url(server_url, label: "MCP server URL") == pending["server_url"] + raise AuthorizationError, "The pending authorization was started for a different MCP server." + end + + as_metadata = pending["authorization_server_metadata"] + # Checked when the authorization began, and checked again because the metadata has since made a round trip + # through the application's storage. + ensure_secure_endpoints!(as_metadata, server_url: pending["server_url"]) + + validate_authorization_response_issuer!( + as_metadata: as_metadata, + iss: callback_param(callback_params, "iss"), + iss_provided: true, + ) + + # The read above and this delete are separate calls, so the storage's atomic delete is what decides + # which of two concurrent callbacks carrying this `state` proceeds. + consumed = @provider.delete_pending_authorization(state) + raise AuthorizationError, UNKNOWN_PENDING_AUTHORIZATION_MESSAGE unless valid_pending_authorization?(consumed) + + error = callback_param(callback_params, "error") + raise authorization_response_error(error, callback_param(callback_params, "error_description")) if error + + code = callback_param(callback_params, "code") + raise AuthorizationError, "Authorization callback carried no authorization code." unless code + + tokens = exchange_authorization_code( + as_metadata: as_metadata, + client_info: pending_client_information(pending, as_metadata: as_metadata), + code: code, + code_verifier: pending["code_verifier"], + resource: pending["resource"], + redirect_uri: pending["redirect_uri"], + ) + + save_tokens_issued_by(tokens, as_metadata: as_metadata) + :authorized + end + private def read_token(key) @@ -1096,6 +1214,88 @@ def states_match?(returned, expected) result.zero? end + def provider_pending_authorization_max_age + return Provider::DEFAULT_PENDING_AUTHORIZATION_MAX_AGE unless @provider.respond_to?(:pending_authorization_max_age) + + @provider.pending_authorization_max_age + end + + # Records what `finish!` needs to redeem the code exactly as this authorization began: the PKCE verifier, the MCP server, + # the `resource` and `redirect_uri` sent, the client identity used, and the authorization server metadata already validated. + # The client secret is not copied: `finish!` reads the registration from storage and requires the same `client_id`. + # Every value is JSON-compatible, so storage can serialize the entry as-is. + def save_pending_authorization(state:, code_verifier:, server_url:, resource:, client_id:, as_metadata:) + @provider.save_pending_authorization( + state, + { + "code_verifier" => code_verifier, + "server_url" => safe_canonicalize_url(server_url, label: "MCP server URL"), + "resource" => resource, + "redirect_uri" => @provider.redirect_uri, + "client_id" => client_id, + "authorization_server_metadata" => as_metadata, + "created_at" => Time.now.to_i, + }, + ) + end + + def valid_pending_authorization?(pending) + return false unless pending.is_a?(Hash) + return false unless ["code_verifier", "server_url", "redirect_uri", "client_id"].all? { |key| non_empty_string?(pending[key]) } + return false unless pending["resource"].nil? || non_empty_string?(pending["resource"]) + + pending["authorization_server_metadata"].is_a?(Hash) && pending["created_at"].is_a?(Integer) + end + + def pending_authorization_expired?(pending) + Time.now.to_i - pending["created_at"] > provider_pending_authorization_max_age + end + + # The registration the authorization began with. A stored registration is used only while it still carries + # that `client_id` and is bound to the recorded authorization server; a Client ID Metadata Document URL, + # which is never stored, is used again while the recorded metadata advertises support for it. + def pending_client_information(pending, as_metadata:) + client_id = pending["client_id"] + + stored = @provider.client_information + if stored.is_a?(Hash) && + client_info_required_value(stored, "client_id") == client_id && + client_info_required_value(stored, "issuer") == as_metadata["issuer"] + return stored + end + + if client_id == provider_client_id_metadata_document_url && as_metadata["client_id_metadata_document_supported"] == true + return { "client_id" => client_id } + end + + raise AuthorizationError, "The client registration changed after the authorization began; start a new authorization." + end + + # Reads one parameter of an authorization response. Only a non-empty String counts: a repeated parameter that + # a framework delivers as an Array, or an empty value, is treated as absent. + def callback_param(params, key) + return unless params.is_a?(Hash) + + value = params[key] || params[key.to_sym] + non_empty_string?(value) ? value : nil + end + + def non_empty_string?(value) + value.is_a?(String) && !value.empty? + end + + # An RFC 6749 Section 4.1.2.1 error response, reported with the bounds a token endpoint error gets. + # Reached only after the `iss` check, so the values are the authorization server's own; they are still text it chose, + # so they are cut to a bounded length and confined to the printable ASCII the RFC permits. + def authorization_response_error(error, description) + error = bounded_diagnostic(error, limit: TOKEN_ENDPOINT_ERROR_MAX_LENGTH) + description = bounded_diagnostic(description, limit: TOKEN_ENDPOINT_ERROR_DESCRIPTION_MAX_LENGTH) + message = "The authorization server returned an error to the authorization callback." + message += " #{[error, description].compact.join(": ")}" if error || description + + AuthorizationError.new(message, error: error, error_description: description) + end + # Per MCP 2025-11-25 Authorization and the TS/Python SDKs, scope resolution # prefers the `WWW-Authenticate` challenge first, then `scopes_supported` # from the Protected Resource Metadata, and falls back to a provider-supplied @@ -1233,11 +1433,11 @@ def build_authorization_url(as_metadata:, client_id:, scope:, state:, code_chall uri end - def exchange_authorization_code(as_metadata:, client_info:, code:, code_verifier:, resource:) + def exchange_authorization_code(as_metadata:, client_info:, code:, code_verifier:, resource:, redirect_uri: @provider.redirect_uri) form = { "grant_type" => "authorization_code", "code" => code, - "redirect_uri" => @provider.redirect_uri, + "redirect_uri" => redirect_uri, "code_verifier" => code_verifier, } form["resource"] = resource if resource diff --git a/lib/mcp/client/oauth/in_memory_storage.rb b/lib/mcp/client/oauth/in_memory_storage.rb index 23edad93..469bf7e5 100644 --- a/lib/mcp/client/oauth/in_memory_storage.rb +++ b/lib/mcp/client/oauth/in_memory_storage.rb @@ -16,6 +16,15 @@ module OAuth # binding the credentials to the authorization server that issued them (SEP-2352); # custom storages should treat the hash as opaque and persist it as-is. # + # A provider without a `callback_handler` also keeps each pending authorization here, keyed by its `state`, + # between the request that sends the user to the authorization server and the request that receives + # the redirect (`save_pending_authorization(state, pending)`, `pending_authorization(state)`, + # `delete_pending_authorization(state)`). A pending authorization holds the PKCE verifier, so custom storages + # should treat it as a secret, persist it as-is, and may expire entries older than the provider's + # `pending_authorization_max_age`, which the flow refuses anyway. `delete_pending_authorization` must remove + # the entry and return it in one atomic step (`Hash#delete` here; `GETDEL` in Redis, `DELETE ... RETURNING` in SQL), + # returning `nil` when there was none: the flow redeems the code only when it gets the entry back. + # # This class keeps everything in process memory, so the credentials live # only for the lifetime of the Ruby process. Applications that need # persistence across restarts should supply a custom object responding to @@ -24,12 +33,15 @@ module OAuth # `Provider.new(storage: ...)`. The shape mirrors Python SDK's # `TokenStorage` Protocol; TypeScript's `OAuthClientProvider` rolls # the same responsibilities into a single object. + # A web application may receive the redirect in another process, so pending authorizations + # need shared storage. class InMemoryStorage attr_accessor :tokens, :client_information def initialize @tokens = nil @client_information = nil + @pending_authorizations = {} end def save_tokens(tokens) @@ -39,6 +51,18 @@ def save_tokens(tokens) def save_client_information(info) @client_information = info end + + def save_pending_authorization(state, pending) + @pending_authorizations[state] = pending + end + + def pending_authorization(state) + @pending_authorizations[state] + end + + def delete_pending_authorization(state) + @pending_authorizations.delete(state) + end end end end diff --git a/lib/mcp/client/oauth/provider.rb b/lib/mcp/client/oauth/provider.rb index 803b5b97..c7d916be 100644 --- a/lib/mcp/client/oauth/provider.rb +++ b/lib/mcp/client/oauth/provider.rb @@ -20,6 +20,8 @@ module OAuth # request. Must be one of `redirect_uris` in `client_metadata`. # - `redirect_handler` - Callable invoked with the fully-built authorization # URL (a `URI`). Implementations typically open the user's browser. + # + # Optional keyword arguments: # - `callback_handler` - Callable invoked after `redirect_handler`. Returns # `[code, state]` or `[code, state, iss]`, where `code` is the authorization code, # `state` is the `state` parameter received on the redirect URI, and `iss` is @@ -28,8 +30,11 @@ module OAuth # must match the authorization server's issuer, and a nil `iss` is # rejected when the AS advertises `authorization_response_iss_parameter_supported`. # The 2-element form skips the check for backward compatibility. - # - # Optional keyword arguments: + # Omit it when the redirect arrives in a later request, as it does in a web application: + # the flow then stops after `redirect_handler` with a pending authorization saved in `storage`, + # and the request that receives the redirect finishes it with `Flow#finish!`. + # - `pending_authorization_max_age` - Seconds a pending authorization stays redeemable after the redirect, + # when `callback_handler` is omitted. Defaults to `DEFAULT_PENDING_AUTHORIZATION_MAX_AGE`. # - `scope` - String of space-separated scopes to request when the server's # `WWW-Authenticate` does not specify one. # - `storage` - Object responding to `tokens`, `save_tokens(tokens)`, @@ -38,6 +43,9 @@ module OAuth # an `"issuer"` member binding it to the authorization server that # issued it (SEP-2352); when the authorization server changes, the SDK discards # the stale registration and tokens and re-registers. + # Without `callback_handler`, it must also respond to `save_pending_authorization(state, pending)`, + # `pending_authorization(state)`, and `delete_pending_authorization(state)`; the last must remove the entry + # and return it atomically, returning `nil` when there was none. # - `client_id_metadata_document_url` - URL where the client publishes its Client ID Metadata Document # (`draft-ietf-oauth-client-id-metadata-document-00` and the MCP authorization specification). # When the authorization server advertises `client_id_metadata_document_supported: true`, @@ -78,25 +86,43 @@ class UnregisteredRedirectURIError < ArgumentError; end # applies and the value must unambiguously identify the document. class InvalidClientIDMetadataDocumentURLError < ArgumentError; end + # Raised when `Provider#initialize` is called without `callback_handler` and with a `storage` + # that cannot hold a pending authorization between the request that sends the user to the authorization server + # and the request that receives the redirect. + class PendingAuthorizationStorageError < ArgumentError; end + + # Seconds a pending authorization stays redeemable after the redirect when the provider has no `callback_handler`. + # Long enough for a user to sign in and consent at the authorization server, short enough that an abandoned + # authorization does not keep its PKCE verifier in `storage` indefinitely. + DEFAULT_PENDING_AUTHORIZATION_MAX_AGE = 600 + + PENDING_AUTHORIZATION_STORAGE_METHODS = [ + :save_pending_authorization, + :pending_authorization, + :delete_pending_authorization, + ].freeze + attr_reader :client_metadata, :redirect_uri, :scope, :storage, :redirect_handler, :callback_handler, - :client_id_metadata_document_url + :client_id_metadata_document_url, + :pending_authorization_max_age def initialize( client_metadata:, redirect_uri:, redirect_handler:, - callback_handler:, + callback_handler: nil, scope: nil, storage: nil, client_id_metadata_document_url: nil, authorization_request_validator: nil, token_request_params: nil, - http_client_customizer: nil + http_client_customizer: nil, + pending_authorization_max_age: DEFAULT_PENDING_AUTHORIZATION_MAX_AGE ) unless Discovery.secure_url?(redirect_uri) raise InsecureRedirectURIError, @@ -120,16 +146,33 @@ def initialize( http_client_customizer = validated_http_client_customizer(http_client_customizer) + storage ||= InMemoryStorage.new + if callback_handler.nil? + missing = PENDING_AUTHORIZATION_STORAGE_METHODS.reject { |method| storage.respond_to?(method) } + unless missing.empty? + raise PendingAuthorizationStorageError, + "Without a callback_handler the authorization finishes in a later request, so storage must also respond to " \ + "#{missing.join(", ")} (#{storage.class} does not)." + end + end + + unless pending_authorization_max_age.is_a?(Integer) && pending_authorization_max_age.positive? + raise ArgumentError, + "pending_authorization_max_age must be a positive Integer number of seconds " \ + "(got #{pending_authorization_max_age.inspect})." + end + @client_metadata = client_metadata @redirect_uri = redirect_uri @redirect_handler = redirect_handler @callback_handler = callback_handler @scope = scope - @storage = storage || InMemoryStorage.new + @storage = storage @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) @http_client_customizer = http_client_customizer + @pending_authorization_max_age = pending_authorization_max_age end # Identifies the OAuth flow this provider drives. @@ -138,6 +181,20 @@ def initialize( def authorization_flow :authorization_code end + + def save_pending_authorization(state, pending) + @storage.save_pending_authorization(state, pending) + end + + def pending_authorization(state) + @storage.pending_authorization(state) + end + + # Returns the entry the storage removed, or `nil`. `Flow#finish!` redeems the code only when it gets + # the entry back, so of two callbacks racing on the same `state`, only one can. + def delete_pending_authorization(state) + @storage.delete_pending_authorization(state) + end end end end diff --git a/test/mcp/client/oauth/http_oauth_test.rb b/test/mcp/client/oauth/http_oauth_test.rb index dcbe3020..0a3cd384 100644 --- a/test/mcp/client/oauth/http_oauth_test.rb +++ b/test/mcp/client/oauth/http_oauth_test.rb @@ -141,6 +141,88 @@ def test_send_request_runs_oauth_flow_on_401_and_retries_with_bearer_token assert_equal("test-token-after-flow", provider.access_token) end + def test_send_request_raises_authorization_pending_for_a_provider_without_a_callback_handler + stub_request(:post, @mcp_url) + .with { |req| req.headers["Authorization"].nil? } + .to_return( + status: 401, + headers: { "WWW-Authenticate" => %(Bearer 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: @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"], + code_challenge_methods_supported: ["S256"], + ), + ) + + 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), + ) + + redirected_to = nil + provider = Provider.new( + client_metadata: { + redirect_uris: ["https://app.example.com/oauth/callback"], + grant_types: ["authorization_code"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }, + redirect_uri: "https://app.example.com/oauth/callback", + redirect_handler: ->(url) { redirected_to = url }, + ) + transport = HTTP.new(url: @mcp_url, oauth: provider) + request = { jsonrpc: "2.0", id: "1", method: "tools/list" } + + error = assert_raises(Flow::AuthorizationPendingError) do + transport.send_request(request: request) + end + + state = URI.decode_www_form(redirected_to.query).to_h.fetch("state") + assert_equal(redirected_to, error.authorization_url) + refute_includes(error.message, state) + # Nothing to retry with until the authorization is finished. + assert_requested(:post, @mcp_url, times: 1) + assert_not_requested(:post, "#{@auth_base}/token") + + Flow.new(provider: provider).finish!( + server_url: @mcp_url, + callback_params: { "code" => "test-auth-code", "state" => state }, + ) + response = transport.send_request(request: request) + + assert_equal({ "ok" => true }, response["result"]) + end + def test_send_request_runs_the_oauth_flow_through_the_provider_customizer stub_request(:post, @mcp_url).with { |req| req.headers["Authorization"].nil? diff --git a/test/mcp/client/oauth/pending_authorization_flow_test.rb b/test/mcp/client/oauth/pending_authorization_flow_test.rb new file mode 100644 index 00000000..00ea8097 --- /dev/null +++ b/test/mcp/client/oauth/pending_authorization_flow_test.rb @@ -0,0 +1,584 @@ +# frozen_string_literal: true + +require "test_helper" +require "base64" +require "digest" +require "json" +require "webmock/minitest" +require "faraday" +require "mcp/client/oauth" + +module MCP + class Client + module OAuth + # The authorization-code flow a web application drives: `run!` sends the user to the authorization server + # in one request, and `finish!` redeems the code in the request that receives the redirect, possibly in another process. + class PendingAuthorizationFlowTest < Minitest::Test + REDIRECT_URI = "https://app.example.com/oauth/callback" + + # Serializes every value through JSON, the way a database- or cache-backed storage shared across processes would, + # so nothing reaches `finish!` except what `run!` actually persisted. + class JSONStorage + attr_reader :pending_lookups + + def initialize + @entries = {} + @pending_lookups = 0 + end + + def tokens + read("tokens") + end + + def save_tokens(tokens) + write("tokens", tokens) + end + + def client_information + read("client_information") + end + + def save_client_information(info) + write("client_information", info) + end + + def save_pending_authorization(state, pending) + write("pending:#{state}", pending) + end + + def pending_authorization(state) + @pending_lookups += 1 + read("pending:#{state}") + end + + def delete_pending_authorization(state) + value = @entries.delete("pending:#{state}") + value && JSON.parse(value) + end + + private + + def read(key) + value = @entries[key] + value && JSON.parse(value) + end + + def write(key, value) + if value.nil? + @entries.delete(key) + else + @entries[key] = JSON.generate(value) + end + end + end + + # Holds each `pending_authorization` read until `parties` callers have made one, so concurrent `finish!` calls + # all pass their checks before any of them consumes the entry: the window a retried redirect can land in. + class BarrierStorage < JSONStorage + def initialize(parties) + super() + @parties = parties + @arrived = 0 + @lock = Mutex.new + @all_arrived = ConditionVariable.new + end + + def pending_authorization(state) + entry = super + @lock.synchronize do + @arrived += 1 + if @arrived >= @parties + @all_arrived.broadcast + else + @all_arrived.wait(@lock, 5) + end + end + entry + end + end + + # Deletes like Redis `DEL`, returning a count rather than the entry. + class CountingDeleteStorage < JSONStorage + def delete_pending_authorization(state) + super ? 1 : 0 + end + end + + def setup + WebMock.enable! + @server_url = "https://srv.example.com/mcp" + @prm_url = "https://srv.example.com/.well-known/oauth-protected-resource/mcp" + @auth_base = "https://auth.example.com" + @as_metadata_url = "#{@auth_base}/.well-known/oauth-authorization-server" + @redirected_to = nil + + stub_request(:get, @prm_url).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(resource: @server_url, authorization_servers: [@auth_base]), + ) + + stub_as_metadata + + 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-from-flow", token_type: "Bearer", expires_in: 3600), + ) + end + + def teardown + WebMock.reset! + end + + def test_run_saves_a_pending_authorization_and_returns_redirect_without_a_callback_handler + storage = JSONStorage.new + provider = provider_without_callback_handler(storage: storage) + flow = Flow.new(provider: provider) + + result = flow.run!(server_url: @server_url, resource_metadata_url: @prm_url) + + assert_equal(:redirect, result) + assert_equal(@redirected_to, flow.authorization_url) + + query = URI.decode_www_form(@redirected_to.query).to_h + pending = storage.pending_authorization(query.fetch("state")) + assert_equal(@server_url, pending["server_url"]) + assert_equal(@server_url, pending["resource"]) + assert_equal(REDIRECT_URI, pending["redirect_uri"]) + assert_equal("test-client", pending["client_id"]) + assert_equal(@auth_base, pending["authorization_server_metadata"]["issuer"]) + assert_kind_of(Integer, pending["created_at"]) + + # The recorded verifier is the one the authorization request committed to. + expected_challenge = Base64.urlsafe_encode64(Digest::SHA256.digest(pending["code_verifier"]), padding: false) + assert_equal(expected_challenge, query.fetch("code_challenge")) + + assert_nil(provider.access_token) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_finish_redeems_the_code_with_a_fresh_provider_over_the_same_storage + storage = JSONStorage.new + state = begin_authorization(storage) + verifier = storage.pending_authorization(state)["code_verifier"] + WebMock::RequestRegistry.instance.reset! + + # A fresh provider and flow over the same storage stand in for the process that receives the redirect. + provider = provider_without_callback_handler(storage: storage) + result = Flow.new(provider: provider).finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => state }, + ) + + assert_equal(:authorized, result) + assert_equal("test-token-from-flow", provider.access_token) + assert_equal(@auth_base, provider.tokens["issuer"]) + assert_nil(storage.pending_authorization(state)) + + assert_requested(:post, "#{@auth_base}/token", times: 1) do |req| + form = URI.decode_www_form(req.body).to_h + form["grant_type"] == "authorization_code" && + form["code"] == "auth-code" && + form["code_verifier"] == verifier && + form["redirect_uri"] == REDIRECT_URI && + form["resource"] == @server_url && + form["client_id"] == "test-client" + end + + # The recorded metadata binds the exchange, so discovery and registration do not run again. + assert_not_requested(:get, @prm_url) + assert_not_requested(:get, @as_metadata_url) + assert_not_requested(:post, "#{@auth_base}/register") + end + + def test_finish_accepts_symbol_keys + storage = JSONStorage.new + state = begin_authorization(storage) + + result = Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { code: "auth-code", state: state }, + ) + + assert_equal(:authorized, result) + end + + def test_finish_lets_only_one_of_two_concurrent_callbacks_redeem_the_code + storage = BarrierStorage.new(2) + state = begin_authorization(storage) + + outcomes = Array.new(2) do + Thread.new do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => state }, + ) + rescue Flow::AuthorizationError => e + e + end + end.map(&:value) + + assert_equal(1, outcomes.count(:authorized)) + refused = outcomes.grep(Flow::AuthorizationError) + assert_equal(1, refused.size) + assert_equal(Flow::UNKNOWN_PENDING_AUTHORIZATION_MESSAGE, refused.first.message) + assert_requested(:post, "#{@auth_base}/token", times: 1) + end + + def test_finish_refuses_when_storage_does_not_return_the_consumed_entry + storage = CountingDeleteStorage.new + state = begin_authorization(storage) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => state }, + ) + end + + assert_equal(Flow::UNKNOWN_PENDING_AUTHORIZATION_MESSAGE, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_finish_redeems_a_pending_authorization_only_once + storage = JSONStorage.new + state = begin_authorization(storage) + flow = Flow.new(provider: provider_without_callback_handler(storage: storage)) + flow.finish!(server_url: @server_url, callback_params: { "code" => "auth-code", "state" => state }) + + error = assert_raises(Flow::AuthorizationError) do + flow.finish!(server_url: @server_url, callback_params: { "code" => "auth-code", "state" => state }) + end + + assert_match(/matches no pending authorization/, error.message) + assert_requested(:post, "#{@auth_base}/token", times: 1) + end + + def test_finish_refuses_an_unknown_state_before_any_request + storage = JSONStorage.new + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => "forged-state" }, + ) + end + + assert_equal( + "Authorization callback `state` matches no pending authorization; it is unknown, already used, or expired.", + error.message, + ) + assert_not_requested(:any, /.*/) + end + + def test_finish_refuses_a_callback_without_a_usable_state + storage = JSONStorage.new + begin_authorization(storage) + flow = Flow.new(provider: provider_without_callback_handler(storage: storage)) + + [{ "code" => "auth-code" }, { "code" => "auth-code", "state" => "" }, { "code" => "auth-code", "state" => ["a", "b"] }, nil].each do |params| + error = assert_raises(Flow::AuthorizationError) do + flow.finish!(server_url: @server_url, callback_params: params) + end + + assert_equal("Authorization callback carried no `state`.", error.message) + end + + assert_equal(0, storage.pending_lookups) + end + + def test_finish_refuses_an_oversized_state_without_asking_storage + storage = JSONStorage.new + + assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => "a" * (Flow::CALLBACK_STATE_MAX_LENGTH + 1) }, + ) + end + + assert_equal(0, storage.pending_lookups) + end + + def test_finish_refuses_an_expired_pending_authorization_and_discards_it + storage = JSONStorage.new + state = begin_authorization(storage, pending_authorization_max_age: 60) + pending = storage.pending_authorization(state) + storage.save_pending_authorization(state, pending.merge("created_at" => pending["created_at"] - 61)) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage, pending_authorization_max_age: 60)).finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => state }, + ) + end + + assert_equal("The pending authorization has expired; start a new authorization.", error.message) + assert_nil(storage.pending_authorization(state)) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_finish_keeps_the_pending_authorization_when_iss_does_not_match + storage = JSONStorage.new + state = begin_authorization(storage) + flow = Flow.new(provider: provider_without_callback_handler(storage: storage)) + + error = assert_raises(Flow::AuthorizationError) do + flow.finish!( + server_url: @server_url, + callback_params: { "code" => "forged-code", "state" => state, "iss" => "https://evil.example.com" }, + ) + end + + assert_match(/`iss` does not match/, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + + # The forged callback did not burn the verifier: the legitimate callback still completes. + result = flow.finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => state, "iss" => @auth_base }, + ) + + assert_equal(:authorized, result) + end + + def test_finish_refuses_a_missing_iss_when_the_authorization_server_advertises_it + stub_as_metadata(authorization_response_iss_parameter_supported: true) + storage = JSONStorage.new + state = begin_authorization(storage) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => state }, + ) + end + + assert_match(/carried no `iss`/, error.message) + refute_nil(storage.pending_authorization(state)) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_finish_reports_an_authorization_error_response_after_the_iss_check + storage = JSONStorage.new + state = begin_authorization(storage) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { + "error" => "access_denied", + "error_description" => "The user declined", + "state" => state, + "iss" => @auth_base, + }, + ) + end + + assert_equal( + "The authorization server returned an error to the authorization callback. access_denied: The user declined", + error.message, + ) + assert_equal("access_denied", error.error) + assert_equal("The user declined", error.error_description) + assert_nil(storage.pending_authorization(state)) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_finish_does_not_surface_an_error_response_whose_iss_does_not_match + storage = JSONStorage.new + state = begin_authorization(storage) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { + "error" => "access_denied", + "error_description" => "Call support at 555-0100", + "state" => state, + "iss" => "https://evil.example.com", + }, + ) + end + + assert_match(/`iss` does not match/, error.message) + refute_includes(error.message, "555-0100") + refute_nil(storage.pending_authorization(state)) + end + + def test_finish_refuses_a_different_mcp_server + storage = JSONStorage.new + state = begin_authorization(storage) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: "https://other.example.com/mcp", + callback_params: { "code" => "auth-code", "state" => state }, + ) + end + + assert_equal("The pending authorization was started for a different MCP server.", error.message) + refute_nil(storage.pending_authorization(state)) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_finish_refuses_when_the_client_registration_changed + storage = JSONStorage.new + state = begin_authorization(storage) + storage.save_client_information("client_id" => "another-client", "issuer" => @auth_base) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => state }, + ) + end + + assert_equal("The client registration changed after the authorization began; start a new authorization.", error.message) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_finish_redeems_the_code_with_a_client_id_metadata_document_url + stub_as_metadata(client_id_metadata_document_supported: true) + cimd_url = "https://app.example.com/oauth/client-metadata.json" + storage = JSONStorage.new + state = begin_authorization(storage, client_id_metadata_document_url: cimd_url) + + provider = provider_without_callback_handler(storage: storage, client_id_metadata_document_url: cimd_url) + result = Flow.new(provider: provider).finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => state }, + ) + + assert_equal(:authorized, result) + assert_not_requested(:post, "#{@auth_base}/register") + assert_requested(:post, "#{@auth_base}/token") do |req| + URI.decode_www_form(req.body).to_h["client_id"] == cimd_url + end + end + + def test_finish_checks_the_recorded_endpoints_again + storage = JSONStorage.new + state = begin_authorization(storage) + pending = storage.pending_authorization(state) + tampered = pending.merge( + "authorization_server_metadata" => pending["authorization_server_metadata"].merge("token_endpoint" => "http://auth.example.com/token"), + ) + storage.save_pending_authorization(state, tampered) + + assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => state }, + ) + end + + assert_not_requested(:post, "http://auth.example.com/token") + end + + def test_finish_refuses_a_malformed_pending_authorization + storage = JSONStorage.new + state = begin_authorization(storage) + storage.save_pending_authorization(state, storage.pending_authorization(state).merge("code_verifier" => nil)) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { "code" => "auth-code", "state" => state }, + ) + end + + assert_match(/matches no pending authorization/, error.message) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_finish_refuses_a_callback_without_a_code + storage = JSONStorage.new + state = begin_authorization(storage) + + error = assert_raises(Flow::AuthorizationError) do + Flow.new(provider: provider_without_callback_handler(storage: storage)).finish!( + server_url: @server_url, + callback_params: { "state" => state }, + ) + end + + assert_equal("Authorization callback carried no authorization code.", error.message) + assert_nil(storage.pending_authorization(state)) + assert_not_requested(:post, "#{@auth_base}/token") + end + + def test_finish_refuses_a_provider_with_a_callback_handler + provider = Provider.new( + client_metadata: client_metadata, + redirect_uri: REDIRECT_URI, + redirect_handler: ->(_url) {}, + callback_handler: -> { ["code", "state"] }, + ) + + assert_raises(ArgumentError) do + Flow.new(provider: provider).finish!(server_url: @server_url, callback_params: { "code" => "c", "state" => "s" }) + end + end + + private + + def client_metadata + { + client_name: "ruby-sdk-test", + redirect_uris: [REDIRECT_URI], + grant_types: ["authorization_code"], + response_types: ["code"], + token_endpoint_auth_method: "none", + } + end + + def provider_without_callback_handler(storage:, **options) + Provider.new( + client_metadata: client_metadata, + redirect_uri: REDIRECT_URI, + redirect_handler: ->(url) { @redirected_to = url }, + storage: storage, + **options, + ) + end + + # Runs `run!` and returns the `state` the authorization server would echo back on the redirect. + def begin_authorization(storage, **options) + result = Flow.new(provider: provider_without_callback_handler(storage: storage, **options)).run!( + server_url: @server_url, + resource_metadata_url: @prm_url, + ) + assert_equal(:redirect, result) + + URI.decode_www_form(@redirected_to.query).to_h.fetch("state") + end + + def stub_as_metadata(**extra) + stub_request(:get, @as_metadata_url).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"], + **extra, + ), + ) + end + end + end + end +end diff --git a/test/mcp/client/oauth/provider_test.rb b/test/mcp/client/oauth/provider_test.rb index 7425131f..6b6d0226 100644 --- a/test/mcp/client/oauth/provider_test.rb +++ b/test/mcp/client/oauth/provider_test.rb @@ -231,6 +231,76 @@ def test_initialize_rejects_token_request_params_that_the_sdk_sets_itself assert_includes(error.message, '"code"') end + + def test_initialize_accepts_a_missing_callback_handler + arguments = args_for("https://app.example.com/callback") + arguments.delete(:callback_handler) + provider = Provider.new(**arguments) + + assert_nil(provider.callback_handler) + assert_equal(Provider::DEFAULT_PENDING_AUTHORIZATION_MAX_AGE, provider.pending_authorization_max_age) + end + + def test_delete_pending_authorization_returns_the_removed_entry + arguments = args_for("https://app.example.com/callback") + arguments.delete(:callback_handler) + provider = Provider.new(**arguments) + provider.save_pending_authorization("state-1", { "code_verifier" => "v1" }) + + assert_equal({ "code_verifier" => "v1" }, provider.delete_pending_authorization("state-1")) + assert_nil(provider.delete_pending_authorization("state-1")) + end + + def test_initialize_rejects_storage_that_cannot_hold_a_pending_authorization_without_a_callback_handler + storage_class = Class.new do + attr_accessor :tokens, :client_information + + def save_tokens(tokens) + @tokens = tokens + end + + def save_client_information(info) + @client_information = info + end + end + arguments = args_for("https://app.example.com/callback") + arguments.delete(:callback_handler) + + error = assert_raises(Provider::PendingAuthorizationStorageError) do + Provider.new(**arguments, storage: storage_class.new) + end + + assert_includes(error.message, "save_pending_authorization, pending_authorization, delete_pending_authorization") + end + + def test_initialize_accepts_storage_without_pending_authorization_methods_when_a_callback_handler_is_given + storage = Object.new + + provider = Provider.new(**args_for("https://app.example.com/callback"), storage: storage) + + assert_same(storage, provider.storage) + end + + def test_initialize_rejects_a_pending_authorization_max_age_that_is_not_a_positive_integer + [0, -1, 1.5, "600", nil].each do |max_age| + assert_raises(ArgumentError) do + Provider.new(**args_for("https://app.example.com/callback"), pending_authorization_max_age: max_age) + end + end + end + + def test_in_memory_storage_keeps_pending_authorizations_by_state + storage = InMemoryStorage.new + storage.save_pending_authorization("state-1", { "code_verifier" => "v1" }) + storage.save_pending_authorization("state-2", { "code_verifier" => "v2" }) + + assert_equal({ "code_verifier" => "v1" }, storage.pending_authorization("state-1")) + + storage.delete_pending_authorization("state-1") + + assert_nil(storage.pending_authorization("state-1")) + assert_equal({ "code_verifier" => "v2" }, storage.pending_authorization("state-2")) + end end end end