diff --git a/docs/_client/authorization.md b/docs/_client/authorization.md index a673a198..30562ade 100644 --- a/docs/_client/authorization.md +++ b/docs/_client/authorization.md @@ -159,6 +159,27 @@ provider = MCP::Client::OAuth::Provider.new( ) ``` +### Token endpoint errors + +When a token exchange or refresh fails, `MCP::Client::OAuth::Flow::AuthorizationError` includes the HTTP status and +the authorization server's `error` and `error_description` from [RFC 6749 Section 5.2](https://www.rfc-editor.org/rfc/rfc6749#section-5.2). +For example: + +```text +Token endpoint returned status 400. invalid_request: Client must not use multiple authentication methods +``` + +The exception exposes `http_status`, `error`, and `error_description` readers for structured diagnostics. Missing or non-string +diagnostic fields are `nil`; non-JSON responses retain the status-only message. Other authorization failures have `nil` readers. +An `invalid_grant` response still raises `Flow::InvalidGrantError`, a subclass of `Flow::AuthorizationError`, so refresh-token +recovery behavior is unchanged. + +Diagnostic fields are limited to 128 characters for `error` and 512 for `error_description`, including a trailing `...` when +truncated. Characters outside the RFC's printable ASCII set are replaced with spaces, and surrounding whitespace is removed. +The SDK excludes all other response fields, including `error_uri`, and does not include the raw response body in these errors. +Descriptions are provider-controlled text, not guaranteed to be free of sensitive information; apply your application's logging +and redaction policy before persisting them or displaying them to users. + ### Client Credentials Grant For a confidential machine-to-machine client (no user, no browser redirect), use `MCP::Client::OAuth::ClientCredentialsProvider` instead of `Provider`. diff --git a/lib/mcp/client/oauth/flow.rb b/lib/mcp/client/oauth/flow.rb index 5b4c0c1d..5d2e915f 100644 --- a/lib/mcp/client/oauth/flow.rb +++ b/lib/mcp/client/oauth/flow.rb @@ -14,7 +14,16 @@ module OAuth # `Provider`; this class consumes a Provider plus signal data extracted from # the failing response (resource_metadata URL, scope challenge). class Flow - class AuthorizationError < StandardError; end + class AuthorizationError < StandardError + attr_reader :http_status, :error, :error_description + + def initialize(message = nil, http_status: nil, error: nil, error_description: nil) + super(message) + @http_status = http_status + @error = error + @error_description = error_description + end + end # Raised specifically when the token endpoint rejects a grant with # `error: "invalid_grant"` (RFC 6749 §5.2). Callers use this to @@ -1062,11 +1071,7 @@ def post_to_token_endpoint(as_metadata:, client_info:, form:) end if response.status < 200 || response.status >= 300 - if token_endpoint_error_code(response) == "invalid_grant" - raise InvalidGrantError, "Token endpoint rejected the grant: invalid_grant." - end - - raise AuthorizationError, "Token endpoint returned status #{response.status}." + raise token_endpoint_error(response) end parsed = begin @@ -1087,17 +1092,35 @@ def post_to_token_endpoint(as_metadata:, client_info:, form:) parsed end - # Extracts the `error` code from an RFC 6749 §5.2 error response body - # when one is parseable. Returns nil on any parse failure or when - # the body is not JSON. - def token_endpoint_error_code(response) - body = response_body_string(response).to_s - return if body.empty? + # Surface only RFC 6749 §5.2 diagnostic fields, never the raw response, + # which may contain tokens or other credentials. Classify the original + # code so sanitization cannot turn malformed input into invalid_grant. + def token_endpoint_error(response) + parsed = begin + JSON.parse(response_body_string(response)) + rescue JSON::ParserError + nil + end + parsed = {} unless parsed.is_a?(Hash) + + error_class = parsed["error"] == "invalid_grant" ? InvalidGrantError : AuthorizationError + error = token_endpoint_diagnostic(parsed["error"], limit: 128) + description = token_endpoint_diagnostic(parsed["error_description"], limit: 512) + message = "Token endpoint returned status #{response.status}." + message += " #{[error, description].compact.join(": ")}" if error || description + + error_class.new(message, http_status: response.status, error: error, error_description: description) + end + + def token_endpoint_diagnostic(value, limit:) + return unless value.is_a?(String) - parsed = JSON.parse(body) - parsed["error"] if parsed.is_a?(Hash) - rescue JSON::ParserError - nil + # RFC 6749 permits printable ASCII except double quotes and backslashes. + # Replace other characters to keep provider text on one log line. + value = value.gsub(/[^\x20-\x21\x23-\x5B\x5D-\x7E]/, " ").strip + return if value.empty? + + value.length > limit ? "#{value[0, limit - 3]}..." : value end # Per RFC 6749 Section 2.3.1, the `client_id` and `client_secret` MUST be diff --git a/test/mcp/client/oauth/flow_test.rb b/test/mcp/client/oauth/flow_test.rb index 326dbd86..bd82fd3c 100644 --- a/test/mcp/client/oauth/flow_test.rb +++ b/test/mcp/client/oauth/flow_test.rb @@ -2731,7 +2731,123 @@ def test_refresh_raises_invalid_grant_error_when_token_endpoint_says_invalid_gra provider.save_client_information("client_id" => "test-client") provider.save_tokens("access_token" => "stale-at", "refresh_token" => "revoked-rt") - assert_raises(Flow::InvalidGrantError) do + error = assert_raises(Flow::InvalidGrantError) do + Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) + end + assert_equal(400, error.http_status) + assert_equal("invalid_grant", error.error) + assert_equal("refresh token expired", error.error_description) + assert_equal("Token endpoint returned status 400. invalid_grant: refresh token expired", error.message) + end + + def test_token_exchange_preserves_oauth_diagnostics + stub_request(:post, "#{@auth_base}/token").to_return( + status: 400, + body: JSON.generate( + error: "invalid_request", + error_description: "Client must not use multiple authentication methods", + access_token: "do-not-log-access-token", + refresh_token: "do-not-log-refresh-token", + client_secret: "do-not-log-client-secret", + error_uri: "https://auth.example.com/error?secret=do-not-log", + ), + ) + + error = assert_raises(Flow::AuthorizationError) do + capture_authorization_scope(grant_types: ["authorization_code"]) + end + + assert_equal(400, error.http_status) + assert_equal("invalid_request", error.error) + assert_equal("Client must not use multiple authentication methods", error.error_description) + assert_equal( + "Token endpoint returned status 400. invalid_request: Client must not use multiple authentication methods", + error.message, + ) + end + + def test_refresh_preserves_oauth_diagnostics + error = refresh_token_endpoint_error( + JSON.generate(error: "invalid_client", error_description: "Client authentication failed"), + status: 401, + ) + + assert_instance_of(Flow::AuthorizationError, error) + assert_equal(401, error.http_status) + assert_equal("invalid_client", error.error) + assert_equal("Client authentication failed", error.error_description) + assert_equal("Token endpoint returned status 401. invalid_client: Client authentication failed", error.message) + end + + def test_token_endpoint_errors_fall_back_for_malformed_bodies + ["", "secret", "{broken", "null", "[]", '"secret"', "42"].each do |body| + error = refresh_token_endpoint_error(body) + + assert_instance_of(Flow::AuthorizationError, error) + assert_equal(400, error.http_status) + assert_nil(error.error) + assert_nil(error.error_description) + assert_equal("Token endpoint returned status 400.", error.message) + end + end + + def test_token_endpoint_errors_ignore_non_string_and_empty_fields + [nil, 42, true, [], { secret: "hidden" }, "", " \r\n\t"].each do |value| + error = refresh_token_endpoint_error(JSON.generate(error: value, error_description: value)) + + assert_nil(error.error) + assert_nil(error.error_description) + assert_equal("Token endpoint returned status 400.", error.message) + end + end + + def test_token_endpoint_errors_preserve_optional_fields_independently + error = refresh_token_endpoint_error(JSON.generate(error: "provider_extension")) + assert_equal("provider_extension", error.error) + assert_nil(error.error_description) + assert_equal("Token endpoint returned status 400. provider_extension", error.message) + + error = refresh_token_endpoint_error(JSON.generate(error_description: "Details without a code")) + assert_nil(error.error) + assert_equal("Details without a code", error.error_description) + assert_equal("Token endpoint returned status 400. Details without a code", error.message) + end + + def test_token_endpoint_errors_sanitize_without_changing_grant_classification + error = refresh_token_endpoint_error( + JSON.generate(error: "invalid_grant\n", error_description: "expired\r\n\t\e\u0000\"\\\u2028token"), + ) + + assert_instance_of(Flow::AuthorizationError, error) + assert_equal("invalid_grant", error.error) + assert_equal("expired token", error.error_description) + assert_equal("Token endpoint returned status 400. invalid_grant: expired token", error.message) + end + + def test_token_endpoint_errors_bound_diagnostic_lengths + error = refresh_token_endpoint_error(JSON.generate(error: "e" * 200, error_description: "d" * 1000)) + + assert_equal("#{"e" * 125}...", error.error) + assert_equal("#{"d" * 509}...", error.error_description) + assert_equal("Token endpoint returned status 400. #{error.error}: #{error.error_description}", error.message) + end + + def test_other_authorization_errors_have_no_token_endpoint_diagnostics + error = Flow::AuthorizationError.new("Discovery failed") + + assert_equal("Discovery failed", error.message) + assert_nil(error.http_status) + assert_nil(error.error) + assert_nil(error.error_description) + end + + def refresh_token_endpoint_error(body, status: 400) + stub_request(:post, "#{@auth_base}/token").to_return(status: status, body: body) + provider = ssrf_test_provider + provider.save_client_information("client_id" => "test-client") + provider.save_tokens("access_token" => "stale-at", "refresh_token" => "saved-rt") + + assert_raises(Flow::AuthorizationError) do Flow.new(provider: provider).refresh!(server_url: @server_url, resource_metadata_url: @prm_url) end end