Ensure force PKCE always forces PKCE, fixes #1654

lib/doorkeeper/oauth/authorization_code_request.rb

@@ -63,7 +63,7 @@ def validate_params
         @missing_param =
           if grant&.uses_pkce? && code_verifier.blank?
             :code_verifier
-          elsif client && !client.confidential && Doorkeeper.config.force_pkce? && code_verifier.blank?
+          elsif Doorkeeper.config.force_pkce? && code_verifier.blank?
             :code_verifier
           elsif redirect_uri.blank?
             :redirect_uri

lib/doorkeeper/oauth/pre_authorization.rb

@@ -147,7 +147,6 @@ def validate_scopes
 
       def validate_code_challenge
         return true unless Doorkeeper.config.force_pkce?
-        return true if client.confidential
         return true if code_challenge.present?
 
         @invalid_request_reason = :invalid_code_challenge

spec/lib/oauth/authorization_code_request_spec.rb

@@ -176,10 +176,10 @@
       end
 
       context "when the app is confidential" do
-        it "issues a new token for the client" do
+        it "does not issue a new token" do
           expect do
             request.authorize
-          end.to change { client.reload.access_tokens.count }.by(1)
+          end.not_to change { client.reload.access_tokens.count }
         end
       end
 
@@ -196,10 +196,10 @@
       end
 
       context "when the app is missing" do
-        it "does not assume non-confidential and forcibly validate pkce params" do
+        it "forcibly validate pkce params" do
           request = described_class.new(server, grant, nil, params)
           request.validate
-          expect(request.error).to eq(Doorkeeper::Errors::InvalidClient)
+          expect(request.error).to eq(Doorkeeper::Errors::InvalidRequest)
         end
       end
     end

spec/lib/oauth/pre_authorization_spec.rb

@@ -404,10 +404,11 @@
           application.update(confidential: true)
         end
 
-        it "accepts a blank code_challenge" do
+        it "does not accept a blank code_challenge" do
           attributes[:code_challenge] = " "
 
-          expect(pre_auth).to be_authorizable
+          expect(pre_auth).to_not be_authorizable
+          expect(pre_auth.error_response.description).to eq(translated_invalid_request_error_message(:invalid_code_challenge, nil))
         end
 
         it "accepts a code challenge" do