Validate the issuer of machine-to-machine providers as an issuer URL

docs/_client/authorization.md

@@ -321,10 +321,12 @@ Keyword arguments:
 - `private_key`, `signing_algorithm`: Required with `private_key_jwt` - the key (a PEM string
   or `OpenSSL::PKey::PKey`, never written to `storage`) signs the client assertion with `"ES256"`
   or `"RS256"`; `client_secret` must not be set, because the private key is the credential.
-- `issuer`: Optional. The issuer identifier of the authorization server the credentials belong to. A different authorization server raises `Flow::AuthorizationError`.
+- `issuer`: Optional. The issuer identifier of the authorization server the credentials belong to, spelled as its metadata states it:
+  an `https` URL (`http` only on a loopback address) without query or fragment. A different authorization server raises `Flow::AuthorizationError`.
   A single trailing slash difference is accepted, as in the TypeScript SDK. When the protected resource metadata lists several authorization servers,
-  the one naming `issuer:` is used. Omitting `issuer:` is deprecated and emits a warning at construction. Without it, credentials go to whichever authorization server
-  the MCP server names, as before. To use credentials for another authorization server, construct a provider with that server's `issuer:` and credentials.
+  the one naming `issuer:` is used. Omitting `issuer:` is deprecated and emits a warning at construction.
+  Without it, credentials go to whichever authorization server the MCP server names, as before. To use credentials for another authorization server,
+  construct a provider with that server's `issuer:` and credentials.
 - `scope`, `storage`, `authorization_request_validator`, `token_request_params`, `http_client_customizer`: Optional, same meaning as on `Provider`.
   Use `token_request_params` for a parameter the authorization server requires on the `client_credentials` grant, such as Auth0's `audience`.
 

lib/mcp/client/oauth/client_credentials_provider.rb

@@ -35,7 +35,8 @@ module OAuth
       #   the server's `token_endpoint_auth_signing_alg_values_supported` fails
       #   loudly at construction instead of as a 401 (the TypeScript and Python SDKs
       #   also take the algorithm as an explicit option).
-      # - `issuer`  - String issuer identifier of the authorization server the credentials belong to.
+      # - `issuer`  - String issuer identifier of the authorization server the credentials belong to,
+      #   as its metadata states it: an `https` URL, or `http` on a loopback address, without query or fragment.
       #   A different authorization server raises `Flow::AuthorizationError`. Omitting this is deprecated
       #   and emits a warning; without it, credentials go to whichever authorization server the MCP server names.
       # - `scope`   - String of space-separated scopes to request when the server's
@@ -93,8 +94,9 @@ def initialize(
                 "the MCP server names. Pass the authorization server's issuer URL as `issuer:` so they are only sent there.",
               uplevel: 1,
             )
-          elsif !issuer.is_a?(String) || blank?(issuer)
-            raise InvalidCredentialsError, "issuer must be a non-empty String: the issuer identifier of the authorization server the credentials belong to."
+          elsif !Discovery.issuer_identifier?(issuer)
+            raise InvalidCredentialsError, "issuer must be the authorization server's issuer URL as its metadata states it, over HTTPS " \
+                                           "(HTTP only on a loopback address) and without query or fragment, got #{issuer.inspect}."
           end
 
           client_information = { "client_id" => client_id, "token_endpoint_auth_method" => token_endpoint_auth_method }

lib/mcp/client/oauth/cross_app_access_provider.rb

@@ -21,9 +21,10 @@ module OAuth
       #
       # Optional keyword arguments:
       #
-      # - `issuer`  - String issuer identifier of the MCP authorization server the credentials belong to.
-      #   A mismatch raises `Flow::AuthorizationError` before `assertion_provider` is called. Omitting this is
-      #   deprecated and emits a warning; without it, credentials go to whichever authorization server the MCP server names.
+      # - `issuer`  - String issuer identifier of the MCP authorization server the credentials belong to,
+      #   as its metadata states it: an `https` URL, or `http` on a loopback address, without query or fragment.
+      #   A mismatch raises `Flow::AuthorizationError` before `assertion_provider` is called. Omitting this is deprecated
+      #   and emits a warning; without it, credentials go to whichever authorization server the MCP server names.
       # - `scope`   - String of space-separated scopes to request when the server's `WWW-Authenticate` and
       #   the Protected Resource Metadata do not specify one.
       # - `storage` - Object responding to `tokens`, `save_tokens(tokens)`, `client_information`, and `save_client_information(info)`.
@@ -74,8 +75,9 @@ def initialize(
                 "the MCP server names. Pass the authorization server's issuer URL as `issuer:` so they are only sent there.",
               uplevel: 1,
             )
-          elsif !issuer.is_a?(String) || blank?(issuer)
-            raise InvalidConfigurationError, "issuer must be a non-empty String: the issuer identifier of the authorization server the credentials belong to."
+          elsif !Discovery.issuer_identifier?(issuer)
+            raise InvalidConfigurationError, "issuer must be the authorization server's issuer URL as its metadata states it, over HTTPS " \
+                                             "(HTTP only on a loopback address) and without query or fragment, got #{issuer.inspect}."
           end
 
           http_client_customizer = validated_http_client_customizer(http_client_customizer)

lib/mcp/client/oauth/discovery.rb

@@ -200,6 +200,20 @@ def secure_url?(url)
             false
           end
 
+          # Whether `value` can be an authorization server issuer identifier the flow could ever match: a String naming an `https` URL,
+          # or `http` on a loopback address as `secure_url?` allows, without query or fragment (RFC 8414 Section 2).
+          # The issuer of the metadata is compared as a String, so a value of another type would never equal it, and an issuer with
+          # a query or fragment is one no authorization server may publish.
+          def issuer_identifier?(value)
+            return false unless value.is_a?(String) && secure_url?(value)
+
+            uri = URI.parse(value)
+
+            uri.query.nil? && uri.fragment.nil?
+          rescue URI::InvalidURIError
+            false
+          end
+
           # Returns true when `url` and `other` share an origin: same scheme, same host,
           # and same port. Scheme and host compare case-insensitively, and an implicit default port compares
           # equal to the same port written out, because `URI::HTTP#port` already resolves to the default.

test/mcp/client/oauth/client_credentials_provider_test.rb

@@ -58,11 +58,29 @@ def test_initialize_stores_the_issuer_with_the_credentials
           assert_equal("https://auth.example.com", provider.client_information["issuer"])
         end
 
-        def test_initialize_rejects_an_issuer_that_is_not_a_non_empty_string
-          ["", " ", :issuer, false, 123].each do |issuer|
-            assert_raises(ClientCredentialsProvider::InvalidCredentialsError) do
+        def test_initialize_rejects_an_issuer_that_is_not_an_issuer_url
+          [
+            "", " ", :issuer, false, 123, :"https://auth.example.com", URI("https://auth.example.com"),
+            "auth.example.com", "ftp://auth.example.com", "https://", "https://auth.example.com/ metadata",
+            "https://auth.example.com/?tenant=1", "https://auth.example.com/#section",
+            "http://auth.example.com", "http://10.0.0.5", "http://localhost.example.com", "http://127.attacker.example"
+          ].each do |issuer|
+            error = assert_raises(ClientCredentialsProvider::InvalidCredentialsError, issuer.inspect) do
               ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", issuer: issuer)
             end
+
+            assert_match(/issuer must be the authorization server's issuer URL/, error.message)
+          end
+        end
+
+        def test_initialize_accepts_an_https_issuer_and_a_loopback_http_issuer
+          [
+            "https://auth.example.com", "https://auth.example.com/", "https://auth.example.com/tenant", "HTTPS://auth.example.com",
+            "http://127.0.0.1:9292", "http://localhost:8080/auth", "http://[::1]:9292"
+          ].each do |issuer|
+            provider = ClientCredentialsProvider.new(client_id: "cc-client", client_secret: "cc-secret", issuer: issuer)
+
+            assert_equal(issuer, provider.client_information["issuer"])
           end
         end
 

test/mcp/client/oauth/cross_app_access_provider_test.rb

@@ -54,16 +54,35 @@ def test_initialize_stores_the_issuer_with_the_credentials
           assert_equal("https://auth.example.com", provider.client_information["issuer"])
         end
 
-        def test_initialize_rejects_an_issuer_that_is_not_a_non_empty_string
-          ["", " ", :issuer, false, 123].each do |issuer|
-            assert_raises(CrossAppAccessProvider::InvalidConfigurationError) do
+        def test_initialize_rejects_an_issuer_that_is_not_an_issuer_url
+          [
+            "", " ", :issuer, false, 123, URI("https://auth.example.com"),
+            "auth.example.com", "ftp://auth.example.com", "https://", "https://auth.example.com/?tenant=1",
+            "http://auth.example.com", "http://10.0.0.5"
+          ].each do |issuer|
+            error = assert_raises(CrossAppAccessProvider::InvalidConfigurationError, issuer.inspect) do
               CrossAppAccessProvider.new(
                 client_id: "xaa-client",
                 client_secret: "xaa-secret",
                 assertion_provider: ->(**) { "id-jag" },
                 issuer: issuer,
               )
             end
+
+            assert_match(/issuer must be the authorization server's issuer URL/, error.message)
+          end
+        end
+
+        def test_initialize_accepts_an_https_issuer_and_a_loopback_http_issuer
+          ["https://auth.example.com", "HTTPS://auth.example.com", "http://127.0.0.1:9292", "http://[::1]:9292"].each do |issuer|
+            provider = CrossAppAccessProvider.new(
+              client_id: "xaa-client",
+              client_secret: "xaa-secret",
+              assertion_provider: ->(**) { "id-jag" },
+              issuer: issuer,
+            )
+
+            assert_equal(issuer, provider.client_information["issuer"])
           end
         end