diff --git a/Gemfile b/Gemfile index 85cc5ab8a..b41b2c1b8 100644 --- a/Gemfile +++ b/Gemfile @@ -21,6 +21,7 @@ gem "rubocop-rails", require: false gem "rubocop-rspec", require: false gem "bcrypt", "~> 3.1", require: false +gem "jwt", require: false gem "activerecord-jdbcsqlite3-adapter", platform: :jruby gem "sqlite3", "~> 2.3", platform: [:ruby, :mswin, :mingw, :x64_mingw] diff --git a/config/locales/en.yml b/config/locales/en.yml index c9190526f..caf6643a5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -125,6 +125,7 @@ en: invalid_token: revoked: "The access token was revoked" expired: "The access token expired" + invalid_dpop_key_binding: "Invalid DPoP key binding" unknown: "The access token is invalid" revoke: unauthorized: "You are not authorized to revoke this token" @@ -132,6 +133,8 @@ en: forbidden_token: missing_scope: 'Access to this resource requires scope "%{oauth_scopes}".' + invalid_dpop_proof: "Invalid DPoP proof" + flash: applications: create: diff --git a/gemfiles/rails_7_0.gemfile b/gemfiles/rails_7_0.gemfile index 4bdb9e93b..fc4554bf5 100644 --- a/gemfiles/rails_7_0.gemfile +++ b/gemfiles/rails_7_0.gemfile @@ -22,5 +22,6 @@ gem "base64" gem "drb" gem "mutex_m" gem "concurrent-ruby", "1.3.4" +gem "jwt", require: false gemspec path: "../" diff --git a/gemfiles/rails_7_1.gemfile b/gemfiles/rails_7_1.gemfile index 250149072..f4ac1c1c2 100644 --- a/gemfiles/rails_7_1.gemfile +++ b/gemfiles/rails_7_1.gemfile @@ -18,5 +18,6 @@ gem "sprockets-rails" gem "sqlite3", "~> 1.4", platform: [:ruby, :mswin, :mingw, :x64_mingw] gem "tzinfo-data", platforms: [:mingw, :mswin, :x64_mingw] gem "timecop" +gem "jwt", require: false gemspec path: "../" diff --git a/gemfiles/rails_7_2.gemfile b/gemfiles/rails_7_2.gemfile index 670dbbfac..9775a339c 100644 --- a/gemfiles/rails_7_2.gemfile +++ b/gemfiles/rails_7_2.gemfile @@ -18,5 +18,6 @@ gem "sprockets-rails" gem "sqlite3", "~> 1.4", platform: [:ruby, :mswin, :mingw, :x64_mingw] gem "tzinfo-data", platforms: [:mingw, :mswin, :x64_mingw] gem "timecop" +gem "jwt", require: false gemspec path: "../" diff --git a/gemfiles/rails_8_0.gemfile b/gemfiles/rails_8_0.gemfile index 5725e4fed..668744986 100644 --- a/gemfiles/rails_8_0.gemfile +++ b/gemfiles/rails_8_0.gemfile @@ -18,5 +18,6 @@ gem "sprockets-rails" gem "sqlite3", "~> 2.3", platform: [:ruby, :mswin, :mingw, :x64_mingw] gem "tzinfo-data", platforms: [:mingw, :mswin, :x64_mingw] gem "timecop" +gem "jwt", require: false gemspec path: "../" diff --git a/gemfiles/rails_edge.gemfile b/gemfiles/rails_edge.gemfile index f9567df42..7b1035f59 100644 --- a/gemfiles/rails_edge.gemfile +++ b/gemfiles/rails_edge.gemfile @@ -18,5 +18,6 @@ gem "sprockets-rails" gem "sqlite3", "~> 2.3", platform: [:ruby, :mswin, :mingw, :x64_mingw] gem "tzinfo-data", platforms: [:mingw, :mswin, :x64_mingw] gem "timecop" +gem "jwt", require: false gemspec path: "../" diff --git a/lib/doorkeeper.rb b/lib/doorkeeper.rb index 97e3f549a..c320b16a7 100644 --- a/lib/doorkeeper.rb +++ b/lib/doorkeeper.rb @@ -47,8 +47,10 @@ module OAuth autoload :Client, "doorkeeper/oauth/client" autoload :ClientCredentialsRequest, "doorkeeper/oauth/client_credentials_request" autoload :CodeRequest, "doorkeeper/oauth/code_request" + autoload :DPoPProof, "doorkeeper/oauth/dpop_proof" autoload :ErrorResponse, "doorkeeper/oauth/error_response" autoload :Error, "doorkeeper/oauth/error" + autoload :InvalidDPoPProofResponse, "doorkeeper/oauth/invalid_dpop_proof_response" autoload :InvalidTokenResponse, "doorkeeper/oauth/invalid_token_response" autoload :InvalidRequestResponse, "doorkeeper/oauth/invalid_request_response" autoload :ForbiddenTokenResponse, "doorkeeper/oauth/forbidden_token_response" diff --git a/lib/doorkeeper/config.rb b/lib/doorkeeper/config.rb index c464f1803..26dacf4af 100644 --- a/lib/doorkeeper/config.rb +++ b/lib/doorkeeper/config.rb @@ -99,7 +99,7 @@ def reuse_access_token @config.instance_variable_set(:@reuse_access_token, true) end - # Choose to use the url path for native autorization codes + # Choose to use the url path for native autorization codes # Enabling this flag sets the authorization code response route for # native redirect uris to oauth/authorize/. The default is # oauth/authorize/native?code=. @@ -129,6 +129,11 @@ def force_pkce @config.instance_variable_set(:@force_pkce, true) end + # Require all access token token requests to include a DPoP proof (disabled by default) + def force_dpop + @config.instance_variable_set(:@force_dpop, true) + end + # Use an API mode for applications generated with --api argument # It will skip applications controller, disable forgery protection def api_only @@ -256,6 +261,8 @@ def configure_secrets_for(type, using:, fallback:) option :pkce_code_challenge_methods, default: %w[plain S256] option :handle_auth_errors, default: :render option :token_lookup_batch_size, default: 10_000 + option :dpop_iat_leeway, default: 300 + option :dpop_signature_algorithms, default: %w[ES256 PS256] # Sets the token_reuse_limit # It will be used only when reuse_access_token option in enabled # By default it will be 100 @@ -513,6 +520,10 @@ def force_pkce? option_set? :force_pkce end + def force_dpop? + option_set? :force_dpop + end + def enforce_configured_scopes? option_set? :enforce_configured_scopes end @@ -575,7 +586,7 @@ def scopes_by_grant_type def pkce_code_challenge_methods_supported return [] unless access_grant_model.pkce_supported? - + pkce_code_challenge_methods end @@ -588,7 +599,7 @@ def access_token_methods from_bearer_authorization from_access_token_param from_bearer_param - ] + ].tap { |it| it.prepend(:from_dpop_authorization) if access_token_model.dpop_supported? } end def enabled_grant_flows @@ -616,7 +627,7 @@ def token_grant_types def deprecated_token_grant_types_resolver @deprecated_token_grant_types ||= calculate_token_grant_types end - + def native_authorization_code_route @use_url_path_for_native_authorization = false unless defined?(@use_url_path_for_native_authorization) @use_url_path_for_native_authorization ? '/:code' : '/native' diff --git a/lib/doorkeeper/errors.rb b/lib/doorkeeper/errors.rb index 15f39ef95..d2b142071 100644 --- a/lib/doorkeeper/errors.rb +++ b/lib/doorkeeper/errors.rb @@ -59,6 +59,12 @@ def self.translate_options end end + class InvalidDPoPProof < BaseResponseError + def self.name_for_response + :invalid_dpop_proof + end + end + UnableToGenerateToken = Class.new(DoorkeeperError) TokenGeneratorNotFound = Class.new(DoorkeeperError) NoOrmCleaner = Class.new(DoorkeeperError) @@ -81,5 +87,6 @@ def self.translate_options TokenRevoked = Class.new(InvalidToken) TokenUnknown = Class.new(InvalidToken) TokenForbidden = Class.new(InvalidToken) + TokenInvalidDPoPKeyBinding = Class.new(InvalidToken) end end diff --git a/lib/doorkeeper/grape/helpers.rb b/lib/doorkeeper/grape/helpers.rb index 4671bcc7d..8e307e095 100644 --- a/lib/doorkeeper/grape/helpers.rb +++ b/lib/doorkeeper/grape/helpers.rb @@ -12,7 +12,7 @@ module Helpers include Doorkeeper::Rails::Helpers # endpoint specific scopes > parameter scopes > default scopes - def doorkeeper_authorize!(*scopes) + def doorkeeper_authorize!(*scopes, dpop: nil) endpoint_scopes = endpoint.route_setting(:scopes) || endpoint.options[:route_options][:scopes] @@ -22,7 +22,10 @@ def doorkeeper_authorize!(*scopes) Doorkeeper::OAuth::Scopes.from_array(scopes) end - super(*scopes) + endpoint_dpop = endpoint.route_setting(:dpop) || + endpoint.options[:route_options][:dpop] + + super(*scopes, dpop: endpoint_dpop || dpop) end def doorkeeper_render_error_with(error) @@ -36,14 +39,7 @@ def endpoint env["api.endpoint"] end - def doorkeeper_token - @doorkeeper_token ||= OAuth::Token.authenticate( - decorated_request, - *Doorkeeper.config.access_token_methods, - ) - end - - def decorated_request + def __doorkeeper_request__ AuthorizationDecorator.new(request) end diff --git a/lib/doorkeeper/models/access_token_mixin.rb b/lib/doorkeeper/models/access_token_mixin.rb index 2c1ce4ace..5e26771f1 100644 --- a/lib/doorkeeper/models/access_token_mixin.rb +++ b/lib/doorkeeper/models/access_token_mixin.rb @@ -326,14 +326,24 @@ def extract_custom_attributes(attributes) attributes.with_indifferent_access.slice( *Doorkeeper.configuration.custom_access_token_attributes) end + + # Checks whether the token can be sender-constrained using DPoP. + # + # @see https://datatracker.ietf.org/doc/html/rfc9449 + # OAuth 2.0 Demonstrating Proof of Possession (DPoP) + def dpop_supported? + column_names.include?("dpop_jkt") + end end - # Access Token type: Bearer. + # Access Token type: Bearer or DPoP + # # @see https://datatracker.ietf.org/doc/html/rfc6750 # The OAuth 2.0 Authorization Framework: Bearer Token Usage - # + # @see https://datatracker.ietf.org/doc/html/rfc9449 + # OAuth 2.0 Demonstrating Proof of Possession (DPoP) def token_type - "Bearer" + uses_dpop? ? "DPoP" : "Bearer" end def use_refresh_token? @@ -438,6 +448,29 @@ def revoke_previous_refresh_token! update_attribute(:previous_refresh_token, "") end + # Checks whether the token has been sender-constrained using DPoP. The token + # is never considered sender-constrained if the DPoP migration was not run. + # + # @see https://datatracker.ietf.org/doc/html/rfc9449 + # OAuth 2.0 Demonstrating Proof of Possession (DPoP) + def uses_dpop? + self.class.dpop_supported? && dpop_jkt.present? + end + + # Checks whether the token is bound to the given DPoP key thumbprint (`jkt`). + # + # DPoP-bound (sender-constrained) access tokens are bound to a public key by its JWK + # SHA-256 thumbprint (`dpop_jkt`). This method returns `false` if the token is not DPoP-bound, + # otherwise it returns whether the token's stored thumbprint matches the provided `jkt`. + # + # @param jkt [String] The JWK SHA-256 thumbprint of the DPoP public key jkt + # @return [Boolean] True if the token is DPoP-bound and bound to the given `jkt` + def dpop_binding_matches?(jkt) + return false unless uses_dpop? + + ActiveSupport::SecurityUtils.secure_compare(dpop_jkt, jkt) + end + private # Searches for Access Token record with `:refresh_token` equal to diff --git a/lib/doorkeeper/oauth/authorization_code_request.rb b/lib/doorkeeper/oauth/authorization_code_request.rb index cb9905eff..f3520b1e3 100644 --- a/lib/doorkeeper/oauth/authorization_code_request.rb +++ b/lib/doorkeeper/oauth/authorization_code_request.rb @@ -13,7 +13,9 @@ class AuthorizationCodeRequest < BaseRequest attr_reader :grant, :client, :redirect_uri, :access_token, :code_verifier, :invalid_request_reason, :missing_param - def initialize(server, grant, client, parameters = {}) + def initialize(server, grant, client, **base_options) + super(**base_options) + @server = server @client = client @grant = grant diff --git a/lib/doorkeeper/oauth/base_request.rb b/lib/doorkeeper/oauth/base_request.rb index 81a804201..efa10c086 100644 --- a/lib/doorkeeper/oauth/base_request.rb +++ b/lib/doorkeeper/oauth/base_request.rb @@ -5,10 +5,20 @@ module OAuth class BaseRequest include Validations - attr_reader :grant_type, :server + attr_reader :grant_type, :parameters, :server delegate :default_scopes, to: :server + def self.inherited(subclass) + super + subclass.validate :dpop_proof, error: Errors::InvalidDPoPProof + end + + def initialize(dpop_proof: nil, parameters: {}) + @dpop_proof = dpop_proof + @parameters = parameters + end + def authorize if valid? before_successful_response @@ -36,6 +46,7 @@ def find_or_create_access_token(client, resource_owner, scopes, custom_attribute scopes: scopes, expires_in: Authorization::Token.access_token_expires_in(server, context), use_refresh_token: Authorization::Token.refresh_token_enabled?(server, context), + **dpop_token_attributes, } @access_token = @@ -52,6 +63,8 @@ def after_successful_response private + attr_reader :dpop_proof + def build_scopes if @original_scopes.present? OAuth::Scopes.from_string(@original_scopes) @@ -63,6 +76,23 @@ def build_scopes client_scopes.allowed(default_scopes) end end + + def dpop_supported? + Doorkeeper.config.access_token_model.dpop_supported? + end + + def dpop_token_attributes + return {} unless dpop_supported? + + { dpop_jkt: dpop_proof&.jkt }.compact + end + + def validate_dpop_proof + return true unless dpop_supported? + return true unless Doorkeeper.config.force_dpop? || dpop_proof.present? + + dpop_proof.valid? + end end end end diff --git a/lib/doorkeeper/oauth/client_credentials_request.rb b/lib/doorkeeper/oauth/client_credentials_request.rb index 4b0f1d794..21269271f 100644 --- a/lib/doorkeeper/oauth/client_credentials_request.rb +++ b/lib/doorkeeper/oauth/client_credentials_request.rb @@ -3,18 +3,17 @@ module Doorkeeper module OAuth class ClientCredentialsRequest < BaseRequest - attr_reader :client, :original_scopes, :parameters, :response + attr_reader :client, :original_scopes, :response alias error_response response - delegate :error, to: :issuer + def initialize(server, client, parameters: {}, **base_options) + super(parameters: parameters.except(:scope), **base_options) - def initialize(server, client, parameters = {}) @client = client @server = server @response = nil @original_scopes = parameters[:scope] - @parameters = parameters.except(:scope) end def access_token @@ -28,12 +27,16 @@ def issuer ) end - private + def error + @error || issuer.error + end - def valid? - issuer.create(client, scopes, custom_token_attributes_with_data) + def validate + super && issuer.create(client, scopes, dpop_token_attributes.merge(custom_token_attributes_with_data)) end + private + def custom_token_attributes_with_data parameters .with_indifferent_access diff --git a/lib/doorkeeper/oauth/dpop_proof.rb b/lib/doorkeeper/oauth/dpop_proof.rb new file mode 100644 index 000000000..94f3096f7 --- /dev/null +++ b/lib/doorkeeper/oauth/dpop_proof.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +module Doorkeeper + module OAuth + class DPoPProof + begin + require "jwt" + rescue LoadError + raise %(The `jwt` gem is required for DPoP support. Add `gem "jwt"` to your Gemfile.) + end + + include Validations + + validate :presence, error: :blank + validate :single_proof, error: :multiple_dpop_proofs + validate :type, error: :invalid_type + validate :signing_algorithm, error: :invalid_signing_algorithm + validate :jwk, error: :invalid_jwk + validate :jti, error: :invalid_jti + validate :iat, error: :invalid_iat + validate :ath, error: :invalid_ath + validate :htm, error: :invalid_htm + validate :htu, error: :invalid_htu + validate :signature, error: :invalid_signature + + delegate :blank?, to: :dpop + + def initialize(request, access_token = nil) + @request = request + @access_token = access_token + + @dpop = request.headers["DPoP"] + end + + def validate + super.tap { @valid = @error.nil? } + end + + def valid? + return @valid if defined?(@valid) + + super + end + + def jkt + return nil unless valid? + + @jkt ||= JWT::JWK::Thumbprint.new(jwk).generate + end + + private + + attr_reader :access_token, :dpop, :request + + def claims + return @claims if defined?(@claims) + + decode_without_verifying_signature + @claims + end + + def headers + return @headers if defined?(@headers) + + decode_without_verifying_signature + @headers + end + + def decode_without_verifying_signature + @claims, @headers = JWT.decode(dpop, nil, false) + rescue JWT::DecodeError + @claims = {} + @headers = {} + end + + def jwk + @jwk ||= headers["jwk"] && JWT::JWK.import(headers["jwk"]) + end + + def validate_presence + present? + end + + def validate_single_proof + dpop.split(",").size == 1 && dpop.split(";").size == 1 + end + + def validate_type + headers["typ"] == "dpop+jwt" + end + + def validate_signing_algorithm + Doorkeeper.config.dpop_signature_algorithms.include?(headers["alg"]) + end + + def validate_jwk + jwk && !jwk.private? + end + + def validate_jti + claims["jti"].present? + end + + def validate_iat + claims["iat"].present? && + (claims["iat"] - Time.now.to_i).abs <= Doorkeeper.config.dpop_iat_leeway + end + + def validate_ath + return true unless access_token + + claims["ath"] == Base64.urlsafe_encode64(Digest::SHA256.digest(access_token), padding: false) + end + + def validate_htm + claims["htm"] == request.request_method + end + + def validate_htu + claims["htu"] == (request.base_url + request.path) + end + + def validate_signature + JWT.decode(dpop, jwk.keypair, true, algorithms: [headers["alg"]]) + rescue JWT::DecodeError, JWT::JWKError + false + end + end + end +end diff --git a/lib/doorkeeper/oauth/error_response.rb b/lib/doorkeeper/oauth/error_response.rb index f266271d7..acb76f802 100644 --- a/lib/doorkeeper/oauth/error_response.rb +++ b/lib/doorkeeper/oauth/error_response.rb @@ -34,6 +34,8 @@ def self.exception_class_for(error) delegate :name, :description, :state, to: :@error def initialize(attributes = {}) + @access_token_method = attributes[:access_token_method] + @dpop = attributes[:dpop] @error = OAuth::Error.new(*attributes.values_at(:name, :state, :translate_options)) @exception_class = attributes[:exception_class] @redirect_uri = attributes[:redirect_uri] @@ -93,8 +95,37 @@ def exception_class private + attr_reader :access_token_method, :dpop + def authenticate_info - %(Bearer realm="#{realm}", error="#{name}", error_description="#{description}") + if dpop_required? + %(DPoP realm="#{realm}", error="#{name}", error_description="#{description}", algs="#{dpop_algs}") + elsif !dpop_supported? + %(Bearer realm="#{realm}", error="#{name}", error_description="#{description}") + elsif dpop_used_to_authenticate? + %(Bearer, DPoP realm="#{realm}", error="#{name}", error_description="#{description}", algs="#{dpop_algs}") + elsif access_token_method + %(Bearer realm="#{realm}", error="#{name}", error_description="#{description}", DPoP algs="#{dpop_algs}") + else + %(Bearer realm="#{realm}", error="#{name}", error_description="#{description}", ) + + %(DPoP realm="#{realm}", error="#{name}", error_description="#{description}", algs="#{dpop_algs}") + end + end + + def dpop_algs + Doorkeeper.config.dpop_signature_algorithms.join(" ") + end + + def dpop_required? + dpop == :required + end + + def dpop_supported? + Doorkeeper.config.access_token_model.dpop_supported? + end + + def dpop_used_to_authenticate? + access_token_method == :from_dpop_authorization end end end diff --git a/lib/doorkeeper/oauth/invalid_dpop_proof_response.rb b/lib/doorkeeper/oauth/invalid_dpop_proof_response.rb new file mode 100644 index 000000000..5be6a6326 --- /dev/null +++ b/lib/doorkeeper/oauth/invalid_dpop_proof_response.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Doorkeeper + module OAuth + class InvalidDPoPProofResponse < ErrorResponse + def initialize(attributes = {}) + super(attributes.merge(name: :invalid_dpop_proof, state: :unauthorized)) + end + + def status + :unauthorized + end + + def description + @description ||= I18n.translate(:invalid_dpop_proof, scope: %i[doorkeeper errors messages]) + end + + protected + + def exception_class + Doorkeeper::Errors::InvalidDPoPProof + end + end + end +end diff --git a/lib/doorkeeper/oauth/invalid_token_response.rb b/lib/doorkeeper/oauth/invalid_token_response.rb index f9fa10930..7317d104d 100644 --- a/lib/doorkeeper/oauth/invalid_token_response.rb +++ b/lib/doorkeeper/oauth/invalid_token_response.rb @@ -10,6 +10,8 @@ def self.from_access_token(access_token, attributes = {}) :revoked elsif access_token&.expired? :expired + elsif access_token&.uses_dpop? && attributes[:access_token_method] == :from_dpop_authorization + :invalid_dpop_key_binding else :unknown end @@ -46,6 +48,7 @@ def errors_mapping { expired: Doorkeeper::Errors::TokenExpired, revoked: Doorkeeper::Errors::TokenRevoked, + invalid_dpop_key_binding: Doorkeeper::Errors::TokenInvalidDPoPKeyBinding, unknown: Doorkeeper::Errors::TokenUnknown, } end diff --git a/lib/doorkeeper/oauth/password_access_token_request.rb b/lib/doorkeeper/oauth/password_access_token_request.rb index b6a59b007..0b2ec0c60 100644 --- a/lib/doorkeeper/oauth/password_access_token_request.rb +++ b/lib/doorkeeper/oauth/password_access_token_request.rb @@ -10,14 +10,15 @@ class PasswordAccessTokenRequest < BaseRequest validate :resource_owner, error: Errors::InvalidGrant validate :scopes, error: Errors::InvalidScope - attr_reader :client, :credentials, :resource_owner, :parameters, :access_token + attr_reader :client, :credentials, :resource_owner, :access_token + + def initialize(server, client, credentials, resource_owner, **base_options) + super(**base_options) - def initialize(server, client, credentials, resource_owner, parameters = {}) @server = server @resource_owner = resource_owner @client = client @credentials = credentials - @parameters = parameters @original_scopes = parameters[:scope] @grant_type = Doorkeeper::OAuth::PASSWORD end diff --git a/lib/doorkeeper/oauth/refresh_token_request.rb b/lib/doorkeeper/oauth/refresh_token_request.rb index bf5b9a680..489943c10 100644 --- a/lib/doorkeeper/oauth/refresh_token_request.rb +++ b/lib/doorkeeper/oauth/refresh_token_request.rb @@ -14,7 +14,9 @@ class RefreshTokenRequest < BaseRequest attr_reader :access_token, :client, :credentials, :refresh_token attr_reader :missing_param - def initialize(server, refresh_token, credentials, parameters = {}) + def initialize(server, refresh_token, credentials, **base_options) + super(**base_options) + @server = server @refresh_token = refresh_token @credentials = credentials @@ -58,7 +60,7 @@ def default_scopes end def create_access_token - attributes = {}.merge(custom_token_attributes_with_data) + attributes = dpop_token_attributes.merge(custom_token_attributes_with_data) resource_owner = if Doorkeeper.config.polymorphic_resource_owner? @@ -136,6 +138,26 @@ def custom_token_attributes_with_data .slice(*Doorkeeper.config.custom_access_token_attributes) .symbolize_keys end + + def dpop_token_attributes + if client&.confidential && refresh_token.uses_dpop? + { dpop_jkt: refresh_token.dpop_jkt }.merge(super) + else + super + end + end + + def validate_dpop_proof + if refresh_token&.uses_dpop? + if client&.confidential + dpop_proof.present? ? dpop_proof.valid? : true + else + dpop_proof.valid? && refresh_token.dpop_binding_matches?(dpop_proof.jkt) + end + else + super + end + end end end end diff --git a/lib/doorkeeper/oauth/token.rb b/lib/doorkeeper/oauth/token.rb index a56c7b3b5..fb0e8ca59 100644 --- a/lib/doorkeeper/oauth/token.rb +++ b/lib/doorkeeper/oauth/token.rb @@ -12,14 +12,21 @@ def from_request(request, *methods) end end - def authenticate(request, *methods) - if (token = from_request(request, *methods)) + def authenticate3(request, *methods) + method, token = methods.lazy.map { |m| [m, from_request(request, m)] }.detect(&:last) + + if token access_token = Doorkeeper.config.access_token_model.by_token(token) if access_token.present? && Doorkeeper.config.refresh_token_enabled? access_token.revoke_previous_refresh_token! end - access_token end + + [method, token, access_token] + end + + def authenticate(request, *methods) + authenticate3(request, *methods)[2] end def from_access_token_param(request) @@ -36,6 +43,12 @@ def from_bearer_authorization(request) token_from_header(header, pattern) if match?(header, pattern) end + def from_dpop_authorization(request) + pattern = /^DPoP /i + header = request.authorization + token_from_header(header, pattern) if match?(header, pattern) + end + def from_basic_authorization(request) pattern = /^Basic /i header = request.authorization diff --git a/lib/doorkeeper/oauth/token_introspection.rb b/lib/doorkeeper/oauth/token_introspection.rb index 4a88118fc..613ec8d45 100644 --- a/lib/doorkeeper/oauth/token_introspection.rb +++ b/lib/doorkeeper/oauth/token_introspection.rb @@ -109,6 +109,8 @@ def success_response token_type: @token.token_type, exp: @token.expires_at.to_i, iat: @token.created_at.to_i, + # Section 6.2 of OAuth 2.0 Demonstrating Proof of Possession (DPoP) [RFC9449] + **(@token.uses_dpop? ? { cnf: { jkt: @token.dpop_jkt } } : {}), ) end diff --git a/lib/doorkeeper/rails/helpers.rb b/lib/doorkeeper/rails/helpers.rb index 92c5f0867..a64e57df8 100644 --- a/lib/doorkeeper/rails/helpers.rb +++ b/lib/doorkeeper/rails/helpers.rb @@ -3,8 +3,17 @@ module Doorkeeper module Rails module Helpers - def doorkeeper_authorize!(*scopes) + def doorkeeper_authorize!(*scopes, dpop: nil) + unless dpop.nil? || dpop == :required + raise ArgumentError, "dpop must be `:required` or `nil`, got: `#{dpop.inspect}`" + end + @_doorkeeper_scopes = scopes.presence || Doorkeeper.config.default_scopes + @_doorkeeper_dpop = if Doorkeeper.config.access_token_methods == %i[from_dpop_authorization] + :required + else + dpop + end doorkeeper_render_error unless valid_doorkeeper_token? end @@ -14,7 +23,24 @@ def doorkeeper_unauthorized_render_options(**); end def doorkeeper_forbidden_render_options(**); end def valid_doorkeeper_token? - doorkeeper_token&.acceptable?(@_doorkeeper_scopes) + doorkeeper_token&.acceptable?(@_doorkeeper_scopes) && + satisified_doorkeeper_token_dpop_binding? + end + + def satisified_doorkeeper_token_dpop_binding? + if @_doorkeeper_dpop == :required || @_doorkeeper_access_token_method == :from_dpop_authorization + doorkeeper_dpop_proof.valid? && doorkeeper_token.dpop_binding_matches?(doorkeeper_dpop_proof.jkt) + elsif Doorkeeper.config.access_token_model.dpop_supported? + if doorkeeper_token.uses_dpop? + return false unless @_doorkeeper_access_token_method == :from_dpop_authorization # downgrade attempt + + doorkeeper_dpop_proof.valid? && doorkeeper_token.dpop_binding_matches?(doorkeeper_dpop_proof.jkt) + else + true + end + else + true + end end private @@ -42,15 +68,19 @@ def doorkeeper_render_error_with(error) end def doorkeeper_error - if doorkeeper_invalid_token_response? - OAuth::InvalidTokenResponse.from_access_token(doorkeeper_token) + error_attributes = { access_token_method: @_doorkeeper_access_token_method, dpop: @_doorkeeper_dpop } + + if doorkeeper_invalid_dpop_proof_response? + OAuth::InvalidDPoPProofResponse.new(error_attributes) + elsif doorkeeper_invalid_token_response? + OAuth::InvalidTokenResponse.from_access_token(doorkeeper_token, error_attributes) else - OAuth::ForbiddenTokenResponse.from_scopes(@_doorkeeper_scopes) + OAuth::ForbiddenTokenResponse.from_scopes(@_doorkeeper_scopes, error_attributes) end end def doorkeeper_render_options(error) - if doorkeeper_invalid_token_response? + if doorkeeper_invalid_token_response? || doorkeeper_invalid_dpop_proof_response? doorkeeper_unauthorized_render_options(error: error) else doorkeeper_forbidden_render_options(error: error) @@ -66,16 +96,44 @@ def doorkeeper_status_for_error(error, respond_not_found_when_forbidden) end def doorkeeper_invalid_token_response? - !doorkeeper_token || !doorkeeper_token.accessible? + !doorkeeper_token || !doorkeeper_token.accessible? || !satisified_doorkeeper_token_dpop_binding? + end + + def doorkeeper_invalid_dpop_proof_response? + @_doorkeeper_access_token_method == :from_dpop_authorization && !doorkeeper_dpop_proof.valid? + end + + def doorkeeper_dpop_proof + @doorkeeper_dpop_proof ||= begin + doorkeeper_authenticate3 + OAuth::DPoPProof.new(__doorkeeper_request__, @_doorkeeper_plaintext_token) + end end def doorkeeper_token return @doorkeeper_token if defined?(@doorkeeper_token) - @doorkeeper_token = OAuth::Token.authenticate( - request, - *Doorkeeper.config.access_token_methods, - ) + doorkeeper_authenticate3 + @doorkeeper_token + end + + def doorkeeper_authenticate3 + return @_doorkeeper_authenticate3 if defined?(@_doorkeeper_authenticate3) + + @_doorkeeper_authenticate3 = begin + access_token_methods = if @_doorkeeper_dpop == :required + %i[from_dpop_authorization] + else + Doorkeeper.config.access_token_methods + end + OAuth::Token.authenticate3(__doorkeeper_request__, *access_token_methods) + end + + @_doorkeeper_access_token_method, @_doorkeeper_plaintext_token, @doorkeeper_token = @_doorkeeper_authenticate3 + end + + def __doorkeeper_request__ + request end end end diff --git a/lib/doorkeeper/request/authorization_code.rb b/lib/doorkeeper/request/authorization_code.rb index 6f69a5650..c737607c8 100644 --- a/lib/doorkeeper/request/authorization_code.rb +++ b/lib/doorkeeper/request/authorization_code.rb @@ -3,14 +3,15 @@ module Doorkeeper module Request class AuthorizationCode < Strategy - delegate :client, :parameters, to: :server + delegate :client, :dpop_proof, :parameters, to: :server def request @request ||= OAuth::AuthorizationCodeRequest.new( Doorkeeper.config, grant, client, - parameters, + dpop_proof:, + parameters:, ) end diff --git a/lib/doorkeeper/request/client_credentials.rb b/lib/doorkeeper/request/client_credentials.rb index a86736d91..526105ee8 100644 --- a/lib/doorkeeper/request/client_credentials.rb +++ b/lib/doorkeeper/request/client_credentials.rb @@ -3,13 +3,14 @@ module Doorkeeper module Request class ClientCredentials < Strategy - delegate :client, :parameters, to: :server + delegate :client, :dpop_proof, :parameters, to: :server def request @request ||= OAuth::ClientCredentialsRequest.new( Doorkeeper.config, client, - parameters, + dpop_proof:, + parameters:, ) end end diff --git a/lib/doorkeeper/request/password.rb b/lib/doorkeeper/request/password.rb index 53699e442..a520ea08d 100644 --- a/lib/doorkeeper/request/password.rb +++ b/lib/doorkeeper/request/password.rb @@ -3,7 +3,7 @@ module Doorkeeper module Request class Password < Strategy - delegate :credentials, :resource_owner, :parameters, :client, to: :server + delegate :credentials, :resource_owner, :dpop_proof, :parameters, :client, to: :server def request @request ||= OAuth::PasswordAccessTokenRequest.new( @@ -11,7 +11,8 @@ def request client, credentials, resource_owner, - parameters, + dpop_proof:, + parameters:, ) end end diff --git a/lib/doorkeeper/request/refresh_token.rb b/lib/doorkeeper/request/refresh_token.rb index 94a3c986c..fa85f54aa 100644 --- a/lib/doorkeeper/request/refresh_token.rb +++ b/lib/doorkeeper/request/refresh_token.rb @@ -3,7 +3,7 @@ module Doorkeeper module Request class RefreshToken < Strategy - delegate :credentials, :parameters, to: :server + delegate :credentials, :dpop_proof, :parameters, to: :server def refresh_token Doorkeeper.config.access_token_model.by_refresh_token(parameters[:refresh_token]) @@ -14,7 +14,8 @@ def request Doorkeeper.config, refresh_token, credentials, - parameters, + dpop_proof:, + parameters:, ) end end diff --git a/lib/doorkeeper/server.rb b/lib/doorkeeper/server.rb index 4ba0e3665..78811dd31 100644 --- a/lib/doorkeeper/server.rb +++ b/lib/doorkeeper/server.rb @@ -40,5 +40,9 @@ def credentials methods = Doorkeeper.config.client_credentials_methods @credentials ||= OAuth::Client::Credentials.from_request(context.request, *methods) end + + def dpop_proof + @dpop_proof ||= OAuth::DPoPProof.new(context.request) + end end end diff --git a/lib/doorkeeper/validations.rb b/lib/doorkeeper/validations.rb index d82ced79e..8fb7e5002 100644 --- a/lib/doorkeeper/validations.rb +++ b/lib/doorkeeper/validations.rb @@ -17,7 +17,7 @@ def validate def valid? validate - @error.nil? + error.nil? end module ClassMethods diff --git a/lib/generators/doorkeeper/dpop_generator.rb b/lib/generators/doorkeeper/dpop_generator.rb new file mode 100644 index 000000000..a564bea3c --- /dev/null +++ b/lib/generators/doorkeeper/dpop_generator.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require "rails/generators" +require "rails/generators/active_record" + +module Doorkeeper + # Generates migration with DPOP required database column for + # Doorkeeper access token table. + # + class DpopGenerator < ::Rails::Generators::Base + include ::Rails::Generators::Migration + source_root File.expand_path("templates", __dir__) + desc "Provide support for DPoP." + + def pkce + migration_template( + "enable_dpop_migration.rb.erb", + "db/migrate/enable_dpop.rb", + migration_version: migration_version, + ) + end + + def self.next_migration_number(dirname) + ActiveRecord::Generators::Base.next_migration_number(dirname) + end + + private + + def migration_version + "[#{ActiveRecord::VERSION::MAJOR}.#{ActiveRecord::VERSION::MINOR}]" + end + end +end diff --git a/lib/generators/doorkeeper/templates/enable_dpop_migration.rb.erb b/lib/generators/doorkeeper/templates/enable_dpop_migration.rb.erb new file mode 100644 index 000000000..dc64f0aa2 --- /dev/null +++ b/lib/generators/doorkeeper/templates/enable_dpop_migration.rb.erb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class EnableDpop < ActiveRecord::Migration<%= migration_version %> + def change + add_column :oauth_access_tokens, :dpop_jkt, :string, null: true + add_index :oauth_access_tokens, :dpop_jkt + end +end diff --git a/spec/controllers/protected_resources_controller_spec.rb b/spec/controllers/protected_resources_controller_spec.rb index 07ebebf99..ab1790f32 100644 --- a/spec/controllers/protected_resources_controller_spec.rb +++ b/spec/controllers/protected_resources_controller_spec.rb @@ -34,6 +34,7 @@ def index Doorkeeper::AccessToken, acceptable?: true, previous_refresh_token: "", revoke_previous_refresh_token!: true, + uses_dpop?: false, ) end @@ -117,6 +118,7 @@ def index accessible?: true, scopes: %w[write public], previous_refresh_token: "", revoke_previous_refresh_token!: true, + uses_dpop?: false, ) expect(token).to receive(:acceptable?).with([:write]).and_return(true) expect( @@ -133,6 +135,7 @@ def index accessible?: true, scopes: ["public"], revoked?: false, expired?: false, previous_refresh_token: "", revoke_previous_refresh_token!: true, + uses_dpop?: false, ) expect( Doorkeeper::AccessToken, @@ -237,6 +240,7 @@ def doorkeeper_forbidden_render_options(*); end accessible?: true, scopes: ["public"], revoked?: false, expired?: false, previous_refresh_token: "", revoke_previous_refresh_token!: true, + uses_dpop?: false, ) end @@ -354,10 +358,177 @@ def doorkeeper_forbidden_render_options(*) end end + context "when token has mismatched public keys" do + it "raises Doorkeeper::Errors::TokenInvalidDPoPKeyBinding exception", token: :dpop do + expect do + @request.env["HTTP_AUTHORIZATION"] = "DPoP #{token_string}" + @request.env["HTTP_DPOP"] = + build_dpop_proof( + ath: Base64.urlsafe_encode64(Digest::SHA256.digest(token_string), padding: false), + htm: "GET", + htu: "http://test.host/anonymous", + signing_key: OpenSSL::PKey::EC.generate("prime256v1"), + ) + get :index + end.to raise_error(Doorkeeper::Errors::TokenInvalidDPoPKeyBinding) + end + end + context "when token is valid" do it "allows into index action", token: :valid do expect(response).to be_successful end end end + + context "when using dpop", token: :dpop do + def build_dpop_proof(ath: Base64.urlsafe_encode64(Digest::SHA256.digest(token_string), padding: false), + htm: "GET", + htu: "http://test.host/anonymous", + signing_key: self.signing_key) + super + end + + controller do + before_action :doorkeeper_authorize! + + include ControllerActions + + def doorkeeper_unauthorized_render_options(error: nil) + { json: ActiveSupport::JSON.encode(error: error.name, error_description: error.description) } + end + end + + it "accepts a valid dpop proof" do + request.env["HTTP_AUTHORIZATION"] = "DPoP #{token_string}" + request.env["HTTP_DPOP"] = build_dpop_proof + get :index + + expect(response).to be_successful + end + + it "rejects an invalid dpop proof with incorrect request details" do + request.env["HTTP_AUTHORIZATION"] = "DPoP #{token_string}" + request.env["HTTP_DPOP"] = build_dpop_proof(htm: "X") + get :index + + expect(response).to be_unauthorized + expect(response.header["WWW-Authenticate"]).to match(/^Bearer, DPoP/) + + expect(json_response["error"]).to match("invalid_dpop_proof") + expect(json_response["error_description"]).to match("Invalid DPoP proof") + end + + it "rejects an invalid dpop proof with ath claim" do + request.env["HTTP_AUTHORIZATION"] = "DPoP #{token_string}" + request.env["HTTP_DPOP"] = build_dpop_proof(ath: "X") + get :index + + expect(response).to be_unauthorized + expect(response.header["WWW-Authenticate"]).to match(/^Bearer, DPoP/) + + expect(json_response["error"]).to match("invalid_dpop_proof") + expect(json_response["error_description"]).to match("Invalid DPoP proof") + end + + it "rejects an invalid dpop proof with mismatched public keys" do + request.env["HTTP_AUTHORIZATION"] = "DPoP #{token_string}" + request.env["HTTP_DPOP"] = build_dpop_proof(signing_key: OpenSSL::PKey::EC.generate("prime256v1")) + get :index + + expect(response).to be_unauthorized + expect(response.header["WWW-Authenticate"]).to match(/^Bearer, DPoP/) + + expect(json_response["error"]).to match("invalid_token") + expect(json_response["error_description"]).to match("Invalid DPoP key binding") + end + + it "rejects a dpop token received as a bearer token" do + request.env["HTTP_AUTHORIZATION"] = "Bearer #{token_string}" + get :index + + expect(response).to be_unauthorized + expect(response.header["WWW-Authenticate"]).to match(/^Bearer.*DPoP algs="ES256 PS256"$/) + + expect(json_response["error"]).to match("invalid_token") + expect(json_response["error_description"]).to match("The access token is invalid") + end + + it "rejects a bearer token received as a dpop token", token: :valid do + request.env["HTTP_AUTHORIZATION"] = "DPoP #{token_string}" + get :index + + expect(response).to be_unauthorized + expect(response.header["WWW-Authenticate"]).to match(/^Bearer, DPoP/) + + expect(json_response["error"]).to match("invalid_dpop_proof") + expect(json_response["error_description"]).to match("Invalid DPoP proof") + end + + it "rejects an empty dpop proof" do + request.env["HTTP_AUTHORIZATION"] = "DPoP #{token_string}" + get :index + + expect(response).to be_unauthorized + expect(response.header["WWW-Authenticate"]).to match(/^Bearer, DPoP/) + + expect(json_response["error"]).to match("invalid_dpop_proof") + expect(json_response["error_description"]).to match("Invalid DPoP proof") + end + + it "rejects a request without an authorization token" do + request.env["HTTP_DPOP"] = build_dpop_proof + get :index + + expect(response).to be_unauthorized + expect(response.header["WWW-Authenticate"]).to include("Bearer") + expect(response.header["WWW-Authenticate"]).to include("DPoP") + expect(response.header["WWW-Authenticate"]).to include("error=").twice + + expect(json_response["error"]).to match("invalid_token") + expect(json_response["error_description"]).to match("The access token is invalid") + end + + context "when dpop is not supported" do + before do + allow(Doorkeeper.config.access_token_model).to receive(:dpop_supported?).and_return(false) + end + + it "rejects an invalid token and challenges with only bearer", token: :invalid do + request.env["HTTP_AUTHORIZATION"] = "Bearer #{token_string}" + get :index + + expect(response).to be_unauthorized + expect(response.header["WWW-Authenticate"]).to match(/^Bearer/) + expect(response.header["WWW-Authenticate"]).not_to include("DPoP") + + expect(json_response["error"]).to match("invalid_token") + expect(json_response["error_description"]).to match("The access token is invalid") + end + end + + context "when dpop required" do + controller do + before_action { doorkeeper_authorize!(dpop: :required) } + + include ControllerActions + + def doorkeeper_unauthorized_render_options(error: nil) + { json: ActiveSupport::JSON.encode(error: error.name, error_description: error.description) } + end + end + + it "rejects a valid bearer token", token: :valid do + request.env["HTTP_AUTHORIZATION"] = "Bearer #{token_string}" + get :index + + expect(response).to be_unauthorized + expect(response.header["WWW-Authenticate"]).to match(/^DPoP/) + expect(response.header["WWW-Authenticate"]).not_to include("Bearer") + + expect(json_response["error"]).to match("invalid_token") + expect(json_response["error_description"]).to match("The access token is invalid") + end + end + end end diff --git a/spec/controllers/tokens_controller_spec.rb b/spec/controllers/tokens_controller_spec.rb index a17357025..d698d11ec 100644 --- a/spec/controllers/tokens_controller_spec.rb +++ b/spec/controllers/tokens_controller_spec.rb @@ -657,5 +657,25 @@ def create ) end end + + context "when access token uses dpop" do + let(:token_for_introspection) { FactoryBot.create(:access_token, application: client, dpop_jkt: "secret") } + + it "responds with full token introspection" do + request.headers["Authorization"] = basic_auth_header_for_client(client) + + post :introspect, params: { token: token_for_introspection.token } + + expect(json_response).to match( + "active" => true, + "client_id" => client.uid, + "token_type" => "DPoP", + "scope" => nil, + "exp" => an_instance_of(Integer), + "iat" => an_instance_of(Integer), + "cnf" => { "jkt" => "secret" }, + ) + end + end end end diff --git a/spec/dummy/db/migrate/20260205014939_enable_dpop.rb b/spec/dummy/db/migrate/20260205014939_enable_dpop.rb new file mode 100644 index 000000000..4535a3649 --- /dev/null +++ b/spec/dummy/db/migrate/20260205014939_enable_dpop.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class EnableDpop < ActiveRecord::Migration[8.0] + def change + add_column :oauth_access_tokens, :dpop_jkt, :string, null: true + add_index :oauth_access_tokens, :dpop_jkt + end +end diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 7ed67be14..ca704dde3 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -2,16 +2,15 @@ # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. # -# Note that this schema.rb definition is the authoritative source for your -# database schema. If you need to create the application database on another -# system, you should be using db:schema:load, not running all the migrations -# from scratch. The latter is a flawed and unsustainable approach (the more migrations -# you'll amass, the slower it'll run and the greater likelihood for issues). +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20230205064514) do - +ActiveRecord::Schema.define(version: 2026_02_05_014939) do create_table "oauth_access_grants", force: :cascade do |t| t.integer "resource_owner_id", null: false t.string "resource_owner_type" # [NOTE] null: false skipped to allow test pass @@ -24,8 +23,8 @@ t.string "scopes" t.string "tenant_name" unless ENV["WITHOUT_PKCE"] - t.string "code_challenge" - t.string "code_challenge_method" + t.string "code_challenge" + t.string "code_challenge_method" end t.index ["token"], name: "index_oauth_access_grants_on_token", unique: true end @@ -42,6 +41,8 @@ t.string "scopes" t.string "previous_refresh_token", default: "", null: false t.string "tenant_name" + t.string "dpop_jkt" + t.index ["dpop_jkt"], name: "index_oauth_access_tokens_on_dpop_jkt" t.index ["refresh_token"], name: "index_oauth_access_tokens_on_refresh_token", unique: true t.index ["resource_owner_id"], name: "index_oauth_access_tokens_on_resource_owner_id" t.index ["token"], name: "index_oauth_access_tokens_on_token", unique: true @@ -68,5 +69,4 @@ t.datetime "updated_at" t.string "password" end - end diff --git a/spec/generators/dpop_generator_spec.rb b/spec/generators/dpop_generator_spec.rb new file mode 100644 index 000000000..7fbc429b9 --- /dev/null +++ b/spec/generators/dpop_generator_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require "spec_helper" +require "generators/doorkeeper/dpop_generator" + +RSpec.describe Doorkeeper::DpopGenerator do + include GeneratorSpec::TestCase + + tests described_class + destination ::File.expand_path("tmp/dummy", __dir__) + + describe "after running the generator" do + before do + prepare_destination + end + + it "creates a migration with a version specifier" do + stub_const("ActiveRecord::VERSION::MAJOR", 5) + stub_const("ActiveRecord::VERSION::MINOR", 0) + + run_generator + + assert_migration "db/migrate/enable_dpop.rb" do |migration| + assert migration.include?("ActiveRecord::Migration[5.0]\n") + end + end + end +end diff --git a/spec/grape/grape_integration_spec.rb b/spec/grape/grape_integration_spec.rb index eff59e923..1482c07a5 100644 --- a/spec/grape/grape_integration_spec.rb +++ b/spec/grape/grape_integration_spec.rb @@ -50,6 +50,30 @@ class API < Grape::API end end + resource :protected_with_endpoint_dpop_required do + before do + doorkeeper_authorize! + end + + desc "Protected resource, requires DPoP token (defined in endpoint)." + + get :status, dpop: :required do + { response: "OK" } + end + end + + resource :protected_with_helper_dpop_required do + before do + doorkeeper_authorize! dpop: :required + end + + desc "Protected resource, requires DPoP token (defined in helper)." + + get :status do + { response: "OK" } + end + end + resource :public do desc "Public resource, no token required." @@ -108,6 +132,58 @@ def json_body expect(last_response).to be_successful expect(json_body).to have_key("response") end + + it "fails request for protected resource that requires dpop (Grape endpoint)" do + get "api/v1/protected_with_endpoint_dpop_required/status.json?access_token=#{access_token.token}" + + expect(last_response).not_to be_successful + expect(json_body).to have_key("error") + end + + it "fails request for protected resource that requires dpop (Doorkeeper helper)" do + get "api/v1/protected_with_helper_dpop_required/status.json?access_token=#{access_token.token}" + + expect(last_response).not_to be_successful + expect(json_body).to have_key("error") + end + end + + context "with dpop Token", token: :dpop do + def build_dpop_proof(htu:, + ath: Base64.urlsafe_encode64(Digest::SHA256.digest(token_string), padding: false), + htm: "GET", + signing_key: self.signing_key) + super + end + + it "successfully requests protected resource" do + get "api/v1/protected/status.json", + {}, + "HTTP_AUTHORIZATION" => "DPoP #{token_string}", + "HTTP_DPOP" => build_dpop_proof(htu: "http://example.org/api/v1/protected/status.json") + + expect(last_response).to be_successful + + expect(json_body["token"]).to eq(token.token) + end + + it "successfully requests protected resource that requires dpop (Grape endpoint)" do + get "api/v1/protected_with_endpoint_dpop_required/status.json", + {}, + "HTTP_AUTHORIZATION" => "DPoP #{token_string}", + "HTTP_DPOP" => build_dpop_proof(htu: "http://example.org/api/v1/protected_with_endpoint_dpop_required/status.json") + + expect(last_response).to be_successful + end + + it "successfully requests protected resource that requires dpop (Doorkeeper helper)" do + get "api/v1/protected_with_helper_dpop_required/status.json", + {}, + "HTTP_AUTHORIZATION" => "DPoP #{token_string}", + "HTTP_DPOP" => build_dpop_proof(htu: "http://example.org/api/v1/protected_with_helper_dpop_required/status.json") + + expect(last_response).to be_successful + end end context "with invalid Access Token" do diff --git a/spec/lib/config_spec.rb b/spec/lib/config_spec.rb index 83970ec89..ceb3616c8 100644 --- a/spec/lib/config_spec.rb +++ b/spec/lib/config_spec.rb @@ -324,7 +324,7 @@ describe "access_token_methods" do it "has defaults order" do expect(config.access_token_methods) - .to eq(%i[from_bearer_authorization from_access_token_param from_bearer_param]) + .to eq(%i[from_dpop_authorization from_bearer_authorization from_access_token_param from_bearer_param]) end it "can change the value" do diff --git a/spec/lib/oauth/authorization_code_request_spec.rb b/spec/lib/oauth/authorization_code_request_spec.rb index 306d71c32..1d4d51d04 100644 --- a/spec/lib/oauth/authorization_code_request_spec.rb +++ b/spec/lib/oauth/authorization_code_request_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Doorkeeper::OAuth::AuthorizationCodeRequest do subject(:request) do - described_class.new(server, grant, client, params) + described_class.new(server, grant, client, dpop_proof:, parameters: params) end let(:server) do @@ -25,6 +25,7 @@ let(:client) { grant.application } let(:redirect_uri) { client.redirect_uri } let(:params) { { redirect_uri: redirect_uri } } + let(:dpop_proof) { nil } before do allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true) @@ -54,33 +55,33 @@ end it "requires the grant" do - request = described_class.new(server, nil, client, params) + request = described_class.new(server, nil, client, parameters: params) request.validate expect(request.error).to eq(Doorkeeper::Errors::InvalidGrant) end it "requires the client" do - request = described_class.new(server, grant, nil, params) + request = described_class.new(server, grant, nil, parameters: params) request.validate expect(request.error).to eq(Doorkeeper::Errors::InvalidClient) end it "requires the redirect_uri" do - request = described_class.new(server, grant, nil, params.except(:redirect_uri)) + request = described_class.new(server, grant, nil, parameters: params.except(:redirect_uri)) request.validate expect(request.error).to eq(Doorkeeper::Errors::InvalidRequest) expect(request.missing_param).to eq(:redirect_uri) end it "matches the redirect_uri with grant's one" do - request = described_class.new(server, grant, client, params.merge(redirect_uri: "http://other.com")) + request = described_class.new(server, grant, client, parameters: params.merge(redirect_uri: "http://other.com")) request.validate expect(request.error).to eq(Doorkeeper::Errors::InvalidGrant) end it "matches the client with grant's one" do other_client = FactoryBot.create :application - request = described_class.new(server, grant, other_client, params) + request = described_class.new(server, grant, other_client, parameters: params) request.validate expect(request.error).to eq(Doorkeeper::Errors::InvalidGrant) end @@ -197,7 +198,7 @@ context "when the app is missing" do it "does not assume non-confidential and forcibly validate pkce params" do - request = described_class.new(server, grant, nil, params) + request = described_class.new(server, grant, nil, parameters: params) request.validate expect(request.error).to eq(Doorkeeper::Errors::InvalidClient) end @@ -312,4 +313,6 @@ expect { request.authorize }.to(change { previous_token.reload.revoked_at }) end end + + include_examples "sender-constraining access_token using dpop" end diff --git a/spec/lib/oauth/client_credentials_integration_spec.rb b/spec/lib/oauth/client_credentials_integration_spec.rb index dd9f8ab35..21da0abb5 100644 --- a/spec/lib/oauth/client_credentials_integration_spec.rb +++ b/spec/lib/oauth/client_credentials_integration_spec.rb @@ -9,7 +9,7 @@ let(:client) { Doorkeeper::OAuth::Client.new(FactoryBot.build_stubbed(:application)) } it "issues an access token" do - request = described_class.new(server, client, {}) + request = described_class.new(server, client) expect do request.authorize end.to change { Doorkeeper::AccessToken.count }.by(1) @@ -18,7 +18,7 @@ describe "with an invalid request" do it "does not issue an access token" do - request = described_class.new(server, nil, {}) + request = described_class.new(server, nil) expect do request.authorize end.not_to(change { Doorkeeper::AccessToken.count }) diff --git a/spec/lib/oauth/client_credentials_request_spec.rb b/spec/lib/oauth/client_credentials_request_spec.rb index 8aa53721b..c7b6935d2 100644 --- a/spec/lib/oauth/client_credentials_request_spec.rb +++ b/spec/lib/oauth/client_credentials_request_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" RSpec.describe Doorkeeper::OAuth::ClientCredentialsRequest do - subject(:request) { described_class.new(server, client) } + subject(:request) { described_class.new(server, client, dpop_proof:) } let(:server) do double( @@ -15,7 +15,8 @@ let(:application) { FactoryBot.create(:application, scopes: "") } let(:client) { double :client, application: application, scopes: "" } - let(:token_creator) { double :issuer, create: true, token: double } + let(:token_creator) { double :issuer, create: true, error: nil, token: double } + let(:dpop_proof) { nil } before do allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true) @@ -62,7 +63,7 @@ end it "issues an access token with requested scopes" do - request = described_class.new(server, client, scope: "email") + request = described_class.new(server, client, parameters: { scope: "email" }) allow(request).to receive(:issuer).and_return(token_creator) expect(token_creator).to receive(:create).with(client, Doorkeeper::OAuth::Scopes.from_string("email"), {}) request.authorize @@ -77,7 +78,7 @@ end it "issues an access token with the custom access token attributes" do - request = described_class.new(server, client, scope: "email", tenant_id: 9000) + request = described_class.new(server, client, parameters: { scope: "email", tenant_id: 9000 }) allow(request).to receive(:issuer).and_return(token_creator) expect(token_creator).to receive(:create).with(client, Doorkeeper::OAuth::Scopes.from_string("email"), { tenant_id: 9000 }) request.authorize @@ -111,10 +112,16 @@ end it "issues an access token with requested scopes" do - request = described_class.new(server, client, scope: "phone") + request = described_class.new(server, client, parameters: { scope: "phone" }) request.authorize expect(request.response).to be_a(Doorkeeper::OAuth::TokenResponse) expect(request.response.token.scopes_string).to eq("phone") end end + + include_examples( + "sender-constraining access_token using dpop", + when_bearer_token_expected: -> { expect(token_creator).to receive(:create).with(client, nil, {}) }, + when_dpop_token_expected: -> { expect(token_creator).to receive(:create).with(client, nil, { dpop_jkt: "jkt_123" }) }, + ) end diff --git a/spec/lib/oauth/code_response_spec.rb b/spec/lib/oauth/code_response_spec.rb index 99e1e3958..a29628f47 100644 --- a/spec/lib/oauth/code_response_spec.rb +++ b/spec/lib/oauth/code_response_spec.rb @@ -13,6 +13,7 @@ state: "state", scopes: Doorkeeper::OAuth::Scopes.from_string("public"), custom_access_token_attributes: {}, + dpop_token_attributes: {}, ) end diff --git a/spec/lib/oauth/dpop_proof_spec.rb b/spec/lib/oauth/dpop_proof_spec.rb new file mode 100644 index 000000000..2195e772f --- /dev/null +++ b/spec/lib/oauth/dpop_proof_spec.rb @@ -0,0 +1,224 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Doorkeeper::OAuth::DPoPProof do + subject(:dpop_proof) { described_class.new(request, access_token) } + + let(:request) do + instance_double(ActionDispatch::Request, base_url:, headers: request_headers, path:, request_method:) + end + let(:base_url) { "https://protected.example.net" } + let(:path) { "/resource" } + let(:request_headers) { ActionDispatch::Http::Headers.from_hash("HTTP_DPOP" => dpop_header) } + let(:request_method) { "GET" } + + let(:access_token) { nil } + + let(:dpop_header) { JWT.encode(claims, signing_key, alg, jwt_headers) } + + let(:claims) { { "jti" => "jti_01", "iat" => iat, "htm" => htm, "htu" => htu } } + let(:htm) { request_method } + let(:htu) { base_url + path } + let(:iat) { Time.current.to_i } + + let(:jwt_headers) { { "typ" => typ, "alg" => alg, "jwk" => jwk } } + let(:alg) { "ES256" } + let(:jwk) { JWT::JWK.new(signing_key).export } + let(:signing_key) { OpenSSL::PKey::EC.generate("prime256v1") } + let(:typ) { "dpop+jwt" } + + before do + allow(Doorkeeper).to receive(:config).and_return( + instance_double(Doorkeeper::Config, dpop_iat_leeway: 300, dpop_signature_algorithms: ["ES256"]), + ) + + Timecop.freeze(Time.current) + end + + after { Timecop.return } + + shared_examples "invalid because" do |expected_error| + it "is invalid and has error #{expected_error}" do + dpop_proof.validate + + expect(dpop_proof).not_to be_valid + expect(dpop_proof.error).to eq(expected_error) + end + end + + describe "#validate" do + it "is valid and has no error" do + dpop_proof.validate + expect(dpop_proof).to be_valid + expect(dpop_proof.error).to be_nil + end + + context "when dpop header is missing" do + let(:dpop_header) { nil } + + include_examples "invalid because", :blank + end + + context "when dpop header is blank" do + let(:dpop_header) { "" } + + include_examples "invalid because", :blank + end + + context "when dpop header is not a jwt" do + let(:dpop_header) { "not-jwt" } + + include_examples "invalid because", :invalid_type + end + + describe "single_proof" do + context "when multiple proofs separated by comma" do + let(:dpop_header) { "a.b.c,d.e.f" } + + include_examples "invalid because", :multiple_dpop_proofs + end + + context "when multiple proofs separated by semicolon" do + let(:dpop_header) { "a.b.c;d.e.f" } + + include_examples "invalid because", :multiple_dpop_proofs + end + end + + describe "type" do + let(:typ) { "not-dpop" } + + include_examples "invalid because", :invalid_type + end + + describe "alg" do + let(:alg) { "RS256" } + let(:signing_key) { OpenSSL::PKey::RSA.generate(2048) } + + include_examples "invalid because", :invalid_signing_algorithm + end + + describe "jwk" do + context "when jwk is missing" do + let(:jwt_headers) { super().except("jwk") } + + include_examples "invalid because", :invalid_jwk + end + + context "when jwk is not a hash" do + let(:jwk) { "not-a-jwk" } + + include_examples "invalid because", :invalid_jwk + end + + context "when jwk includes private material" do + let(:jwk) { JWT::JWK.new(signing_key).export(include_private: true) } + + include_examples "invalid because", :invalid_jwk + end + end + + describe "jti" do + let(:claims) { super().except("jti") } + + include_examples "invalid because", :invalid_jti + end + + describe "iat" do + context "when iat is missing" do + let(:claims) { super().except("iat") } + + include_examples "invalid because", :invalid_iat + end + + context "when iat outside leeway in the future" do + let(:claims) { super().merge("iat" => iat + Doorkeeper.config.dpop_iat_leeway + 1) } + + include_examples "invalid because", :invalid_iat + end + + context "when iat outside leeway in the past" do + let(:claims) { super().merge("iat" => iat - Doorkeeper.config.dpop_iat_leeway - 1) } + + include_examples "invalid because", :invalid_iat + end + end + + describe "ath" do + let(:access_token) { "access_token_01" } + + context "when ath is missing" do + include_examples "invalid because", :invalid_ath + end + + context "when ath mismatches access_token" do + let(:claims) { super().merge("ath" => "wrong") } + + include_examples "invalid because", :invalid_ath + end + + context "when ath matches access_token" do + let(:claims) do + digest = Digest::SHA256.digest(access_token) + super().merge("ath" => Base64.urlsafe_encode64(digest, padding: false)) + end + + it "is valid" do + dpop_proof.validate + expect(dpop_proof).to be_valid + expect(dpop_proof.error).to be_nil + end + end + end + + describe "htm" do + let(:htm) { "POST" } + + include_examples "invalid because", :invalid_htm + end + + describe "htu" do + context "when htu does not match request URI" do + let(:htu) { "#{base_url}/other" } + + include_examples "invalid because", :invalid_htu + end + + context "when htu includes query string" do + let(:htu) { "#{base_url}#{path}?foo=bar" } + + include_examples "invalid because", :invalid_htu + end + end + + describe "signature" do + context "when jwt was signed with different private key pair to the jwk in the header" do + let(:dpop_header) do + JWT.encode(claims, OpenSSL::PKey::EC.generate("prime256v1"), alg, jwt_headers) + end + + include_examples "invalid because", :invalid_signature + end + end + end + + describe "#jkt" do + context "when the proof is valid" do + it "returns the thumbprint for the public jwk in the header" do + expected_jwk = JWT::JWK.import(jwt_headers.fetch("jwk")) + expected_jkt = JWT::JWK::Thumbprint.new(expected_jwk).generate + + expect(dpop_proof.jkt).to eq(expected_jkt) + end + end + + context "when the proof is invalid" do + let(:dpop_header) { "not-jwt" } + + it "returns nil" do + expect(dpop_proof.jkt).to be_nil + end + end + end +end diff --git a/spec/lib/oauth/error_response_spec.rb b/spec/lib/oauth/error_response_spec.rb index 0f7a5f374..2ee52193d 100644 --- a/spec/lib/oauth/error_response_spec.rb +++ b/spec/lib/oauth/error_response_spec.rb @@ -70,6 +70,8 @@ describe "WWW-Authenticate header" do subject(:headers) { error_response.headers["WWW-Authenticate"] } + before { allow(Doorkeeper::AccessToken).to receive(:dpop_supported?).and_return(false) } + it { expect(headers).to include("realm=\"#{error_response.send(:realm)}\"") } it { expect(headers).to include("error=\"#{error_response.name}\"") } it { expect(headers).to include("error_description=\"#{error_response.description}\"") } diff --git a/spec/lib/oauth/invalid_token_response_spec.rb b/spec/lib/oauth/invalid_token_response_spec.rb index f1e90bb65..24297c164 100644 --- a/spec/lib/oauth/invalid_token_response_spec.rb +++ b/spec/lib/oauth/invalid_token_response_spec.rb @@ -41,7 +41,7 @@ end context "when unknown" do - let(:access_token) { double(revoked?: false, expired?: false) } + let(:access_token) { double(revoked?: false, expired?: false, uses_dpop?: false) } it "sets a description" do expect(response.description).to include("invalid") diff --git a/spec/lib/oauth/password_access_token_request_spec.rb b/spec/lib/oauth/password_access_token_request_spec.rb index 448e325be..76f83f221 100644 --- a/spec/lib/oauth/password_access_token_request_spec.rb +++ b/spec/lib/oauth/password_access_token_request_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Doorkeeper::OAuth::PasswordAccessTokenRequest do subject(:request) do - described_class.new(server, client, credentials, owner) + described_class.new(server, client, credentials, owner, dpop_proof:) end let(:server) do @@ -22,6 +22,7 @@ let(:credentials) { Doorkeeper::OAuth::Client::Credentials.new("uid", "secret") } let(:application) { client.application } let(:owner) { FactoryBot.build_stubbed(:resource_owner) } + let(:dpop_proof) { nil } before do allow(server).to receive(:option_defined?).with(:custom_access_token_expires_in).and_return(true) @@ -63,7 +64,7 @@ end it "doesn't issue a new token with an invalid client" do - request = described_class.new(server, nil, credentials, owner, { client_id: "bad_id" }) + request = described_class.new(server, nil, credentials, owner, parameters: { client_id: "bad_id" }) expect do request.authorize end.not_to(change { Doorkeeper::AccessToken.count }) @@ -131,7 +132,7 @@ describe "with scopes" do subject(:request) do - described_class.new(server, client, credentials, owner, scope: "public") + described_class.new(server, client, credentials, owner, parameters: { scope: "public" }) end context "when scopes_by_grant_type is not configured for grant_type" do @@ -197,7 +198,7 @@ end it "checks scopes" do - request = described_class.new(server, client, credentials, owner, scope: "public") + request = described_class.new(server, client, credentials, owner, parameters: { scope: "public" }) allow(server).to receive(:scopes).and_return(Doorkeeper::OAuth::Scopes.from_string("public")) expect do @@ -208,7 +209,7 @@ end it "falls back to the default otherwise" do - request = described_class.new(server, client, credentials, owner, scope: "private") + request = described_class.new(server, client, credentials, owner, parameters: { scope: "private" }) allow(server).to receive(:scopes).and_return(Doorkeeper::OAuth::Scopes.from_string("private")) expect do @@ -218,4 +219,6 @@ expect(Doorkeeper::AccessToken.last.expires_in).to eq(2.hours) end end + + include_examples "sender-constraining access_token using dpop" end diff --git a/spec/lib/oauth/refresh_token_request_spec.rb b/spec/lib/oauth/refresh_token_request_spec.rb index f3612b2d8..c1cd9bdff 100644 --- a/spec/lib/oauth/refresh_token_request_spec.rb +++ b/spec/lib/oauth/refresh_token_request_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" RSpec.describe Doorkeeper::OAuth::RefreshTokenRequest do - subject(:request) { described_class.new(server, refresh_token, credentials) } + subject(:request) { described_class.new(server, refresh_token, credentials, dpop_proof:) } let(:server) do double :server, access_token_expires_in: 2.minutes @@ -15,6 +15,7 @@ let(:client) { refresh_token.application } let(:credentials) { Doorkeeper::OAuth::Client::Credentials.new(client.uid, client.secret) } + let(:dpop_proof) { nil } before do allow(Doorkeeper::AccessToken).to receive(:refresh_token_revoked_on_use?).and_return(false) @@ -150,7 +151,7 @@ end context "with scopes" do - subject(:request) { described_class.new(server, refresh_token, credentials, parameters) } + subject(:request) { described_class.new(server, refresh_token, credentials, parameters:) } let(:refresh_token) do FactoryBot.create :access_token, @@ -194,7 +195,7 @@ end context "with dynamic scopes enabled" do - subject(:request) { described_class.new(server, refresh_token, credentials, parameters) } + subject(:request) { described_class.new(server, refresh_token, credentials, parameters:) } let(:application_scopes) { "public write user:*" } let(:application) { FactoryBot.create(:application, scopes: application_scopes) } @@ -241,4 +242,6 @@ expect(Doorkeeper::AccessToken.last.scopes).to eq(%i[public]) end end + + include_examples "sender-constraining access_token using dpop" end diff --git a/spec/requests/flows/authorization_code_spec.rb b/spec/requests/flows/authorization_code_spec.rb index c0bd5495d..d3e738286 100644 --- a/spec/requests/flows/authorization_code_spec.rb +++ b/spec/requests/flows/authorization_code_spec.rb @@ -579,4 +579,98 @@ def authorize(redirect_url) expect(access_token.tenant_name).to eq("Tenant 1") end end + + context "when using dpop" do + scenario "resource owner requests an access token with valid dpop proof" do + visit authorization_endpoint_url(client: @client) + click_on "Authorize" + + authorization_code = Doorkeeper::AccessGrant.first.token + with_dpop_proof_header(htm: "POST", htu: "http://www.example.com/oauth/token") + create_access_token authorization_code, @client + + access_token_should_exist_for(@client, @resource_owner) + + expect(json_response).to match( + "access_token" => Doorkeeper::AccessToken.first.token, + "token_type" => "DPoP", + "expires_in" => 7200, + "scope" => "default", + "created_at" => an_instance_of(Integer), + ) + end + + scenario "resource owner requests an access token with an invalid dpop proof" do + visit authorization_endpoint_url(client: @client) + click_on "Authorize" + + authorization_code = Doorkeeper::AccessGrant.first.token + with_dpop_proof_header(htm: "X", htu: "X") + create_access_token authorization_code, @client + + access_token_should_not_exist + + response_status_should_be(400) + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => translated_error_message(:invalid_dpop_proof), + ) + end + + context "when dpop is not supported" do + before { allow(Doorkeeper::AccessToken).to receive(:dpop_supported?).and_return(false) } + + scenario "resource owner requests an access token with valid dpop proof" do + visit authorization_endpoint_url(client: @client) + click_on "Authorize" + + authorization_code = Doorkeeper::AccessGrant.first.token + with_dpop_proof_header(htm: "POST", htu: "http://www.example.com/oauth/token") + create_access_token authorization_code, @client + + access_token_should_exist_for(@client, @resource_owner) + + expect(json_response).to match( + "access_token" => Doorkeeper::AccessToken.first.token, + "token_type" => "Bearer", + "expires_in" => 7200, + "scope" => "default", + "created_at" => an_instance_of(Integer), + ) + end + + scenario "resource owner requests an access token with an invalid dpop proof" do + visit authorization_endpoint_url(client: @client) + click_on "Authorize" + + authorization_code = Doorkeeper::AccessGrant.first.token + with_dpop_proof_header(htm: "X", htu: "X") + create_access_token authorization_code, @client + + access_token_should_exist_for(@client, @resource_owner) + + response_status_should_be(200) + end + end + + context "when dpop is required" do + before { config_is_set(:force_dpop, true) } + + scenario "resource owner requests an access token without a dpop proof" do + visit authorization_endpoint_url(client: @client) + click_on "Authorize" + + authorization_code = Doorkeeper::AccessGrant.first.token + create_access_token authorization_code, @client + + access_token_should_not_exist + + response_status_should_be(400) + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => translated_error_message(:invalid_dpop_proof), + ) + end + end + end end diff --git a/spec/requests/flows/client_credentials_spec.rb b/spec/requests/flows/client_credentials_spec.rb index d30765b51..c57f096b2 100644 --- a/spec/requests/flows/client_credentials_spec.rb +++ b/spec/requests/flows/client_credentials_spec.rb @@ -226,8 +226,95 @@ end end + context "when using dpop" do + it "authorizes the client and returns the dpop token response if the dpop proof is valid" do + headers = + authorization(client.uid, client.secret) + .merge(dpop(htm: "POST", htu: "http://www.example.com/oauth/token")) + + params = { grant_type: "client_credentials" } + + post "/oauth/token", params: params, headers: headers + + expect(json_response).to match( + "access_token" => Doorkeeper::AccessToken.first.token, + "token_type" => "DPoP", + "expires_in" => Doorkeeper.configuration.access_token_expires_in, + "created_at" => an_instance_of(Integer), + ) + end + + it "does not authorize the client and returns the error if the dpop proof is invalid" do + headers = authorization(client.uid, client.secret).merge(dpop(htm: "X", htu: "X")) + params = { grant_type: "client_credentials" } + + post "/oauth/token", params: params, headers: headers + + access_token_should_not_exist + + response_status_should_be(400) + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => translated_error_message(:invalid_dpop_proof), + ) + end + + context "when dpop is not supported" do + before { allow(Doorkeeper::AccessToken).to receive(:dpop_supported?).and_return(false) } + + it "authorizes the client and returns the bearer token response if the dpop proof is valid" do + headers = + authorization(client.uid, client.secret) + .merge(dpop(htm: "POST", htu: "http://www.example.com/oauth/token")) + + params = { grant_type: "client_credentials" } + + post "/oauth/token", params: params, headers: headers + + expect(json_response).to match( + "access_token" => Doorkeeper::AccessToken.first.token, + "token_type" => "Bearer", + "expires_in" => Doorkeeper.configuration.access_token_expires_in, + "created_at" => an_instance_of(Integer), + ) + end + + it "authorizes the client validates the request without trying to validate the would be invalid dpop proof" do + headers = authorization(client.uid, client.secret).merge(dpop(htm: "X", htu: "X")) + params = { grant_type: "client_credentials" } + + post "/oauth/token", params: params, headers: headers + + response_status_should_be(200) + end + end + + context "when dpop is required" do + before { config_is_set(:force_dpop, true) } + + it "does not authorize the client and returns the error if the dpop proof is missing" do + headers = authorization(client.uid, client.secret) + params = { grant_type: "client_credentials" } + + post "/oauth/token", params: params, headers: headers + + access_token_should_not_exist + + response_status_should_be(400) + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => translated_error_message(:invalid_dpop_proof), + ) + end + end + end + def authorization(username, password) credentials = ActionController::HttpAuthentication::Basic.encode_credentials username, password { "HTTP_AUTHORIZATION" => credentials } end + + def dpop(**kwargs) + { "HTTP_DPOP" => build_dpop_proof(**kwargs) } + end end diff --git a/spec/requests/flows/password_spec.rb b/spec/requests/flows/password_spec.rb index 64306a41e..bc87ab1da 100644 --- a/spec/requests/flows/password_spec.rb +++ b/spec/requests/flows/password_spec.rb @@ -411,5 +411,84 @@ end.not_to(change { Doorkeeper::AccessToken.count }) end end + + context "when using dpop" do + it "issues a new dpop token if the dpop proof is valid" do + expect do + post( + password_token_endpoint_url(client: @client, resource_owner: @resource_owner), + headers: { "HTTP_DPOP" => build_dpop_proof(htm: "POST", htu: "http://www.example.com/oauth/token") }, + ) + end.to change(Doorkeeper::AccessToken, :count).by(1) + + token = Doorkeeper::AccessToken.first + + expect(token.application_id).to eq(@client.id) + expect(json_response).to include("access_token" => token.token, "token_type" => "DPoP") + end + + it "doesn't issue new token if the dpop proof is invalid" do + expect do + post( + password_token_endpoint_url(client: @client, resource_owner: @resource_owner), + headers: { "HTTP_DPOP" => build_dpop_proof(htm: "X", htu: "X") }, + ) + end.not_to change(Doorkeeper::AccessToken, :count) + + expect(response.status).to eq(400) + expect(json_response).to include( + "error" => "invalid_dpop_proof", + "error_description" => an_instance_of(String), + ) + end + + context "when dpop is not supported" do + before { allow(Doorkeeper::AccessToken).to receive(:dpop_supported?).and_return(false) } + + it "issues a new bearer token if the dpop proof is valid" do + expect do + post( + password_token_endpoint_url(client: @client, resource_owner: @resource_owner), + headers: { "HTTP_DPOP" => build_dpop_proof(htm: "POST", htu: "http://www.example.com/oauth/token") }, + ) + end.to change(Doorkeeper::AccessToken, :count).by(1) + + token = Doorkeeper::AccessToken.first + + expect(token.application_id).to eq(@client.id) + expect(json_response).to include("access_token" => token.token, "token_type" => "Bearer") + end + + it "issues a new bearer token if the dpop proof is invalid" do + expect do + post( + password_token_endpoint_url(client: @client, resource_owner: @resource_owner), + headers: { "HTTP_DPOP" => build_dpop_proof(htm: "X", htu: "X") }, + ) + end.to change(Doorkeeper::AccessToken, :count).by(1) + + token = Doorkeeper::AccessToken.first + + expect(token.application_id).to eq(@client.id) + expect(json_response).to include("access_token" => token.token, "token_type" => "Bearer") + end + end + + context "when dpop is required" do + before { config_is_set(:force_dpop, true) } + + it "doesn't issue new token if the dpop proof is missing" do + expect do + post(password_token_endpoint_url(client: @client, resource_owner: @resource_owner)) + end.not_to change(Doorkeeper::AccessToken, :count) + + expect(response.status).to eq(400) + expect(json_response).to include( + "error" => "invalid_dpop_proof", + "error_description" => an_instance_of(String), + ) + end + end + end end end diff --git a/spec/requests/flows/refresh_token_spec.rb b/spec/requests/flows/refresh_token_spec.rb index c319bfedf..0a4ba2bc2 100644 --- a/spec/requests/flows/refresh_token_spec.rb +++ b/spec/requests/flows/refresh_token_spec.rb @@ -279,4 +279,297 @@ def last_token ) end end + + context "when using dpop" do + def build_dpop_proof(htm: "POST", htu: "http://www.example.com/oauth/token", signing_key: self.signing_key) + super + end + + let(:invalid_dpop_proof) { build_dpop_proof(htm: "X") } + let(:mismatched_dpop_proof) { build_dpop_proof(signing_key: OpenSSL::PKey::EC.generate("prime256v1")) } + let(:valid_dpop_proof) { build_dpop_proof } + + let(:jkt) { JWT::JWK::Thumbprint.new(JWT::JWK.new(signing_key)).generate } + let(:signing_key) { OpenSSL::PKey::EC.generate("prime256v1") } + + let!(:token) do + FactoryBot.create( + :access_token, + application: client, + resource_owner_id: resource_owner.id, + resource_owner_type: resource_owner.class.name, + use_refresh_token: true, + ) + end + + context "with a private client" do + def make_refresh_request(headers: {}) + post refresh_token_endpoint_url(client:, refresh_token: token.refresh_token), headers: + end + + let(:client) { @client } + + it "binds the new access token to the dpop proof's public key if it is valid" do + make_refresh_request(headers: { "HTTP_DPOP" => valid_dpop_proof }) + + new_token = Doorkeeper::AccessToken.last + expect(json_response).to include( + "access_token" => new_token.token, + "token_type" => "DPoP", + "refresh_token" => new_token.refresh_token, + ) + expect(new_token).to be_uses_dpop + expect(new_token.dpop_jkt).to eq(jkt) + end + + it "does not issue a new access token if dpop proof is invalid" do + expect do + make_refresh_request(headers: { "HTTP_DPOP" => invalid_dpop_proof }) + end.not_to change(Doorkeeper::AccessToken, :count) + + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => "Invalid DPoP proof", + ) + end + + context "when refreshing an access token that uses dpop" do + let!(:token) { super().tap { |it| it.update!(dpop_jkt: jkt) } } + + it "issues a new access token without presenting a dpop proof since it already presents client credentials to refresh" do + make_refresh_request + + new_token = Doorkeeper::AccessToken.last + expect(json_response).to include( + "access_token" => new_token.token, + "token_type" => "DPoP", + "refresh_token" => new_token.refresh_token, + ) + expect(new_token).to be_uses_dpop + expect(new_token.dpop_jkt).to eq(token.dpop_jkt) + end + + it "does not issue a new access token if dpop proof is invalid" do + expect do + make_refresh_request(headers: { "HTTP_DPOP" => invalid_dpop_proof }) + end.not_to change(Doorkeeper::AccessToken, :count) + + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => "Invalid DPoP proof", + ) + end + + it "binds the new access token to the dpop proof's new public key so long as it is valid" do + make_refresh_request(headers: { "HTTP_DPOP" => mismatched_dpop_proof }) + + new_token = Doorkeeper::AccessToken.last + expect(json_response).to include( + "access_token" => new_token.token, + "token_type" => "DPoP", + "refresh_token" => new_token.refresh_token, + ) + expect(new_token).to be_uses_dpop + expect(new_token.dpop_jkt).not_to eq(token.dpop_jkt) + + jkt_from_mismatched_dpop_proof = + JWT::JWK::Thumbprint.new(JWT::JWK.import(JWT.decode(mismatched_dpop_proof, nil, false)[1]["jwk"])).generate + expect(new_token.dpop_jkt).to eq(jkt_from_mismatched_dpop_proof) + end + end + + context "when dpop is not supported" do + before { allow(Doorkeeper::AccessToken).to receive(:dpop_supported?).and_return(false) } + + it "issues a new unbound access token if the dpop proof is valid" do + make_refresh_request(headers: { "HTTP_DPOP" => valid_dpop_proof }) + + new_token = Doorkeeper::AccessToken.last + expect(json_response).to include( + "access_token" => new_token.token, + "token_type" => "Bearer", + "refresh_token" => new_token.refresh_token, + ) + expect(new_token).not_to be_uses_dpop + end + + it "issues a new unbound access token if the dpop proof is invalid" do + make_refresh_request(headers: { "HTTP_DPOP" => invalid_dpop_proof }) + + new_token = Doorkeeper::AccessToken.last + expect(json_response).to include( + "access_token" => new_token.token, + "token_type" => "Bearer", + "refresh_token" => new_token.refresh_token, + ) + expect(new_token).not_to be_uses_dpop + end + end + + context "when dpop is required" do + before { config_is_set(:force_dpop, true) } + + it "does not issue a new access token if the dpop proof is missing" do + expect do + make_refresh_request + end.not_to change(Doorkeeper::AccessToken, :count) + + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => "Invalid DPoP proof", + ) + end + + context "when refreshing an access token that uses dpop" do + let!(:token) { super().tap { |it| it.update!(dpop_jkt: jkt) } } + + it "issues a new access token without presenting a dpop proof since it already presents client credentials to refresh" do + make_refresh_request + + new_token = Doorkeeper::AccessToken.last + expect(json_response).to include( + "access_token" => new_token.token, + "token_type" => "DPoP", + "refresh_token" => new_token.refresh_token, + ) + expect(new_token).to be_uses_dpop + expect(new_token.dpop_jkt).to eq(token.dpop_jkt) + end + + it "binds the new access token to the dpop proof's new public key so long as it is valid" do + make_refresh_request(headers: { "HTTP_DPOP" => mismatched_dpop_proof }) + + new_token = Doorkeeper::AccessToken.last + expect(json_response).to include( + "access_token" => new_token.token, + "token_type" => "DPoP", + "refresh_token" => new_token.refresh_token, + ) + expect(new_token).to be_uses_dpop + expect(new_token.dpop_jkt).not_to eq(token.dpop_jkt) + + jkt_from_mismatched_dpop_proof = + JWT::JWK::Thumbprint.new(JWT::JWK.import(JWT.decode(mismatched_dpop_proof, nil, false)[1]["jwk"])).generate + expect(new_token.dpop_jkt).to eq(jkt_from_mismatched_dpop_proof) + end + end + end + end + + context "with a public client" do + def make_refresh_request(headers: {}) + post refresh_token_endpoint_url(client_id: client.uid, refresh_token: token.refresh_token), headers: + end + + let(:client) { FactoryBot.create(:application, confidential: false) } + + it "binds the new access token to the dpop proof's public key if it is valid" do + make_refresh_request(headers: { "HTTP_DPOP" => valid_dpop_proof }) + + new_token = Doorkeeper::AccessToken.last + expect(json_response).to include( + "access_token" => new_token.token, + "token_type" => "DPoP", + "refresh_token" => new_token.refresh_token, + ) + expect(new_token).to be_uses_dpop + expect(new_token.dpop_jkt).to eq(jkt) + end + + it "does not issue a new access token if dpop proof's is invalid" do + expect do + make_refresh_request(headers: { "HTTP_DPOP" => invalid_dpop_proof }) + end.not_to change(Doorkeeper::AccessToken, :count) + + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => "Invalid DPoP proof", + ) + end + + context "when refreshing an access token that uses dpop" do + let!(:token) { super().tap { |it| it.update!(dpop_jkt: jkt) } } + + it "does not issue a new access token if dpop proof is missing" do + expect do + make_refresh_request + end.not_to change(Doorkeeper::AccessToken, :count) + + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => "Invalid DPoP proof", + ) + end + + it "does not issue a new access token if the public keys are mismatched" do + expect do + make_refresh_request(headers: { "HTTP_DPOP" => mismatched_dpop_proof }) + end.not_to change(Doorkeeper::AccessToken, :count) + + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => "Invalid DPoP proof", + ) + end + end + + context "when dpop is not supported" do + before { allow(Doorkeeper::AccessToken).to receive(:dpop_supported?).and_return(false) } + + it "issues a new unbound access token if the dpop proof is valid" do + make_refresh_request(headers: { "HTTP_DPOP" => valid_dpop_proof }) + + new_token = Doorkeeper::AccessToken.last + expect(json_response).to include( + "access_token" => new_token.token, + "token_type" => "Bearer", + "refresh_token" => new_token.refresh_token, + ) + expect(new_token).not_to be_uses_dpop + end + + it "issues a new unbound access token if the dpop proof is invalid" do + make_refresh_request(headers: { "HTTP_DPOP" => invalid_dpop_proof }) + + new_token = Doorkeeper::AccessToken.last + expect(json_response).to include( + "access_token" => new_token.token, + "token_type" => "Bearer", + "refresh_token" => new_token.refresh_token, + ) + expect(new_token).not_to be_uses_dpop + end + end + + context "when dpop is required" do + before { config_is_set(:force_dpop, true) } + + it "does not issue a new access token if the dpop proof is missing" do + expect do + make_refresh_request + end.not_to change(Doorkeeper::AccessToken, :count) + + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => "Invalid DPoP proof", + ) + end + + context "when refreshing an access token that uses dpop" do + let!(:token) { super().tap { |it| it.update!(dpop_jkt: jkt) } } + + it "does not issue a new access token if the dpop proof is missing" do + expect do + make_refresh_request + end.not_to change(Doorkeeper::AccessToken, :count) + + expect(json_response).to match( + "error" => "invalid_dpop_proof", + "error_description" => "Invalid DPoP proof", + ) + end + end + end + end + end end diff --git a/spec/support/helpers/dpop_helper.rb b/spec/support/helpers/dpop_helper.rb new file mode 100644 index 000000000..edd8a7314 --- /dev/null +++ b/spec/support/helpers/dpop_helper.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require "jwt" + +module DPoPHelper + def build_dpop_proof(htm:, htu:, ath: nil, signing_key: OpenSSL::PKey::EC.generate("prime256v1")) + claims = { "jti" => "jti_01", "iat" => Time.current.to_i, "ath" => ath, "htm" => htm, "htu" => htu }.compact + + headers = { "typ" => "dpop+jwt", "alg" => "ES256", "jwk" => JWT::JWK.new(signing_key).export } + + JWT.encode(claims, signing_key, headers["alg"], headers) + end + + def dpop_proof_double(messages = {}) + blank = messages.fetch(:blank?, false) + valid = messages.fetch(:valid?, !blank) + + raise ArgumentError, "can't be blank? && valid?" if blank && valid + + instance_double(Doorkeeper::OAuth::DPoPProof, blank?: blank) + .tap { |it| allow(it).to receive(:valid?) { it.present? && valid } } + .tap { |it| allow(it).to receive(:jkt) { !it.valid? ? nil : "jkt_123" } } + end +end + +RSpec.configuration.send :include, DPoPHelper diff --git a/spec/support/helpers/request_spec_helper.rb b/spec/support/helpers/request_spec_helper.rb index 0b5769123..b8cd81578 100644 --- a/spec/support/helpers/request_spec_helper.rb +++ b/spec/support/helpers/request_spec_helper.rb @@ -49,6 +49,10 @@ def with_access_token_header(token) with_header "Authorization", "Bearer #{token}" end + def with_dpop_proof_header(**kwargs) + with_header "DPoP", build_dpop_proof(**kwargs) + end + def with_header(header, value) page.driver.header(header, value) end diff --git a/spec/support/shared/controllers_shared_context.rb b/spec/support/shared/controllers_shared_context.rb index 19b1bed2d..5216ce95f 100644 --- a/spec/support/shared/controllers_shared_context.rb +++ b/spec/support/shared/controllers_shared_context.rb @@ -8,6 +8,7 @@ Doorkeeper::AccessToken, accessible?: true, includes_scope?: true, acceptable?: true, previous_refresh_token: "", revoke_previous_refresh_token!: true, + uses_dpop?: false, ) end @@ -27,6 +28,7 @@ accessible?: false, revoked?: false, expired?: false, includes_scope?: false, acceptable?: false, previous_refresh_token: "", revoke_previous_refresh_token!: true, + uses_dpop?: false, ) end @@ -48,6 +50,7 @@ accessible?: false, revoked?: false, expired?: true, includes_scope?: false, acceptable?: false, previous_refresh_token: "", revoke_previous_refresh_token!: true, + uses_dpop?: false, ) end @@ -69,6 +72,7 @@ accessible?: false, revoked?: true, expired?: false, includes_scope?: false, acceptable?: false, previous_refresh_token: "", revoke_previous_refresh_token!: true, + uses_dpop?: false, ) end @@ -89,6 +93,7 @@ Doorkeeper::AccessToken, accessible?: true, includes_scope?: true, acceptable?: false, previous_refresh_token: "", revoke_previous_refresh_token!: true, + uses_dpop?: false, ) end @@ -98,3 +103,15 @@ ).to receive(:by_token).with(token_string).and_return(token) end end + +shared_context "dpop token", token: :dpop do + let(:jkt) { JWT::JWK::Thumbprint.new(JWT::JWK.new(signing_key)).generate } + let(:signing_key) { OpenSSL::PKey::EC.generate("prime256v1") } + + let(:token_string) { "1A2B3C4D" } + let(:token) { FactoryBot.create(:access_token, token: token_string, dpop_jkt: jkt) } + + before do + allow(Doorkeeper::AccessToken).to receive(:by_token).with(token_string).and_return(token) + end +end diff --git a/spec/support/shared/requests_shared_examples.rb b/spec/support/shared/requests_shared_examples.rb new file mode 100644 index 000000000..55ac8576b --- /dev/null +++ b/spec/support/shared/requests_shared_examples.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +shared_examples "sender-constraining access_token using dpop" do |when_bearer_token_expected: nil, when_dpop_token_expected: nil| + context "when using dpop" do + context "when the dpop proof is valid" do + let(:dpop_proof) { dpop_proof_double } + + it "issues a dpop token" do + instance_exec(&when_dpop_token_expected) if when_dpop_token_expected + request.authorize + + next if when_dpop_token_expected + + issued_token = Doorkeeper::AccessToken.last + expect(issued_token).to be_uses_dpop + expect(issued_token.token_type).to eq("DPoP") + expect(issued_token.dpop_jkt).to eq("jkt_123") + end + end + + context "when the dpop proof is invalid" do + let(:dpop_proof) { dpop_proof_double(valid?: false) } + + it "invalidates the request" do + request.validate + expect(request.error).to eq(Doorkeeper::Errors::InvalidDPoPProof) + end + end + + context "when dpop is not supported" do + before { allow(Doorkeeper::AccessToken).to receive(:dpop_supported?).and_return(false) } + + context "when the dpop proof is valid" do + let(:dpop_proof) { dpop_proof_double } + + it "issues a Bearer token" do + instance_exec(&when_bearer_token_expected) if when_bearer_token_expected + request.authorize + + next if when_bearer_token_expected + + issued_token = Doorkeeper::AccessToken.last + expect(issued_token).not_to be_uses_dpop + expect(issued_token.token_type).to eq("Bearer") + end + end + + context "when the dpop proof is invalid" do + let(:dpop_proof) { dpop_proof_double(valid?: false) } + + it "validates the request without trying to validate the would be invalid dpop proof" do + request.validate + expect(request.error).to be_nil + end + end + end + + context "when dpop is required" do + before { config_is_set(:force_dpop, true) } + + context "when the dpop proof is blank" do + let(:dpop_proof) { dpop_proof_double(blank?: true) } + + it "invalidates the request" do + request.validate + expect(request.error).to eq(Doorkeeper::Errors::InvalidDPoPProof) + end + end + end + end +end