Skip to content

Commit

Permalink
Add configuration callbacks select_account_for_resource_owner to su…
Browse files Browse the repository at this point in the history
…pport param prompt=select_account
  • Loading branch information
linhdangduy committed May 25, 2020
1 parent 3930ae6 commit 6e500d9
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
4 changes: 4 additions & 0 deletions lib/doorkeeper/openid_connect/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}
Expand Down
24 changes: 19 additions & 5 deletions lib/doorkeeper/openid_connect/helpers/controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
)

Expand All @@ -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
Expand Down
12 changes: 12 additions & 0 deletions lib/generators/doorkeeper/openid_connect/templates/initializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 38 additions & 8 deletions spec/controllers/doorkeeper/authorizations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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'
Expand Down
4 changes: 4 additions & 0 deletions spec/dummy/config/initializers/doorkeeper_openid_connect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions spec/dummy/config/locales/doorkeeper_openid_connect.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
18 changes: 18 additions & 0 deletions spec/lib/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down

0 comments on commit 6e500d9

Please sign in to comment.