From 208f616c3fb6417ec0a71bee321674c75c0ac90e Mon Sep 17 00:00:00 2001 From: lurz Date: Wed, 18 Dec 2024 21:30:03 -0800 Subject: [PATCH 1/2] Fix the error message for force_pkce Bug When the `force_pkce` option is enabled for a public client, and the authorization code request doesn't include a code_challenge, the error message returned is "The authorization server encountered an unexpected condition which prevented it from fulfilling the request." Root cause PR https://github.com/doorkeeper-gem/doorkeeper/pull/1705 didn't add the error message in the locales. Fix Base on RFC https://datatracker.ietf.org/doc/html/rfc7636#section-4.4.1, we should return an "invalid_request" error with a proper description. I added a new entry for the error message and included a test to verify the proper message is returned. --- config/locales/en.yml | 1 + lib/doorkeeper/errors.rb | 1 - lib/doorkeeper/oauth/pre_authorization.rb | 10 +++++++--- spec/lib/oauth/pre_authorization_spec.rb | 1 + 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index b3c4b2c66..c9190526f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -96,6 +96,7 @@ en: unknown: 'The request is missing a required parameter, includes an unsupported parameter value, or is otherwise malformed.' missing_param: 'Missing required parameter: %{value}.' request_not_authorized: 'Request need to be authorized. Required parameter for authorizing request is missing or invalid.' + invalid_code_challenge: 'Code challenge is required.' invalid_redirect_uri: "The requested redirect uri is malformed or doesn't match client redirect URI." unauthorized_client: 'The client is not authorized to perform this request using this method.' access_denied: 'The resource owner or authorization server denied the request.' diff --git a/lib/doorkeeper/errors.rb b/lib/doorkeeper/errors.rb index 4f925c41f..15f39ef95 100644 --- a/lib/doorkeeper/errors.rb +++ b/lib/doorkeeper/errors.rb @@ -68,7 +68,6 @@ def self.translate_options InvalidClient = Class.new(BaseResponseError) InvalidScope = Class.new(BaseResponseError) InvalidRedirectUri = Class.new(BaseResponseError) - InvalidCodeChallenge = Class.new(BaseResponseError) InvalidGrant = Class.new(BaseResponseError) UnauthorizedClient = Class.new(BaseResponseError) diff --git a/lib/doorkeeper/oauth/pre_authorization.rb b/lib/doorkeeper/oauth/pre_authorization.rb index 3c2dab201..e222e9404 100644 --- a/lib/doorkeeper/oauth/pre_authorization.rb +++ b/lib/doorkeeper/oauth/pre_authorization.rb @@ -14,12 +14,13 @@ 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, error: Errors::InvalidRequest validate :code_challenge_method, error: Errors::InvalidCodeChallengeMethod attr_reader :client, :code_challenge, :code_challenge_method, :missing_param, :redirect_uri, :resource_owner, :response_type, :state, - :authorization_response_flow, :response_mode, :custom_access_token_attributes + :authorization_response_flow, :response_mode, :custom_access_token_attributes, + :invalid_request_reason def initialize(server, parameters = {}, resource_owner = nil) @server = server @@ -147,7 +148,10 @@ def validate_scopes def validate_code_challenge return true unless Doorkeeper.config.force_pkce? return true if client.confidential - code_challenge.present? + return true if code_challenge.present? + + @invalid_request_reason = :invalid_code_challenge + false end def validate_code_challenge_method diff --git a/spec/lib/oauth/pre_authorization_spec.rb b/spec/lib/oauth/pre_authorization_spec.rb index 0baa6dd00..79ca1ab87 100644 --- a/spec/lib/oauth/pre_authorization_spec.rb +++ b/spec/lib/oauth/pre_authorization_spec.rb @@ -427,6 +427,7 @@ attributes[:code_challenge] = " " expect(pre_auth).not_to 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 From b3d54e9b568cbcef86535e2d012dcd5b9aa7744c Mon Sep 17 00:00:00 2001 From: lurz Date: Wed, 18 Dec 2024 21:40:20 -0800 Subject: [PATCH 2/2] Add CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15e53116e..2386e32ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ User-visible changes worth mentioning. ## main Add your entry here. +- [#1755] Fix the error message for force_pkce ## 5.8.1