Support confidential clients with client_secret_post

app/controllers/concerns/oauth/client_authentication.rb

@@ -0,0 +1,134 @@
+# Client credentials arrive one of two ways (RFC 6749 §2.3.1):
+#
+# - client_secret_basic: an Authorization: Basic header carrying the
+#   form-urlencoded client_id and client_secret.
+# - client_secret_post: client_id and client_secret in the request body. A
+#   secret in the query string never authenticates, since secrets there leak
+#   into proxy and access logs.
+#
+# A confidential client may use either, whichever it registered: the
+# registered token_endpoint_auth_method is advisory. Using both at once is
+# refused, since a client MUST NOT use more than one method per request (§2.3).
+module Oauth::ClientAuthentication
+  # The scheme alone, credentials or not: a bare "Basic" is a malformed attempt,
+  # not an absent one.
+  BASIC_SCHEME = /\ABasic(?: |\z)/i
+
+  private
+    # RFC 6749 §5.2 names more than one authentication method invalid_request.
+    # That answer turns on the request's shape alone, so it says nothing about
+    # which clients exist. A malformed Basic header, or one naming a different
+    # client than the body, fails like any other bad credential.
+    def reject_ambiguous_client_credentials
+      if client_secret_basic?
+        if request.request_parameters["client_secret"].present?
+          oauth_error "invalid_request", "Client credentials must use only one authentication method"
+        elsif basic_client_credentials.nil? || conflicting_client_id?
+          client_authentication_failed
+        end
+      end
+    end
+
+    # The client the request authenticates as, or nil. A confidential client
+    # authenticates with its secret. A public client identifies by client_id
+    # alone, but never alongside a secret: it holds no password, so Basic or a
+    # body client_secret naming one is a failed attempt, not identification.
+    def authenticated_client
+      if client = requesting_client
+        client if client.confidential? ? client_secret_authenticates?(client) : !client_secret_basic? && oauth_client_secret.blank?
+      end
+    end
+
+    # A request attempts client authentication when it uses Basic, names a
+    # confidential client, or carries a client secret. This turns on what the
+    # request presents, so refusing a failed attempt reveals nothing about
+    # which clients exist.
+    def attempts_client_authentication?
+      client_secret_basic? || requesting_client&.confidential? || oauth_client_secret.present?
+    end
+
+    def requesting_client
+      if oauth_client_id
+        @requesting_client ||= Oauth::Client.find_by(client_id: oauth_client_id)
+      end
+    end
+
+    def client_secret_authenticates?(client)
+      (client_secret_basic? || request.request_parameters["client_id"] == client.client_id) &&
+        client.authenticate_secret(oauth_client_secret)
+    end
+
+    # A failed Basic authentication is a 401 challenging Basic (RFC 6749 §5.2):
+    # the endpoint evaluated the Authorization header, so the challenge applies
+    # (RFC 9110 §15.5.2). Every other failure is a 400 with no challenge: a
+    # body credential, a public client presenting a secret, or a secret that
+    # authenticates no client. A bare 401 would break RFC 9110, a challenge for
+    # a scheme the client never tried applies to nothing, oauth4webapi and
+    # openid-client surface a challenged 401 as a WWWAuthenticateChallengeError
+    # rather than invalid_client, and a Basic challenge on a form POST can pop
+    # a browser's password dialog. Either way the answer turns on the request's
+    # own header alone, so an unknown client and a wrong secret look the same.
+    # RFC 7617 defines no error parameter for Basic, only realm.
+    def client_authentication_failed
+      if client_secret_basic?
+        response.headers["WWW-Authenticate"] = %(Basic realm="#{oauth_issuer}")
+        oauth_error "invalid_client", "Client authentication failed", status: :unauthorized
+      else
+        oauth_error "invalid_client", "Client authentication failed"
+      end
+    end
+
+    # Whether the request used the Basic scheme, well-formed or not.
+    def client_secret_basic?
+      request.authorization.to_s.match?(BASIC_SCHEME)
+    end
+
+    def conflicting_client_id?
+      params[:client_id].present? && params[:client_id] != basic_client_credentials.first
+    end
+
+    # A Basic header wins over the body. Only a scalar String names a client.
+    def oauth_client_id
+      if client_secret_basic?
+        basic_client_credentials&.first
+      elsif params[:client_id].is_a?(String)
+        params[:client_id]
+      end
+    end
+
+    def oauth_client_secret
+      if client_secret_basic?
+        basic_client_credentials&.last
+      else
+        request.request_parameters["client_secret"]
+      end
+    end
+
+    # [client_id, client_secret] from Authorization: Basic, or nil when it's
+    # malformed. Each half is form-urlencoded before the pair is Basic-encoded
+    # (RFC 6749 Appendix B), so the split happens on the first raw colon and
+    # each half decodes after it: %3A is a literal colon and + a space.
+    def basic_client_credentials
+      if defined?(@basic_client_credentials)
+        @basic_client_credentials
+      else
+        @basic_client_credentials = decode_basic_client_credentials
+      end
+    end
+
+    def decode_basic_client_credentials
+      decoded = Base64.strict_decode64(request.authorization.to_s.sub(BASIC_SCHEME, "").strip)
+      client_id, client_secret = decoded.split(":", 2).map { |part| URI.decode_www_form_component(part) }
+
+      if client_id.present? && client_secret.is_a?(String) && client_id.valid_encoding? && client_secret.valid_encoding?
+        [ client_id, client_secret ]
+      end
+    rescue ArgumentError
+      nil
+    end
+
+    # The pilot exemption keys on the client that authenticates, header or body.
+    def piloting_client_id
+      oauth_client_id
+    end
+end

app/controllers/oauth/base_controller.rb

@@ -1,6 +1,14 @@
 class Oauth::BaseController < ApplicationController
   disallow_account_scope
 
+  # Every authorization server response, errors, consent and sign-in
+  # redirects included, is uncacheable and sends no Referer, so a code, a
+  # token, a secret or an authorization request's parameters reach neither a
+  # shared cache nor the page a browser goes to next. Prepended, so it runs
+  # ahead of every callback that can halt, the app's authentication included:
+  # a halted chain skips after_actions (RFC 6749 §5.1, §5.2).
+  prepend_before_action :prevent_caching, :prevent_referrer
+
   private
     # Discovery follows acceptance (see Oauth::Availability): a dark server
     # answers as if it had no authorization server at all.
@@ -32,6 +40,15 @@ def oauth_issuer
       root_url(script_name: nil)
     end
 
+    def prevent_caching
+      response.headers["Cache-Control"] = "no-store"
+      response.headers["Pragma"] = "no-cache"
+    end
+
+    def prevent_referrer
+      response.headers["Referrer-Policy"] = "no-referrer"
+    end
+
     def oauth_error(error, description = nil, status: :bad_request)
       render json: { error: error, error_description: description }.compact, status: status
     end

app/controllers/oauth/clients_controller.rb

@@ -15,6 +15,7 @@ def create
       name: registered_client_name,
       redirect_uris: Array(params[:redirect_uris]),
       scopes: validated_scopes,
+      token_endpoint_auth_method: registered_auth_method,
       dynamically_registered: true
 
     render json: dynamic_client_registration_response(client), status: :created
@@ -54,11 +55,15 @@ def validate_redirect_uri_origins
     end
 
     def validate_auth_method
-      unless performed? || params[:token_endpoint_auth_method].blank? || params[:token_endpoint_auth_method] == "none"
-        oauth_error "invalid_client_metadata", "Only 'none' token_endpoint_auth_method is supported"
+      unless performed? || registered_auth_method.in?(Oauth::Client::AUTH_METHODS)
+        oauth_error "invalid_client_metadata", "token_endpoint_auth_method must be one of #{Oauth::Client::AUTH_METHODS.join(", ")}"
       end
     end
 
+    def registered_auth_method
+      params[:token_endpoint_auth_method].presence || "none"
+    end
+
     def all_registrable_uris?(uris)
       uris.is_a?(Array) &&
         uris.all? { |uri| uri.is_a?(String) && registrable_uri?(uri) }
@@ -95,12 +100,14 @@ def validated_scopes
     def dynamic_client_registration_response(client)
       {
         client_id: client.client_id,
+        client_secret: client.client_secret,
+        client_secret_expires_at: (0 if client.client_secret),
         client_name: client.name,
         redirect_uris: client.redirect_uris,
-        token_endpoint_auth_method: "none",
+        token_endpoint_auth_method: client.token_endpoint_auth_method,
         grant_types: %w[ authorization_code refresh_token ],
         response_types: %w[ code ],
         scope: client.scopes.join(" ")
-      }
+      }.compact
     end
 end

app/controllers/oauth/metadata_controller.rb

@@ -10,11 +10,11 @@ def show
       token_endpoint: oauth_token_url,
       registration_endpoint: oauth_clients_url,
       revocation_endpoint: oauth_revocation_url,
-      revocation_endpoint_auth_methods_supported: %w[ none ],
+      revocation_endpoint_auth_methods_supported: Oauth::Client::AUTH_METHODS,
       response_types_supported: %w[ code ],
       response_modes_supported: %w[ query ],
       grant_types_supported: %w[ authorization_code refresh_token ],
-      token_endpoint_auth_methods_supported: %w[ none ],
+      token_endpoint_auth_methods_supported: Oauth::Client::AUTH_METHODS,
       code_challenge_methods_supported: %w[ S256 ],
       scopes_supported: %w[ read write ],
       authorization_response_iss_parameter_supported: true

app/controllers/oauth/revocations_controller.rb

@@ -1,22 +1,54 @@
+# Token revocation (RFC 7009). The server validates the client's credentials
+# and that the token was issued to it (§2.1), so a client revokes only its own
+# tokens: a confidential client authenticates, and a public client names its
+# client_id. Anything else that fails to identify a client, and any token that
+# is unknown or belongs to another client, gets the same 200 as a successful
+# revocation (§2.2), so the endpoint is no oracle for which tokens or clients
+# exist. Only failed authentication is refused: a known confidential client
+# with a missing or wrong secret, a Basic header that doesn't authenticate,
+# or a client secret that authenticates no client.
+#
+# A personal access token was issued to no client, so possession of it is the
+# credential: whoever presents one revokes it, client or not. Client
+# credentials presented beside it must still authenticate, by the same rules.
 class Oauth::RevocationsController < Oauth::BaseController
+  include Oauth::ClientAuthentication
+
   allow_unauthenticated_access
   skip_forgery_protection
 
-  before_action :set_access_token
+  # Client authentication happens here too, so secrets can be guessed here:
+  # throttled like the token endpoint (RFC 6749 §2.3.1, RFC 7009 §5).
+  rate_limit to: 20, within: 1.minute, only: :create, with: :oauth_rate_limit_exceeded
+
+  before_action :reject_ambiguous_client_credentials
+  before_action :require_token
 
   def create
-    @access_token&.destroy
+    if attempts_client_authentication? && !authenticated_client
+      client_authentication_failed
+    else
+      revocable_token&.destroy
 
-    head :ok  # Don't behave as oracle, per RFC 7009
+      head :ok
+    end
   end
 
   private
-    def set_access_token
-      if (token = params[:token]).present?
-        @access_token = Identity::AccessToken.find_by(token: token) ||
-          Identity::AccessToken.find_by(refresh_token: token)
-      else
+    def require_token
+      unless params[:token].is_a?(String) && params[:token].present?
         oauth_error "invalid_request", "token is required"
       end
     end
+
+    def revocable_token
+      Identity::AccessToken.personal.find_by(token: params[:token]) || client_token
+    end
+
+    def client_token
+      if client = authenticated_client
+        tokens = Identity::AccessToken.oauth.where(oauth_client: client)
+        tokens.find_by(token: params[:token]) || tokens.find_by(refresh_token: params[:token])
+      end
+    end
 end

app/controllers/oauth/tokens_controller.rb

@@ -1,4 +1,6 @@
 class Oauth::TokensController < Oauth::BaseController
+  include Oauth::ClientAuthentication
+
   allow_unauthenticated_access
   skip_forgery_protection
 
@@ -7,18 +9,29 @@ class Oauth::TokensController < Oauth::BaseController
   before_action :require_issuance_enabled
   rate_limit to: 20, within: 1.minute, only: :create, with: :oauth_rate_limit_exceeded
   before_action :validate_grant_type
+
+  # Authenticate the client the request names before looking anything up about
+  # the grant: a confidential client that fails auth must see invalid_client,
+  # not an invalid_grant it could misread as a revoked grant and discard, and
+  # the answer must not depend on whether the code or refresh token is live,
+  # or it tells whoever holds one without the secret.
+  before_action :authenticate_client
   before_action :require_params
 
   with_options if: :authorization_code_grant? do
     before_action :set_auth_code
     before_action :set_client
+  end
+
+  before_action :set_refreshable_access_token, unless: :authorization_code_grant?
+  before_action :validate_client_id
+
+  with_options if: :authorization_code_grant? do
     before_action :validate_pkce
     before_action :validate_redirect_uri
     before_action :set_identity
   end
 
-  before_action :set_refreshable_access_token, unless: :authorization_code_grant?
-  before_action :validate_client_id
   before_action :set_refresh_scope, unless: :authorization_code_grant?
 
   def create
@@ -55,7 +68,8 @@ def validate_grant_type
 
     # A missing parameter is a malformed request (invalid_request), not a dead
     # grant (invalid_grant), which a client would act on by discarding it.
-    # client_id is required of every client: none authenticates by header.
+    # client_id is required of every client, in the body unless a Basic
+    # header carries it.
     # Each is a single string; an array or object from a JSON body or a
     # name[] form field is just as malformed, and must not reach a lookup.
     def require_params
@@ -65,7 +79,7 @@ def require_params
     end
 
     def required_params
-      authorization_code_grant? ? %w[ code code_verifier redirect_uri client_id ] : %w[ refresh_token client_id ]
+      (authorization_code_grant? ? %w[ code code_verifier redirect_uri ] : %w[ refresh_token ]) + (client_secret_basic? ? [] : %w[ client_id ])
     end
 
     def string_param?(name)
@@ -110,9 +124,11 @@ def set_refreshable_access_token
 
     # The code or refresh token must have been issued to the client_id in the
     # request (RFC 6749 §4.1.3, §6).
+    # One issued to another client gets the same answer as a dead one, so
+    # naming a public client tells nobody whether someone else's grant is live.
     def validate_client_id
-      unless params[:client_id] == (@client || @access_token.oauth_client).client_id
-        oauth_error "invalid_grant", "Grant was not issued to this client"
+      unless oauth_client_id == (@client || @access_token.oauth_client).client_id
+        oauth_error "invalid_grant", authorization_code_grant? ? "Invalid or expired authorization code" : "Invalid refresh token"
       end
     end
 
@@ -137,6 +153,22 @@ def granted_scopes(permission)
       Oauth.canonical_scope(permission).split
     end
 
+    # A request attempts client authentication when it uses Basic, names a
+    # confidential client, or carries a client secret, and then it must
+    # authenticate a confidential client whatever the grant (see
+    # Oauth::ClientAuthentication). A request naming a public client attempts
+    # none; if its grant belongs to a confidential client, validate_client_id
+    # refuses it as issued to another client, the same answer as for a dead
+    # grant. So a confidential grant is only ever redeemed by the client it
+    # names, authenticated.
+    def authenticate_client
+      reject_ambiguous_client_credentials
+
+      unless performed? || !attempts_client_authentication? || authenticated_client&.confidential?
+        client_authentication_failed
+      end
+    end
+
     def token_response(access_token, scope: nil)
       {
         access_token: access_token.token,