Fix client_secret not sent during OAuth token exchange

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"],

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,17 +101,14 @@ 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,
             client_id: client_info.client_id,
             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

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
         {

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