Skip to content

Commit

Permalink
Merge pull request #1762 from gkemmey/allow-invalid-client-when-force…
Browse files Browse the repository at this point in the history
…-pkce

Allow missing client to trigger invalid client error when force_pkce is enabled
  • Loading branch information
nbulaj authored Jan 31, 2025
2 parents 884e648 + bc3310f commit ecf943d
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ User-visible changes worth mentioning.
Add your entry here.
- [#1755] Fix the error message for force_pkce
- [#1761] Memoize authentication failure
- [#1762] Allow missing client to trigger invalid client error when force_pkce is enabled

## 5.8.1

Expand Down
6 changes: 1 addition & 5 deletions lib/doorkeeper/oauth/authorization_code_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,11 @@ 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?
elsif client && !client.confidential && Doorkeeper.config.force_pkce? && code_verifier.blank?
:code_verifier
elsif redirect_uri.blank?
:redirect_uri
Expand Down
8 changes: 8 additions & 0 deletions spec/lib/oauth/authorization_code_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,14 @@
end.not_to change { client.reload.access_tokens.count }
end
end

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.validate
expect(request.error).to eq(Doorkeeper::Errors::InvalidClient)
end
end
end

context "when PKCE is supported" do
Expand Down

0 comments on commit ecf943d

Please sign in to comment.