Add Client Authentication Methods Registry

.rspec

@@ -1 +1 @@
---colour
+--colour --format documentation

lib/doorkeeper.rb

@@ -7,6 +7,7 @@
 #
 module Doorkeeper
   autoload :Errors, "doorkeeper/errors"
+  autoload :ClientAuthentication, "doorkeeper/client_authentication"
   autoload :GrantFlow, "doorkeeper/grant_flow"
   autoload :OAuth, "doorkeeper/oauth"
   autoload :Rake, "doorkeeper/rake"
@@ -33,7 +34,6 @@ module Request
     autoload :RefreshToken, "doorkeeper/request/refresh_token"
     autoload :Token, "doorkeeper/request/token"
   end
-
   module RevocableTokens
     autoload :RevocableAccessToken, "doorkeeper/revocable_tokens/revocable_access_token"
     autoload :RevocableRefreshToken, "doorkeeper/revocable_tokens/revocable_refresh_token"
@@ -62,17 +62,19 @@ module OAuth
     autoload :TokenRequest, "doorkeeper/oauth/token_request"
     autoload :TokenResponse, "doorkeeper/oauth/token_response"
 
+    module ClientAuthentication
+      autoload :None, "doorkeeper/oauth/client_authentication/none"
+      autoload :ClientSecretBasic, "doorkeeper/oauth/client_authentication/client_secret_basic"
+      autoload :ClientSecretPost, "doorkeeper/oauth/client_authentication/client_secret_post"
+    end
+
     module Authorization
       autoload :Code, "doorkeeper/oauth/authorization/code"
       autoload :Context, "doorkeeper/oauth/authorization/context"
       autoload :Token, "doorkeeper/oauth/authorization/token"
       autoload :URIBuilder, "doorkeeper/oauth/authorization/uri_builder"
     end
 
-    class Client
-      autoload :Credentials, "doorkeeper/oauth/client/credentials"
-    end
-
     module ClientCredentials
       autoload :Validator, "doorkeeper/oauth/client_credentials/validator"
       autoload :Creator, "doorkeeper/oauth/client_credentials/creator"

lib/doorkeeper/client_authentication.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+require "doorkeeper/client_authentication/credentials"
+require "doorkeeper/client_authentication/fallback_method"
+require "doorkeeper/client_authentication/method"
+require "doorkeeper/client_authentication/registry"
+
+module Doorkeeper
+  module ClientAuthentication
+    extend Registry
+
+    register(
+      :none,
+      Doorkeeper::OAuth::ClientAuthentication::None,
+    )
+
+    register(
+      :client_secret_post,
+      Doorkeeper::OAuth::ClientAuthentication::ClientSecretPost,
+    )
+
+    register(
+      :client_secret_basic,
+      Doorkeeper::OAuth::ClientAuthentication::ClientSecretBasic,
+    )
+  end
+end

lib/doorkeeper/client_authentication/credentials.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module Doorkeeper
+  module ClientAuthentication
+    Credentials = Struct.new(:uid, :secret) do
+      # Public clients may have their secret blank, but "credentials" are
+      # still present
+      delegate :blank?, to: :uid
+    end
+  end
+end

lib/doorkeeper/client_authentication/fallback_method.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module Doorkeeper
+  module ClientAuthentication
+    class FallbackMethod
+      def self.matches_request?(request)
+        true
+      end
+
+      def self.authenticate(request)
+        nil
+      end
+    end
+  end
+end

lib/doorkeeper/client_authentication/method.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module Doorkeeper
+  module ClientAuthentication
+    class Method
+      attr_reader :name, :method
+      delegate :matches_request?, to: :method
+
+      def initialize(name, method)
+        @name = name
+        @method = method
+      end
+    end
+  end
+end

lib/doorkeeper/client_authentication/registry.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+module Doorkeeper
+  module ClientAuthentication
+    module Registry
+      mattr_accessor :methods
+      self.methods = {}
+
+      # Allows to register custom OAuth client authentication method so that
+      # Doorkeeper could recognize and process it.
+      #
+      def register(name, method)
+        name_key = name.to_sym
+
+        if methods.key?(name_key)
+          ::Kernel.warn <<~WARNING
+            [DOORKEEPER] '#{name_key}' client authentication strategy is already registered and will be overridden
+            in #{caller(1..1).first}
+          WARNING
+        end
+
+        methods[name_key] = Doorkeeper::ClientAuthentication::Method.new(name, method)
+      end
+
+      # [NOTE]: make it to use #fetch after removing fallbacks
+      def get(name)
+        methods[name.to_sym]
+      end
+    end
+  end
+end

lib/doorkeeper/config.rb

@@ -70,8 +70,30 @@ def scopes_by_grant_type(hash = {})
       # `params` object.
       #
       # @param methods [Array] Define client credentials
+      # @deprecated
       def client_credentials(*methods)
-        @config.instance_variable_set(:@client_credentials_methods, methods)
+        deprecated("client_credentials", "Use the client_authentication option instead. Automatically converting to client_authentication")
+
+        client_authentication = methods.map {|method|
+          case method
+          when :from_basic
+            :client_secret_basic 
+          when :from_params
+            :client_secret_post
+          else
+            if method.respond_to? :call
+              Kernel.warn("[DOORKEEPER] Unknown client_credentials method detected, received callable block")
+            else
+              Kernel.warn("[DOORKEEPER] Unknown client_credentials method detected: #{method}")
+            end
+          end
+        }.reject(&:nil?)
+
+        if client_authentication.empty?
+          Kernel.warn("[DOORKEEPER] No known client_credentials method detected, cannot automatically convert to client_authentication option")
+        else
+          @config.instance_variable_set(:@client_credentials_methods, client_authentication.concat([:none]))
+        end
       end
 
       # Change the way access token is authenticated from the request object.
@@ -186,6 +208,13 @@ def hash_application_secrets(using: nil, fallback: nil)
 
       private
 
+      def deprecated(name, message=nil)
+        warning = "[DOORKEEPER] #{name} has been deprecated and will soon be removed"
+        warning = "#{warning}\n#{message}" if message.present?
+
+        Kernel.warn(warning)
+      end
+
       # Configure the secret storing functionality
       def configure_secrets_for(type, using:, fallback:)
         raise ArgumentError, "Invalid type #{type}" if %i[application token].exclude?(type)
@@ -253,9 +282,11 @@ def configure_secrets_for(type, using:, fallback:)
     option :orm,                            default: :active_record
     option :native_redirect_uri,            default: "urn:ietf:wg:oauth:2.0:oob", deprecated: true
     option :grant_flows,                    default: %w[authorization_code client_credentials]
+    option :client_authentication,          default: %i[client_secret_basic client_secret_post none]
     option :pkce_code_challenge_methods,    default: %w[plain S256]
     option :handle_auth_errors,             default: :render
     option :token_lookup_batch_size,        default: 10_000
+
     # Sets the token_reuse_limit
     # It will be used only when reuse_access_token option in enabled
     # By default it will be 100
@@ -579,8 +610,23 @@ def pkce_code_challenge_methods_supported
       pkce_code_challenge_methods
     end
 
-    def client_credentials_methods
-      @client_credentials_methods ||= %i[from_basic from_params]
+    def client_authentication_methods
+      return @client_authentication_methods if defined?(@client_authentication_methods)
+
+      methods = if instance_variable_defined?("@client_credentials_methods")
+        if instance_variable_defined?("@client_authentication")
+          Kernel.warn("[DOORKEEPER] Both client_credentials and client_authentication are set, using client_authentication")
+          client_authentication
+        else
+          instance_variable_get("@client_credentials_methods")
+        end
+      else
+        client_authentication
+      end
+
+      @client_authentication_methods = methods.map do |name|
+        Doorkeeper::ClientAuthentication.get(name)  
+      end.compact
     end
 
     def access_token_methods

lib/doorkeeper/config/validations.rb

@@ -8,6 +8,7 @@ module Validations
       # Validates configuration options to be set properly.
       #
       def validate!
+        validate_client_authentication_value
         validate_reuse_access_token_value
         validate_token_reuse_limit
         validate_secret_strategies
@@ -16,6 +17,16 @@ def validate!
 
       private
 
+      def validate_client_authentication_value
+        return if client_authentication.is_a?(Array)
+
+        ::Rails.logger.warn(
+          "[DOORKEEPER] You have configured client_authentication as a non-array value. Using default value"
+        )
+
+        @client_authentication = [:client_secret_basic, :client_secret_post, :none]
+      end
+
       # Determine whether +reuse_access_token+ and a non-restorable
       # +token_secret_strategy+ have both been activated.
       #

lib/doorkeeper/oauth/client.rb

@@ -17,6 +17,8 @@ def self.find(uid, method = Doorkeeper.config.application_model.method(:by_uid))
         new(application)
       end
 
+      # TODO: Figure out a way to have this just get the client but not assert
+      # authentication if not secret
       def self.authenticate(credentials, method = Doorkeeper.config.application_model.method(:by_uid_and_secret))
         return if credentials.blank?
         return unless (application = method.call(credentials.uid, credentials.secret))

lib/doorkeeper/oauth/client_authentication/client_secret_basic.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module Doorkeeper
+  module OAuth
+    module ClientAuthentication
+      class ClientSecretBasic
+        def self.matches_request?(request)
+          request.authorization.present? && request.authorization.downcase.start_with?('basic')
+        end
+
+        def self.authenticate(request)
+          value = request.authorization.to_s.split(" ", 2).second
+          client_id, client_secret = Base64.decode64(value).split(':', 2)
+
+          return unless client_id.present? && client_secret.present?
+
+          Doorkeeper::ClientAuthentication::Credentials.new(client_id, client_secret)
+        end
+      end
+    end
+  end
+end

lib/doorkeeper/oauth/client_authentication/client_secret_post.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module Doorkeeper
+  module OAuth
+    module ClientAuthentication
+      class ClientSecretPost
+        def self.matches_request?(request)
+          request.method.upcase === "POST" && request.request_parameters[:client_id].present? && request.request_parameters[:client_secret].present?
+        end
+
+        def self.authenticate(request)
+          client_id = request.request_parameters[:client_id]
+          client_secret = request.request_parameters[:client_secret]
+
+          Doorkeeper::ClientAuthentication::Credentials.new(
+            client_id,
+            client_secret
+          )
+        end
+      end
+    end
+  end
+end

lib/doorkeeper/oauth/client_authentication/none.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module Doorkeeper
+  module OAuth
+    module ClientAuthentication
+      class None
+        def self.matches_request?(request)
+          !request.authorization && request.request_parameters[:client_id] && !request.request_parameters[:client_secret]
+        end
+
+        def self.authenticate(request)
+          Doorkeeper::ClientAuthentication::Credentials.new(
+            request.request_parameters[:client_id], nil
+          )
+        end
+      end
+    end
+  end
+end

lib/doorkeeper/request.rb

@@ -3,6 +3,21 @@
 module Doorkeeper
   module Request
     class << self
+      def client_authentication_method(request)
+        # TODO: Should we support theoretically more than one method matching a
+        # request and then check each for authentication? Currently we only
+        # check the first that matches the request
+        strategy = client_authentication_methods.detect do |strategy|
+          strategy.matches_request?(request)
+        end
+
+        if strategy
+          strategy.method
+        else
+          Doorkeeper::ClientAuthentication::FallbackMethod
+        end
+      end
+
       def authorization_strategy(response_type)
         grant_flow = authorization_flows.detect do |flow|
           flow.matches_response_type?(response_type)
@@ -40,6 +55,10 @@ def token_strategy(grant_type)
 
       private
 
+      def client_authentication_methods
+        Doorkeeper.configuration.client_authentication_methods
+      end
+
       def authorization_flows
         Doorkeeper.configuration.authorization_response_flows
       end