diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f05400..6366315 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +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 ## v1.7.2 (2020-05-20) diff --git a/README.md b/README.md index 2e0729f..0dcc3ce 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,10 @@ The following settings are optional, but recommended for better client compatibi - Defines how to trigger reauthentication for the current user (e.g. display a password prompt, or sign-out the user and redirect to the login form). - 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 + - 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. The following settings are optional: diff --git a/config/locales/en.yml b/config/locales/en.yml index 96ee438..1bed506 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -19,4 +19,5 @@ en: resource_owner_from_access_token_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.resource_owner_from_access_token missing configuration.' auth_time_from_resource_owner_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.auth_time_from_resource_owner missing configuration.' reauthenticate_resource_owner_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.reauthenticate_resource_owner missing configuration.' + select_account_for_resource_owner_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.select_account_for_resource_owner missing configuration.' subject_not_configured: 'ID Token generation failed due to Doorkeeper::OpenidConnect.configure.subject missing configuration.' diff --git a/lib/doorkeeper/openid_connect/config.rb b/lib/doorkeeper/openid_connect/config.rb index 1198f02..fb04965 100644 --- a/lib/doorkeeper/openid_connect/config.rb +++ b/lib/doorkeeper/openid_connect/config.rb @@ -115,6 +115,10 @@ def extended(base) raise Errors::InvalidConfiguration, I18n.translate('doorkeeper.openid_connect.errors.messages.reauthenticate_resource_owner_not_configured') } + option :select_account_for_resource_owner, default: lambda { |*_| + raise Errors::InvalidConfiguration, I18n.translate('doorkeeper.openid_connect.errors.messages.select_account_for_resource_owner_not_configured') + } + option :subject, default: lambda { |*_| raise Errors::InvalidConfiguration, I18n.translate('doorkeeper.openid_connect.errors.messages.subject_not_configured') } diff --git a/lib/doorkeeper/openid_connect/helpers/controller.rb b/lib/doorkeeper/openid_connect/helpers/controller.rb index c9c7b7a..b7aaff6 100644 --- a/lib/doorkeeper/openid_connect/helpers/controller.rb +++ b/lib/doorkeeper/openid_connect/helpers/controller.rb @@ -75,8 +75,7 @@ def handle_oidc_prompt_param!(owner) when 'consent' render :new when 'select_account' - # TODO: let the user implement this - raise Errors::AccountSelectionRequired + select_account_for_oidc_resource_owner(owner) else raise Errors::InvalidRequest end @@ -97,16 +96,21 @@ def handle_oidc_max_age_param!(owner) end end - def reauthenticate_oidc_resource_owner(owner) + def return_without_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(/\blogin\s*\b/, '').strip + params['prompt'] = params['prompt'].to_s.sub(/\b#{prompt_value}\s*\b/, '').strip params.delete('prompt') if params['prompt'].blank? end.to_query + return_to.to_s + end + + def reauthenticate_oidc_resource_owner(owner) + return_to = return_without_prompt_param('login') instance_exec( owner, - return_to.to_s, + return_to, &Doorkeeper::OpenidConnect.configuration.reauthenticate_resource_owner ) @@ -116,6 +120,16 @@ def reauthenticate_oidc_resource_owner(owner) def oidc_consent_required? !skip_authorization? && !matching_token? end + + def select_account_for_oidc_resource_owner(owner) + return_to = return_without_prompt_param('select_account') + + instance_exec( + owner, + return_to, + &Doorkeeper::OpenidConnect.configuration.select_account_for_resource_owner + ) + end end end end diff --git a/lib/generators/doorkeeper/openid_connect/templates/initializer.rb b/lib/generators/doorkeeper/openid_connect/templates/initializer.rb index 2d9d6c3..409b9fe 100644 --- a/lib/generators/doorkeeper/openid_connect/templates/initializer.rb +++ b/lib/generators/doorkeeper/openid_connect/templates/initializer.rb @@ -28,6 +28,18 @@ # 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) + # self.response_body = nil + # @_response_body = nil + select_account_for_resource_owner do |resource_owner, return_to| + # Example implementation: + # store_location_for resource_owner, return_to + # redirect_to account_select_url + end + subject do |resource_owner, application| # Example implementation: # resource_owner.id diff --git a/spec/controllers/doorkeeper/authorizations_controller_spec.rb b/spec/controllers/doorkeeper/authorizations_controller_spec.rb index 02967b6..4922994 100644 --- a/spec/controllers/doorkeeper/authorizations_controller_spec.rb +++ b/spec/controllers/doorkeeper/authorizations_controller_spec.rb @@ -189,16 +189,10 @@ def expect_successful_callback! end context 'with a prompt=select_account parameter' do - it 'returns an account_selection_required error' do + it 'redirects to the select account screen' do authorize! prompt: 'select_account' - error_params = { - 'error' => 'account_selection_required', - 'error_description' => 'The authorization server requires end-user account selection', - } - - expect(response.status).to redirect_to build_redirect_uri(error_params) - expect(JSON.parse(response.body)).to eq(error_params) + expect(response).to redirect_to('/select_account') end end @@ -307,6 +301,42 @@ def reauthenticate! end end + describe '#select_account_for_resource_owner' do + before do + allow(subject.request).to receive(:path).and_return('/oauth/authorize') + allow(subject.request).to receive(:query_parameters) { + { client_id: 'foo', prompt: 'login consent select_account' }.with_indifferent_access + } + end + + def select_account! + passed_args = nil + + Doorkeeper::OpenidConnect.configure do + select_account_for_resource_owner do |*args| + passed_args = args + end + end + + subject.send :select_account_for_oidc_resource_owner, user + passed_args + end + + it 'calls select_account_for_resource_owner with the current user and the return path' do + resource_owner, return_to = select_account! + + expect(resource_owner).to eq user + expect(return_to).to eq '/oauth/authorize?client_id=foo&prompt=login+consent' + end + + it 'removes select_account from the prompt parameter and keeps other values' do + _, return_to = select_account! + return_params = Rack::Utils.parse_query(URI.parse(return_to).query) + + expect(return_params['prompt']).to eq 'login consent' + end + end + describe '#pre_auth' do it 'permits nonce parameter' do authorize! nonce: '123456' diff --git a/spec/dummy/config/initializers/doorkeeper_openid_connect.rb b/spec/dummy/config/initializers/doorkeeper_openid_connect.rb index 7e30f4d..a381972 100644 --- a/spec/dummy/config/initializers/doorkeeper_openid_connect.rb +++ b/spec/dummy/config/initializers/doorkeeper_openid_connect.rb @@ -47,6 +47,10 @@ redirect_to '/reauthenticate' end + select_account_for_resource_owner do |_resource_owner, _return_to| + redirect_to '/select_account' + end + subject do |resource_owner| resource_owner.id end diff --git a/spec/dummy/config/locales/doorkeeper_openid_connect.en.yml b/spec/dummy/config/locales/doorkeeper_openid_connect.en.yml index 96ee438..1bed506 100644 --- a/spec/dummy/config/locales/doorkeeper_openid_connect.en.yml +++ b/spec/dummy/config/locales/doorkeeper_openid_connect.en.yml @@ -19,4 +19,5 @@ en: resource_owner_from_access_token_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.resource_owner_from_access_token missing configuration.' auth_time_from_resource_owner_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.auth_time_from_resource_owner missing configuration.' reauthenticate_resource_owner_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.reauthenticate_resource_owner missing configuration.' + select_account_for_resource_owner_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.select_account_for_resource_owner missing configuration.' subject_not_configured: 'ID Token generation failed due to Doorkeeper::OpenidConnect.configure.subject missing configuration.' diff --git a/spec/lib/config_spec.rb b/spec/lib/config_spec.rb index 2e39d9d..067b9ab 100644 --- a/spec/lib/config_spec.rb +++ b/spec/lib/config_spec.rb @@ -105,6 +105,24 @@ end end + describe 'select_account_for_resource_owner' do + it 'sets the block that is accessible via select_account_for_resource_owner' do + block = proc {} + described_class.configure do + select_account_for_resource_owner(&block) + end + expect(subject.select_account_for_resource_owner).to eq(block) + end + + it 'fails if unset' do + described_class.configure {} + + expect do + subject.select_account_for_resource_owner.call + end.to raise_error Doorkeeper::OpenidConnect::Errors::InvalidConfiguration + end + end + describe 'subject' do it 'sets the block that is accessible via subject' do block = proc {}