Merge branch 'doorkeeper-gem:main' into kmayer-api-4415

.github/workflows/ci.yml

@@ -24,7 +24,6 @@ jobs:
           - '3.0'
           - '3.1'
           - '3.2'
-          - truffleruby-head
         gemfile:
           - gemfiles/rails_6_0.gemfile
           - gemfiles/rails_6_1.gemfile

CHANGELOG.md

@@ -10,6 +10,14 @@ User-visible changes worth mentioning.
 Add your entry here.
 - [#????] DRY out AuthorizationsController#render_success
 
+- [#1715] Fix token introspection invalid request reason
+- [#1714] Fix `Doorkeeper::AccessToken.find_or_create_for` with empty scopes which raises NoMethodError
+- [#1712] Add `Pragma: no-cache` to token response
+
+## 5.7.1
+
+- [#1705] Add `force_pkce` option that requires non-confidential clients to use PKCE when requesting an access_token using an authorization code
+
 ## 5.7.0
 
 - [#1696] Add missing `#issued_token` method to `OAuth::TokenResponse`

Gemfile

@@ -23,7 +23,7 @@ gem "rubocop-rspec", require: false
 gem "bcrypt", "~> 3.1", require: false
 
 gem "activerecord-jdbcsqlite3-adapter", platform: :jruby
-gem "sqlite3", "~> 1.4", platform: %i[ruby mswin mingw x64_mingw]
+gem "sqlite3", "~> 2.0", platform: %i[ruby mswin mingw x64_mingw]
 
 gem "tzinfo-data", platforms: %i[mingw mswin x64_mingw]
 gem "timecop"

lib/doorkeeper/config.rb

@@ -113,6 +113,12 @@ def revoke_previous_authorization_code_token
         @config.instance_variable_set(:@revoke_previous_authorization_code_token, true)
       end
 
+      # Require non-confidential apps to use PKCE (send a code_verifier) when requesting
+      # an access_token using an authorization code (disabled by default)
+      def force_pkce
+        @config.instance_variable_set(:@force_pkce, true)
+      end
+
       # Use an API mode for applications generated with --api argument
       # It will skip applications controller, disable forgery protection
       def api_only
@@ -492,6 +498,10 @@ def revoke_previous_authorization_code_token?
       option_set? :revoke_previous_authorization_code_token
     end
 
+    def force_pkce?
+      option_set? :force_pkce
+    end
+
     def enforce_configured_scopes?
       option_set? :enforce_configured_scopes
     end

lib/doorkeeper/errors.rb

@@ -54,6 +54,7 @@ def self.name_for_response
     InvalidClient = Class.new(BaseResponseError)
     InvalidScope = Class.new(BaseResponseError)
     InvalidRedirectUri = Class.new(BaseResponseError)
+    InvalidCodeChallenge = Class.new(BaseResponseError)
     InvalidCodeChallengeMethod = Class.new(BaseResponseError)
     InvalidGrant = Class.new(BaseResponseError)
 

lib/doorkeeper/models/access_token_mixin.rb

@@ -214,6 +214,8 @@ def custom_attributes_match?(token, custom_attributes)
       # @return [Doorkeeper::AccessToken] existing record or a new one
       #
       def find_or_create_for(application:, resource_owner:, scopes:, **token_attributes)
+        scopes = Doorkeeper::OAuth::Scopes.from_string(scopes) if scopes.is_a?(String)
+
         if Doorkeeper.config.reuse_access_token
           custom_attributes = extract_custom_attributes(token_attributes).presence
           access_token = matching_token_for(

lib/doorkeeper/oauth/authorization_code_request.rb

@@ -59,10 +59,16 @@ def pkce_supported?
         Doorkeeper.config.access_grant_model.pkce_supported?
       end
 
+      def confidential?
+        client&.confidential
+      end
+
       def validate_params
         @missing_param =
           if grant&.uses_pkce? && code_verifier.blank?
             :code_verifier
+          elsif !confidential? && Doorkeeper.config.force_pkce? && code_verifier.blank?
+            :code_verifier
           elsif redirect_uri.blank?
             :redirect_uri
           end

lib/doorkeeper/oauth/client.rb

@@ -5,7 +5,7 @@ module OAuth
     class Client
       attr_reader :application
 
-      delegate :id, :name, :uid, :redirect_uri, :scopes, to: :@application
+      delegate :id, :name, :uid, :redirect_uri, :scopes, :confidential, to: :@application
 
       def initialize(application)
         @application = application

lib/doorkeeper/oauth/pre_authorization.rb

@@ -14,6 +14,7 @@ class PreAuthorization
       validate :response_type, error: Errors::UnsupportedResponseType
       validate :response_mode, error: Errors::UnsupportedResponseMode
       validate :scopes, error: Errors::InvalidScope
+      validate :code_challenge, error: Errors::InvalidCodeChallenge
       validate :code_challenge_method, error: Errors::InvalidCodeChallengeMethod
 
       attr_reader :client, :code_challenge, :code_challenge_method, :missing_param,
@@ -143,6 +144,12 @@ def validate_scopes
         )
       end
 
+      def validate_code_challenge
+        return true unless Doorkeeper.config.force_pkce?
+        return true if client.confidential
+        code_challenge.present?
+      end
+
       def validate_code_challenge_method
         return true unless Doorkeeper.config.access_grant_model.pkce_supported?
 

lib/doorkeeper/oauth/token_introspection.rb

@@ -6,7 +6,7 @@ module OAuth
     #
     # @see https://datatracker.ietf.org/doc/html/rfc7662
     class TokenIntrospection
-      attr_reader :error
+      attr_reader :error, :invalid_request_reason
 
       def initialize(server, token)
         @server = server
@@ -38,7 +38,6 @@ def to_json(*)
       private
 
       attr_reader :server, :token
-      attr_reader :invalid_request_reason
 
       # If the protected resource uses OAuth 2.0 client credentials to
       # authenticate to the introspection endpoint and its credentials are

lib/doorkeeper/oauth/token_response.rb

@@ -30,6 +30,7 @@ def headers
         {
           "Cache-Control" => "no-store, no-cache",
           "Content-Type" => "application/json; charset=utf-8",
+          "Pragma" => "no-cache",
         }
       end
     end

lib/doorkeeper/version.rb

@@ -5,7 +5,7 @@ module VERSION
     # Semantic versioning
     MAJOR = 5
     MINOR = 7
-    TINY = 0
+    TINY = 1
     PRE = nil
 
     # Full version number

lib/generators/doorkeeper/templates/initializer.rb

@@ -173,6 +173,11 @@
   #
   # revoke_previous_authorization_code_token
 
+  # Require non-confidential clients to use PKCE when using an authorization code
+  # to obtain an access_token (disabled by default)
+  #
+  # force_pkce
+
   # Hash access and refresh tokens before persisting them.
   # This will disable the possibility to use +reuse_access_token+
   # since plain values can no longer be retrieved.

spec/controllers/tokens_controller_spec.rb

@@ -536,7 +536,7 @@
 
         expect(json_response).to match(
           "error" => "invalid_request",
-          "error_description" => an_instance_of(String),
+          "error_description" => I18n.t("doorkeeper.errors.messages.invalid_request.request_not_authorized"),
         )
       end
     end

spec/lib/oauth/authorization_code_request_spec.rb

@@ -170,6 +170,32 @@
   end
 
   context "when using PKCE params" do
+    context "when force_pkce is enabled" do
+      before do
+        allow_any_instance_of(Doorkeeper::Config).to receive(:force_pkce?).and_return(true)
+      end
+
+      context "when the app is confidential" do
+        it "issues a new token for the client" do
+          expect do
+            request.authorize
+          end.to change { client.reload.access_tokens.count }.by(1)
+        end
+      end
+
+      context "when the app is not confidential" do
+        before do
+          client.update(confidential: false)
+        end
+
+        it "does not issue a token" do
+          expect do
+            request.authorize
+          end.not_to change { client.reload.access_tokens.count }
+        end
+      end
+    end
+
     context "when PKCE is supported" do
       before do
         allow(Doorkeeper::AccessGrant).to receive(:pkce_supported?).and_return(true)

spec/lib/oauth/pre_authorization_spec.rb

@@ -40,6 +40,7 @@
       response_type
       response_mode
       scopes
+      code_challenge
       code_challenge_method
     ])
   end
@@ -343,5 +344,49 @@
         expect(pre_auth).to be_authorizable
       end
     end
+
+    context "when force_pkce is enabled" do
+      before do
+        allow_any_instance_of(Doorkeeper::Config).to receive(:force_pkce?).and_return(true)
+      end
+
+      context "when the app is confidential" do
+        before do
+          application.update(confidential: true)
+        end
+
+        it "accepts a blank code_challenge" do
+          attributes[:code_challenge] = " "
+
+          expect(pre_auth).to be_authorizable
+        end
+
+        it "accepts a code challenge" do
+          attributes[:code_challenge] = "a45a9fea-0676-477e-95b1-a40f72ac3cfb"
+          attributes[:code_challenge_method] = "plain"
+
+          expect(pre_auth).to be_authorizable
+        end
+      end
+
+      context "when the app is not confidential" do
+        before do
+          application.update(confidential: false)
+        end
+
+        it "does not accept a blank code_challenge" do
+          attributes[:code_challenge] = " "
+
+          expect(pre_auth).not_to be_authorizable
+        end
+
+        it "accepts a code challenge" do
+          attributes[:code_challenge] = "a45a9fea-0676-477e-95b1-a40f72ac3cfb"
+          attributes[:code_challenge_method] = "plain"
+
+          expect(pre_auth).to be_authorizable
+        end
+      end
+    end
   end
 end

spec/lib/oauth/token_response_spec.rb

@@ -8,6 +8,7 @@
   it "includes access token response headers" do
     headers = response.headers
     expect(headers.fetch("Cache-Control")).to eq("no-store, no-cache")
+    expect(headers.fetch("Pragma")).to eq("no-cache")
   end
 
   it "status is ok" do

spec/requests/endpoints/token_spec.rb

@@ -17,6 +17,7 @@
 
     expect(headers["Cache-Control"]).to be_in(["no-store", "no-cache, no-store", "private, no-store"])
     expect(headers["Content-Type"]).to eq("application/json; charset=utf-8")
+    expect(headers["Pragma"]).to eq("no-cache")
   end
 
   it "accepts client credentials with basic auth header" do