Merge pull request #119 from patvice/token-manager-raise-oauth-error-response

Align token error handling and tests with RFC 6749 section 5.2

Author: patvice

lib/ruby_llm/mcp/auth/token_manager.rb

@@ -71,9 +71,17 @@ def refresh_token(server_metadata, client_info, token, server_url)
 
           # Return nil on error responses
           return nil if response.is_a?(HTTPX::ErrorResponse)
-          return nil unless response.status == 200
+
+          if response.status != 200
+            oauth_error = extract_oauth_error(response.body.to_s)
+            raise_oauth_error!("Token refresh", oauth_error, response.status) if oauth_error
+            return nil
+          end
 
           parse_refresh_response(response, token)
+        rescue Errors::TransportError => e
+          logger.warn(e.message)
+          nil
         rescue JSON::ParserError => e
           logger.warn("Invalid token refresh response: #{e.message}")
           nil
@@ -194,6 +202,9 @@ def validate_token_response!(response, context)
             raise Errors::TransportError.new(message: "#{context} failed: #{error_message}")
           end
 
+          oauth_error = extract_oauth_error(response.body.to_s)
+          raise_oauth_error!(context, oauth_error, response.status) if oauth_error
+
           return if response.status == 200
 
           raise Errors::TransportError.new(
@@ -207,8 +218,18 @@ def validate_token_response!(response, context)
         # @return [Token] parsed token
         def parse_token_response(response)
           data = JSON.parse(response.body.to_s)
+          raise_oauth_error!("Token exchange", extract_oauth_error(data), response.status)
+
+          access_token = data["access_token"]
+          if access_token.nil? || access_token.empty?
+            raise Errors::TransportError.new(
+              message: "Token exchange failed: invalid token response (missing access_token)",
+              code: response.status
+            )
+          end
+
           Token.new(
-            access_token: data["access_token"],
+            access_token: access_token,
             token_type: data["token_type"] || "Bearer",
             expires_in: data["expires_in"],
             scope: data["scope"],
@@ -222,14 +243,64 @@ def parse_token_response(response)
         # @return [Token] new token
         def parse_refresh_response(response, old_token)
           data = JSON.parse(response.body.to_s)
+          raise_oauth_error!("Token refresh", extract_oauth_error(data), response.status)
+
+          access_token = data["access_token"]
+          if access_token.nil? || access_token.empty?
+            raise Errors::TransportError.new(
+              message: "Token refresh failed: invalid token response (missing access_token)",
+              code: response.status
+            )
+          end
+
           Token.new(
-            access_token: data["access_token"],
+            access_token: access_token,
             token_type: data["token_type"] || "Bearer",
             expires_in: data["expires_in"],
             scope: data["scope"],
             refresh_token: data["refresh_token"] || old_token.refresh_token
           )
         end
+
+        # Extract OAuth error fields from JSON response data
+        # @param source [String, Hash] response body string or parsed JSON hash
+        # @return [Hash, nil] OAuth error fields or nil
+        def extract_oauth_error(source)
+          data = source.is_a?(Hash) ? source : JSON.parse(source)
+          error = data["error"] || data[:error]
+          return nil unless error
+
+          {
+            error: error,
+            error_description: data["error_description"] || data[:error_description],
+            error_uri: data["error_uri"] || data[:error_uri]
+          }
+        rescue JSON::ParserError
+          nil
+        end
+
+        # Raise TransportError for OAuth error responses
+        # @param context [String] context for the error
+        # @param oauth_error [Hash, nil] OAuth error fields
+        # @param status_code [Integer, nil] HTTP response status code
+        # @raise [Errors::TransportError] when oauth_error is present
+        def raise_oauth_error!(context, oauth_error, status_code)
+          return unless oauth_error
+
+          error = oauth_error[:error]
+          description = oauth_error[:error_description]
+          error_uri = oauth_error[:error_uri]
+
+          message = "#{context} failed: OAuth error '#{error}'"
+          message += ": #{description}" if description
+          message += " (#{error_uri})" if error_uri
+
+          raise Errors::TransportError.new(
+            message: message,
+            code: status_code,
+            error: error
+          )
+        end
       end
     end
   end

spec/ruby_llm/mcp/auth/token_manager_spec.rb

@@ -174,6 +174,97 @@
         expect(options[:form][:scope]).to eq(scope)
       end
     end
+
+    context "when token endpoint returns OAuth error response" do
+      let(:oauth_error_response) do
+        {
+          "error" => "invalid_request",
+          "error_description" => "Missing required parameter: grant_type",
+          "error_uri" => "https://example.com/docs/oauth-errors#invalid_request"
+        }
+      end
+
+      before do
+        response = instance_double(HTTPX::Response, status: 400, body: oauth_error_response.to_json)
+        allow(http_client).to receive(:post).and_return(response)
+      end
+
+      it "raises TransportError with OAuth error details from RFC 6749 section 5.2 format" do
+        expect do
+          manager.exchange_client_credentials(server_metadata, client_info_with_secret, scope, server_url)
+        end.to raise_error(RubyLLM::MCP::Errors::TransportError) { |error|
+          expect(error.message).to include("OAuth error 'invalid_request'")
+          expect(error.message).to include("Missing required parameter: grant_type")
+          expect(error.code).to eq(400)
+          expect(error.error).to eq("invalid_request")
+        }
+      end
+    end
+
+    context "when token endpoint returns 401 invalid_client error response" do
+      let(:oauth_error_response) do
+        {
+          "error" => "invalid_client",
+          "error_description" => "Client authentication failed"
+        }
+      end
+
+      before do
+        response = instance_double(HTTPX::Response, status: 401, body: oauth_error_response.to_json)
+        allow(http_client).to receive(:post).and_return(response)
+      end
+
+      it "raises TransportError with RFC 6749 invalid_client semantics" do
+        expect do
+          manager.exchange_client_credentials(server_metadata, client_info_with_secret, scope, server_url)
+        end.to raise_error(RubyLLM::MCP::Errors::TransportError) { |error|
+          expect(error.message).to include("OAuth error 'invalid_client'")
+          expect(error.message).to include("Client authentication failed")
+          expect(error.code).to eq(401)
+          expect(error.error).to eq("invalid_client")
+        }
+      end
+    end
+
+    context "when token endpoint returns HTTP 200 with OAuth error payload" do
+      let(:oauth_error_response) do
+        {
+          "error" => "invalid_client",
+          "error_description" => "Client authentication failed."
+        }
+      end
+
+      before do
+        response = instance_double(HTTPX::Response, status: 200, body: oauth_error_response.to_json)
+        allow(http_client).to receive(:post).and_return(response)
+      end
+
+      it "raises TransportError instead of creating a token with missing access_token" do
+        expect do
+          manager.exchange_client_credentials(server_metadata, client_info_with_secret, scope, server_url)
+        end.to raise_error(RubyLLM::MCP::Errors::TransportError, /OAuth error 'invalid_client'/)
+      end
+    end
+
+    context "when token endpoint returns success status but no access token" do
+      let(:incomplete_response) do
+        {
+          "token_type" => "Bearer",
+          "expires_in" => 3600
+        }
+      end
+
+      before do
+        response = instance_double(HTTPX::Response, status: 200, body: incomplete_response.to_json)
+        allow(http_client).to receive(:post).and_return(response)
+      end
+
+      it "raises a clear TransportError for invalid token payload" do
+        expect do
+          manager.exchange_client_credentials(server_metadata, client_info_with_secret, scope, server_url)
+        end.to raise_error(RubyLLM::MCP::Errors::TransportError, /missing access_token/)
+      end
+    end
   end
 
   describe "#refresh_token" do
@@ -265,5 +356,51 @@
         expect(logger).to have_received(:warn).with(/Invalid token refresh response/)
       end
     end
+
+    context "when refresh response contains OAuth error fields" do
+      let(:oauth_error_response) do
+        {
+          "error" => "invalid_grant",
+          "error_description" => "Refresh token is expired"
+        }
+      end
+
+      before do
+        response = instance_double(HTTPX::Response, status: 200, body: oauth_error_response.to_json)
+        allow(http_client).to receive(:post).and_return(response)
+      end
+
+      it "returns nil and logs warning" do
+        result = manager.refresh_token(server_metadata, client_info, token, server_url)
+
+        expect(result).to be_nil
+        expect(logger).to have_received(:warn).with(
+          /Token refresh failed: OAuth error 'invalid_grant'/
+        )
+      end
+    end
+
+    context "when refresh endpoint returns non-200 with OAuth error payload" do
+      let(:oauth_error_response) do
+        {
+          "error" => "invalid_grant",
+          "error_description" => "Refresh token is expired"
+        }
+      end
+
+      before do
+        response = instance_double(HTTPX::Response, status: 400, body: oauth_error_response.to_json)
+        allow(http_client).to receive(:post).and_return(response)
+      end
+
+      it "returns nil and logs OAuth error details" do
+        result = manager.refresh_token(server_metadata, client_info, token, server_url)
+
+        expect(result).to be_nil
+        expect(logger).to have_received(:warn).with(
+          /Token refresh failed: OAuth error 'invalid_grant': Refresh token is expired/
+        )
+      end
+    end
   end
 end