diff --git a/lib/doorkeeper/oauth/authorization_code_request.rb b/lib/doorkeeper/oauth/authorization_code_request.rb index cb9905eff..d70786c5f 100644 --- a/lib/doorkeeper/oauth/authorization_code_request.rb +++ b/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 diff --git a/lib/doorkeeper/oauth/pre_authorization.rb b/lib/doorkeeper/oauth/pre_authorization.rb index e222e9404..ecead03ac 100644 --- a/lib/doorkeeper/oauth/pre_authorization.rb +++ b/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 diff --git a/spec/lib/oauth/authorization_code_request_spec.rb b/spec/lib/oauth/authorization_code_request_spec.rb index 306d71c32..f0618b7d1 100644 --- a/spec/lib/oauth/authorization_code_request_spec.rb +++ b/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 diff --git a/spec/lib/oauth/pre_authorization_spec.rb b/spec/lib/oauth/pre_authorization_spec.rb index 79ca1ab87..a82a69ffb 100644 --- a/spec/lib/oauth/pre_authorization_spec.rb +++ b/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