diff --git a/lib/ruby_llm/mcp/auth/client_registrar.rb b/lib/ruby_llm/mcp/auth/client_registrar.rb index 88188ad..041065c 100644 --- a/lib/ruby_llm/mcp/auth/client_registrar.rb +++ b/lib/ruby_llm/mcp/auth/client_registrar.rb @@ -123,7 +123,8 @@ def post_registration(server_metadata, metadata) def parse_registered_metadata(data, redirect_uri) ClientMetadata.new( redirect_uris: data["redirect_uris"] || [redirect_uri], - token_endpoint_auth_method: data["token_endpoint_auth_method"] || "none", + token_endpoint_auth_method: data["token_endpoint_auth_method"] || + (data["client_secret"] ? "client_secret_post" : "none"), grant_types: data["grant_types"] || %w[authorization_code refresh_token], response_types: data["response_types"] || ["code"], scope: data["scope"], diff --git a/lib/ruby_llm/mcp/auth/token_manager.rb b/lib/ruby_llm/mcp/auth/token_manager.rb index a75c7c1..d9593d3 100644 --- a/lib/ruby_llm/mcp/auth/token_manager.rb +++ b/lib/ruby_llm/mcp/auth/token_manager.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "base64" + module RubyLLM module MCP module Auth @@ -26,8 +28,8 @@ def exchange_authorization_code(server_metadata, client_info, code, pkce, server registered_redirect_uri = client_info.metadata.redirect_uris.first params = build_auth_code_params(client_info, code, pkce, registered_redirect_uri, server_url) - response = post_token_exchange(server_metadata, params) - response = retry_if_redirect_mismatch(response, server_metadata, params, registered_redirect_uri) + response = post_token_exchange(server_metadata, params, client_info) + response = retry_if_redirect_mismatch(response, server_metadata, params, registered_redirect_uri, client_info) validate_token_response!(response, "Token exchange") parse_token_response(response) @@ -45,12 +47,11 @@ def exchange_client_credentials(server_metadata, client_info, scope, server_url) params = { grant_type: "client_credentials", client_id: client_info.client_id, - client_secret: client_info.client_secret, scope: scope, resource: server_url }.compact - response = post_token_exchange(server_metadata, params) + response = post_token_exchange(server_metadata, params, client_info) validate_token_response!(response, "Token exchange") parse_token_response(response) end @@ -67,7 +68,7 @@ def refresh_token(server_metadata, client_info, token, server_url) logger.debug("Refreshing access token") params = build_refresh_params(client_info, token, server_url) - response = post_token_refresh(server_metadata, params) + response = post_token_refresh(server_metadata, params, client_info) # Return nil on error responses return nil if response.is_a?(HTTPX::ErrorResponse) @@ -100,7 +101,7 @@ def refresh_token(server_metadata, client_info, token, server_url) # @param server_url [String] MCP server URL # @return [Hash] token exchange parameters def build_auth_code_params(client_info, code, pkce, redirect_uri, server_url) - params = { + { grant_type: "authorization_code", code: code, redirect_uri: redirect_uri, @@ -108,9 +109,6 @@ def build_auth_code_params(client_info, code, pkce, redirect_uri, server_url) code_verifier: pkce.code_verifier, resource: server_url } - - add_client_secret_if_needed(params, client_info) - params end # Build parameters for token refresh @@ -119,49 +117,52 @@ def build_auth_code_params(client_info, code, pkce, redirect_uri, server_url) # @param server_url [String] MCP server URL # @return [Hash] refresh parameters def build_refresh_params(client_info, token, server_url) - params = { + { grant_type: "refresh_token", refresh_token: token.refresh_token, client_id: client_info.client_id, resource: server_url } - - add_client_secret_if_needed(params, client_info) - params end - # Add client secret to params if needed - # @param params [Hash] token request parameters - # @param client_info [ClientInfo] client info - def add_client_secret_if_needed(params, client_info) + # Apply client authentication per RFC 6749 §2.3 + # Supports "client_secret_post" (secret in body), "client_secret_basic" (HTTP Basic), + # and "none" (public client, no secret sent). + # @param params [Hash] token request form parameters (mutated in place) + # @param headers [Hash] HTTP headers (mutated in place) + # @param client_info [ClientInfo] client info with secret and auth method + def apply_client_auth!(params, headers, client_info) return unless client_info.client_secret - return unless client_info.metadata.token_endpoint_auth_method == "client_secret_post" - params[:client_secret] = client_info.client_secret + case client_info.metadata&.token_endpoint_auth_method + when "client_secret_post" + params[:client_secret] = client_info.client_secret + when "client_secret_basic" + credentials = Base64.strict_encode64("#{client_info.client_id}:#{client_info.client_secret}") + headers["Authorization"] = "Basic #{credentials}" + end end # Post token exchange request # @param server_metadata [ServerMetadata] server metadata # @param params [Hash] form parameters + # @param client_info [ClientInfo, nil] client info for authentication # @return [HTTPX::Response] HTTP response - def post_token_exchange(server_metadata, params) - http_client.post( - server_metadata.token_endpoint, - headers: { "Content-Type" => "application/x-www-form-urlencoded" }, - form: params - ) + def post_token_exchange(server_metadata, params, client_info = nil) + headers = { "Content-Type" => "application/x-www-form-urlencoded" } + apply_client_auth!(params, headers, client_info) if client_info + http_client.post(server_metadata.token_endpoint, headers:, form: params) end # Post token refresh request # @param server_metadata [ServerMetadata] server metadata # @param params [Hash] form parameters + # @param client_info [ClientInfo, nil] client info for authentication # @return [HTTPX::Response] HTTP response - def post_token_refresh(server_metadata, params) - response = http_client.post( - server_metadata.token_endpoint, - headers: { "Content-Type" => "application/x-www-form-urlencoded" }, - form: params - ) + def post_token_refresh(server_metadata, params, client_info = nil) + headers = { "Content-Type" => "application/x-www-form-urlencoded" } + apply_client_auth!(params, headers, client_info) if client_info + response = http_client.post(server_metadata.token_endpoint, headers:, form: params) if response.is_a?(HTTPX::ErrorResponse) logger.warn("Token refresh failed: #{response.error&.message || 'Request failed'}") @@ -176,8 +177,9 @@ def post_token_refresh(server_metadata, params) # @param server_metadata [ServerMetadata] server metadata # @param params [Hash] exchange parameters # @param registered_redirect_uri [String] registered redirect URI + # @param client_info [ClientInfo] client info for authentication # @return [HTTPX::Response] response (possibly retried) - def retry_if_redirect_mismatch(response, server_metadata, params, registered_redirect_uri) + def retry_if_redirect_mismatch(response, server_metadata, params, registered_redirect_uri, client_info) # Don't retry on error responses return response if response.is_a?(HTTPX::ErrorResponse) return response if response.status == 200 @@ -188,7 +190,7 @@ def retry_if_redirect_mismatch(response, server_metadata, params, registered_red logger.warn("Redirect URI mismatch, retrying with: #{redirect_hint[:expected]}") params[:redirect_uri] = redirect_hint[:expected] - post_token_exchange(server_metadata, params) + post_token_exchange(server_metadata, params, client_info) end # Validate token response diff --git a/spec/ruby_llm/mcp/auth/client_registrar_spec.rb b/spec/ruby_llm/mcp/auth/client_registrar_spec.rb index 41e64cb..4ec3e4c 100644 --- a/spec/ruby_llm/mcp/auth/client_registrar_spec.rb +++ b/spec/ruby_llm/mcp/auth/client_registrar_spec.rb @@ -146,6 +146,54 @@ end end + context "when server omits token_endpoint_auth_method but returns client_secret" do + let(:registration_response) do + { + "client_id" => "test_client_id", + "client_secret" => "test_secret", + "redirect_uris" => [redirect_uri] + } + end + + it "defaults to client_secret_post" do + result = registrar.register(server_url, server_metadata, :authorization_code, redirect_uri, scope) + + expect(result.metadata.token_endpoint_auth_method).to eq("client_secret_post") + end + end + + context "when server omits both token_endpoint_auth_method and client_secret" do + let(:registration_response) do + { + "client_id" => "test_client_id", + "redirect_uris" => [redirect_uri] + } + end + + it "defaults to none for public clients" do + result = registrar.register(server_url, server_metadata, :authorization_code, redirect_uri, scope) + + expect(result.metadata.token_endpoint_auth_method).to eq("none") + end + end + + context "when server explicitly returns token_endpoint_auth_method" do + let(:registration_response) do + { + "client_id" => "test_client_id", + "client_secret" => "test_secret", + "redirect_uris" => [redirect_uri], + "token_endpoint_auth_method" => "client_secret_post" + } + end + + it "uses the server-provided value" do + result = registrar.register(server_url, server_metadata, :authorization_code, redirect_uri, scope) + + expect(result.metadata.token_endpoint_auth_method).to eq("client_secret_post") + end + end + context "when server changes redirect_uri" do let(:registration_response) do { diff --git a/spec/ruby_llm/mcp/auth/token_manager_spec.rb b/spec/ruby_llm/mcp/auth/token_manager_spec.rb index 4fd4237..eb19a13 100644 --- a/spec/ruby_llm/mcp/auth/token_manager_spec.rb +++ b/spec/ruby_llm/mcp/auth/token_manager_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "spec_helper" +require "base64" RSpec.describe RubyLLM::MCP::Auth::TokenManager do let(:http_client) { instance_double(HTTPX::Session) } @@ -103,6 +104,33 @@ end end + context "with client_secret_basic auth (RFC 6749 §2.3.1)" do + let(:client_metadata_basic) do + RubyLLM::MCP::Auth::ClientMetadata.new( + redirect_uris: ["http://localhost:8080/callback"], + token_endpoint_auth_method: "client_secret_basic" + ) + end + + let(:client_info_basic) do + RubyLLM::MCP::Auth::ClientInfo.new( + client_id: "test_client_id", + client_secret: "test_secret", + metadata: client_metadata_basic + ) + end + + it "sends credentials via HTTP Basic Authorization header" do + manager.exchange_authorization_code(server_metadata, client_info_basic, code, pkce, server_url) + + expect(http_client).to have_received(:post) do |_url, options| + expected = Base64.strict_encode64("test_client_id:test_secret") + expect(options[:headers]["Authorization"]).to eq("Basic #{expected}") + expect(options[:form]).not_to have_key(:client_secret) + end + end + end + context "with redirect URI mismatch" do let(:error_response) do { @@ -136,11 +164,18 @@ describe "#exchange_client_credentials" do let(:scope) { "mcp:read" } + let(:client_metadata_with_secret) do + RubyLLM::MCP::Auth::ClientMetadata.new( + redirect_uris: ["http://localhost:8080/callback"], + token_endpoint_auth_method: "client_secret_post" + ) + end + let(:client_info_with_secret) do RubyLLM::MCP::Auth::ClientInfo.new( client_id: "test_client_id", client_secret: "test_secret", - metadata: client_metadata + metadata: client_metadata_with_secret ) end