Skip to content

Commit

Permalink
update: using response_mode which supported by doorkeeper v5.5
Browse files Browse the repository at this point in the history
  • Loading branch information
linhdangduy committed Mar 21, 2021
1 parent 0b681cb commit d07ba31
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 122 deletions.
1 change: 0 additions & 1 deletion lib/doorkeeper/openid_connect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
require 'doorkeeper/openid_connect/id_token'
require 'doorkeeper/openid_connect/id_token_token'
require 'doorkeeper/openid_connect/user_info'
require 'doorkeeper/openid_connect/response_mode'
require 'doorkeeper/openid_connect/version'

require 'doorkeeper/openid_connect/helpers/controller'
Expand Down
13 changes: 7 additions & 6 deletions lib/doorkeeper/openid_connect/helpers/controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,16 @@ def handle_oidc_error!(exception)
redirect_uri: params[:redirect_uri],
response_on_fragment: pre_auth.response_on_fragment?,
)
end
end

response.headers.merge!(error_response.headers)

if error_response.redirectable?
render json: error_response.body, status: :found, location: error_response.redirect_uri
else
render json: error_response.body, status: error_response.status
end
# NOTE: Assign error_response to @authorize_response then use redirect_or_render method that are defined at
# doorkeeper's authorizations_controller.
# - https://github.com/doorkeeper-gem/doorkeeper/blob/v5.5.0/app/controllers/doorkeeper/authorizations_controller.rb#L110
# - https://github.com/doorkeeper-gem/doorkeeper/blob/v5.5.0/app/controllers/doorkeeper/authorizations_controller.rb#L52
@authorize_response = error_response
redirect_or_render(@authorize_response)
end

def handle_oidc_prompt_param!(owner)
Expand Down
18 changes: 8 additions & 10 deletions lib/doorkeeper/openid_connect/oauth/pre_authorization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,16 @@ def initialize(server, attrs = {}, resource_owner = nil)
@nonce = attrs[:nonce]
end

# This method will be updated when doorkeeper move to version > 5.2.2
# TODO: delete this method and refactor response_on_fragment? method (below) when doorkeeper gem version constrains is > 5.2.2
def error_response
if error == :invalid_request
Doorkeeper::OAuth::InvalidRequestResponse.from_request(self, response_on_fragment: response_on_fragment?)
else
Doorkeeper::OAuth::ErrorResponse.from_request(self, response_on_fragment: response_on_fragment?)
# NOTE: Auto get default response_mode of specified response_type if response_mode is not
# yet present. We can delete this method after Doorkeeper's minimize version support it.
def response_on_fragment?
return response_mode == 'fragment' if response_mode.present?

grant_flow = server.authorization_response_flows.detect do |flow|
flow.matches_response_type?(response_type)
end
end

def response_on_fragment?
Doorkeeper::OpenidConnect::ResponseMode.new(response_type).fragment?
grant_flow&.default_response_mode == 'fragment'
end
end
end
Expand Down
30 changes: 0 additions & 30 deletions lib/doorkeeper/openid_connect/response_mode.rb

This file was deleted.

95 changes: 58 additions & 37 deletions spec/controllers/doorkeeper/authorizations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
def authorize!(params = {})
get :new, params: {
response_type: 'code',
response_mode: '',
current_user: user.id,
client_id: application.uid,
scope: default_scopes,
Expand Down Expand Up @@ -120,30 +121,38 @@ def expect_successful_callback!
expect_successful_callback!
end

it 'returns an invalid_request error if another prompt value is present' do
authorize! prompt: 'none login'
context 'when another prompt value is present' do
let(:error_params) do
{
'error' => 'invalid_request',
'error_description' => 'The request is missing a required parameter, includes an unsupported parameter value, or is otherwise malformed.',
}
end
let(:request_param) { { prompt: 'none login' } }

error_params = {
'error' => 'invalid_request',
'error_description' => 'The request is missing a required parameter, includes an unsupported parameter value, or is otherwise malformed.',
}
it 'redirect as the query uri with an invalid_request error' do
authorize! request_param

expect(response.status).to redirect_to build_redirect_uri(error_params)
expect(JSON.parse(response.body)).to eq(error_params)
end
expect(response).to redirect_to build_redirect_uri(error_params)
end

it 'redirect as the fragment style uri when response_type is implicit flow request' do
allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])

it 'uses the fragment style uris when redirecting an invalid request error for implicit flow request' do
allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])
authorize! request_param.merge(response_type: 'id_token token')

authorize! response_type: 'id_token token', prompt: 'none login'
expect(response).to redirect_to build_redirect_uri(error_params, type: 'fragment')
end

error_params = {
'error' => 'invalid_request',
'error_description' => 'The request is missing a required parameter, includes an unsupported parameter value, or is otherwise malformed.',
}
it 'set @authorize_response variable and render form_post template and when the form_post response_mode is specified' do
allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])

authorize! request_param.merge(response_type: 'id_token token', response_mode: 'form_post')

expect(response.status).to redirect_to build_redirect_uri(error_params, type: 'fragment')
expect(JSON.parse(response.body)).to eq(error_params)
authorize_response = controller.instance_variable_get :@authorize_response
expect(authorize_response.body.to_json).to eq(error_params.to_json)
expect(response).to render_template(:form_post)
end
end

context 'when not logged in' do
Expand All @@ -154,21 +163,30 @@ def expect_successful_callback!
'state' => 'somestate',
}
end
let(:request_param) { { current_user: nil } }

it 'returns a login_required error' do
authorize! prompt: 'none', current_user: nil, state: 'somestate'
authorize! request_param.merge(prompt: 'none', state: 'somestate')

expect(response).to redirect_to build_redirect_uri(error_params)
expect(JSON.parse(response.body)).to eq(error_params)
end

it 'uses the fragment style uris when redirecting an error for implicit flow request' do
it 'redirect as the fragment style uri when response_type is implicit flow request' do
allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])

authorize! response_type: 'id_token token', prompt: 'none', current_user: nil, state: 'somestate'
authorize! request_param.merge(response_type: 'id_token token', prompt: 'none', state: 'somestate')

expect(response).to redirect_to build_redirect_uri(error_params, type: 'fragment')
expect(JSON.parse(response.body)).to eq(error_params)
end

it 'set @authorize_response variable and render form_post template and when the form_post response_mode is specified' do
allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])

authorize! request_param.merge(response_type: 'id_token token', response_mode: 'form_post', prompt: 'none', state: 'somestate')

authorize_response = controller.instance_variable_get :@authorize_response
expect(authorize_response.body.to_json).to eq(error_params.to_json)
expect(response).to render_template(:form_post)
end
end
end
Expand All @@ -182,32 +200,37 @@ def expect_successful_callback!
end

context 'when not logged in' do
it 'returns the login_required error when not logged in' do
error_params = {
let(:error_params) do
{
'error' => 'login_required',
'error_description' => 'The authorization server requires end-user authentication',
'state' => 'somestate',
}
end
let(:request_param) { { current_user: nil } }

authorize! prompt: 'none', current_user: nil, state: 'somestate'
it 'returns the login_required error when not logged in' do
authorize! request_param.merge(prompt: 'none', state: 'somestate')

expect(response).to redirect_to build_redirect_uri(error_params)
expect(JSON.parse(response.body)).to eq(error_params)
end

it 'uses the fragment style uris when redirecting an error for implicit flow request' do
allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])

error_params = {
'error' => 'login_required',
'error_description' => 'The authorization server requires end-user authentication',
'state' => 'somestate',
}

authorize! response_type: 'id_token token', prompt: 'none', current_user: nil, state: 'somestate'
authorize! request_param.merge(response_type: 'id_token token', prompt: 'none', state: 'somestate')

expect(response).to redirect_to build_redirect_uri(error_params, type: 'fragment')
expect(JSON.parse(response.body)).to eq(error_params)
end

it 'set @authorize_response variable and render form_post template and when the form_post response_mode is specified' do
allow(Doorkeeper.configuration).to receive(:grant_flows).and_return(['implicit_oidc'])

authorize! request_param.merge(response_type: 'id_token token', response_mode: 'form_post', prompt: 'none', state: 'somestate')

authorize_response = controller.instance_variable_get :@authorize_response
expect(authorize_response.body.to_json).to eq(error_params.to_json)
expect(response).to render_template(:form_post)
end
end

Expand All @@ -220,7 +243,6 @@ def expect_successful_callback!
}

expect(response).to redirect_to build_redirect_uri(error_params)
expect(JSON.parse(response.body)).to eq(error_params)
end
end
end
Expand Down Expand Up @@ -266,7 +288,6 @@ def expect_successful_callback!
}

expect(response).to redirect_to build_redirect_uri(error_params)
expect(JSON.parse(response.body)).to eq(error_params)
end

it 'does not redirect to an invalid redirect_uri' do
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/oauth/pre_authorization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
describe Doorkeeper::OpenidConnect::OAuth::PreAuthorization do
subject { Doorkeeper::OAuth::PreAuthorization.new server, attrs }

let(:server) { double }
let(:server) { Doorkeeper.configuration }
let(:attrs) {}

describe '#initialize' do
Expand Down
37 changes: 0 additions & 37 deletions spec/lib/response_mode_spec.rb

This file was deleted.

0 comments on commit d07ba31

Please sign in to comment.