From ddc0ede949f40e745ed51c49f96826da6adb165f Mon Sep 17 00:00:00 2001 From: Linh Dang Date: Mon, 25 May 2020 23:59:16 +0900 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Markus Koller --- CHANGELOG.md | 2 +- README.md | 2 +- lib/doorkeeper/openid_connect/errors.rb | 1 - lib/doorkeeper/openid_connect/helpers/controller.rb | 6 +++--- .../doorkeeper/openid_connect/templates/initializer.rb | 8 ++++---- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6366315..7883512 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ - [#114] Fix user_info endpoint when used in api mode - [#112] Add grant_types_supported to discovery response -- [#111] Add configuration callbacks `select_account_for_resource_owner` to support param prompt=select_account +- [#111] Add configuration callback `select_account_for_resource_owner` to support the `prompt=select_account` param ## v1.7.2 (2020-05-20) diff --git a/README.md b/README.md index 0dcc3ce..0dc58ed 100644 --- a/README.md +++ b/README.md @@ -140,7 +140,7 @@ The following settings are optional, but recommended for better client compatibi - Required to support the `max_age` and `prompt=login` parameters. - The block is executed in the controller's scope, so you have access to methods like `params`, `redirect_to` etc. - `select_account_for_resource_owner` - - Defines how to trigger account selecting to choose the current login user + - Defines how to trigger account selection to choose the current login user. - Required to support the `prompt=select_account` parameter. - The block is executed in the controller's scope, so you have access to methods like `params`, `redirect_to` etc. diff --git a/lib/doorkeeper/openid_connect/errors.rb b/lib/doorkeeper/openid_connect/errors.rb index 9dd7035..ca7823b 100644 --- a/lib/doorkeeper/openid_connect/errors.rb +++ b/lib/doorkeeper/openid_connect/errors.rb @@ -26,7 +26,6 @@ class InvalidRequest < OpenidConnectError; end class LoginRequired < OpenidConnectError; end class ConsentRequired < OpenidConnectError; end class InteractionRequired < OpenidConnectError; end - class AccountSelectionRequired < OpenidConnectError; end end end end diff --git a/lib/doorkeeper/openid_connect/helpers/controller.rb b/lib/doorkeeper/openid_connect/helpers/controller.rb index b7aaff6..4cbc79f 100644 --- a/lib/doorkeeper/openid_connect/helpers/controller.rb +++ b/lib/doorkeeper/openid_connect/helpers/controller.rb @@ -96,7 +96,7 @@ def handle_oidc_max_age_param!(owner) end end - def return_without_prompt_param(prompt_value) + def return_without_oidc_prompt_param(prompt_value) return_to = URI.parse(request.path) return_to.query = request.query_parameters.tap do |params| params['prompt'] = params['prompt'].to_s.sub(/\b#{prompt_value}\s*\b/, '').strip @@ -106,7 +106,7 @@ def return_without_prompt_param(prompt_value) end def reauthenticate_oidc_resource_owner(owner) - return_to = return_without_prompt_param('login') + return_to = return_without_oidc_prompt_param('login') instance_exec( owner, @@ -122,7 +122,7 @@ def oidc_consent_required? end def select_account_for_oidc_resource_owner(owner) - return_to = return_without_prompt_param('select_account') + return_to = return_without_oidc_prompt_param('select_account') instance_exec( owner, diff --git a/lib/generators/doorkeeper/openid_connect/templates/initializer.rb b/lib/generators/doorkeeper/openid_connect/templates/initializer.rb index 409b9fe..5a6f8db 100644 --- a/lib/generators/doorkeeper/openid_connect/templates/initializer.rb +++ b/lib/generators/doorkeeper/openid_connect/templates/initializer.rb @@ -28,10 +28,10 @@ # redirect_to new_user_session_url end - # Depend on the configuration, the DoubleRenderError could be occurred - # because of render/redirect is called at some configuration before this configure is called. - # To avoid DoubleRenderError, you could adding these two lines at the beginning - # of this configuration: (Reference: https://github.com/rails/rails/issues/25106) + # Depending on your configuration, a DoubleRenderError could be raised + # if render/redirect_to is called at some point before this callback is executed. + # To avoid the DoubleRenderError, you could add these two lines at the beginning + # of this callback: (Reference: https://github.com/rails/rails/issues/25106) # self.response_body = nil # @_response_body = nil select_account_for_resource_owner do |resource_owner, return_to|