Skip to content

Commit

Permalink
Merge pull request #13591 from opf/fix/oauth-remapping-using-case-sen…
Browse files Browse the repository at this point in the history
…sitive-login-matching-49834

use case insensitive login matching for oauth remapping
  • Loading branch information
machisuji committed Aug 31, 2023
2 parents ec6c36b + 0e60756 commit 6b4c8b1
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 19 deletions.
2 changes: 1 addition & 1 deletion app/services/authentication/omniauth_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def find_existing_user
def remap_existing_user
return unless Setting.oauth_allow_remapping_of_existing_users?

User.not_builtin.find_by(login: user_attributes[:login])
User.not_builtin.find_by_login(user_attributes[:login]) # rubocop:disable Rails/DynamicFindBy
end

##
Expand Down
61 changes: 43 additions & 18 deletions spec/services/authentication/omniauth_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@
provider: 'google',
uid: '123545',
info: { name: 'foo',
email: 'foo@bar.com',
email: auth_email,
first_name: 'foo',
last_name: 'bar' }
)
end
let(:auth_email) { 'foo@bar.com' }
let(:controller) { double('Controller', session: session_stub) }
let(:session_stub) { [] }

Expand Down Expand Up @@ -127,27 +128,51 @@

context 'with an active user remapped',
with_settings: { oauth_allow_remapping_of_existing_users?: true } do
let!(:user) { create(:user, identity_url: 'foo', login: 'foo@bar.com') }

it 'does not call register user service and logs in the user' do
allow(Users::RegisterUserService).to receive(:new)
let!(:user) { create(:user, identity_url: 'foo', login: auth_email.downcase) }

shared_examples_for 'a successful remapping of foo' do
before do
allow(Users::RegisterUserService).to receive(:new)
allow(OpenProject::OmniAuth::Authorization)
.to(receive(:after_login!))
.with(user, auth_hash, instance)
end

expect(OpenProject::OmniAuth::Authorization)
.to(receive(:after_login!))
.with(user, auth_hash, instance)
it 'does not call register user service and logs in the user' do
aggregate_failures 'Service call' do
expect(call).to be_success
expect(call.result).to eq user
expect(call.result.firstname).to eq 'foo'
expect(call.result.lastname).to eq 'bar'
expect(call.result.login).to eq auth_email
expect(call.result.mail).to eq auth_email
end

user.reload

aggregate_failures 'User attributes' do
expect(user.firstname).to eq 'foo'
expect(user.lastname).to eq 'bar'
expect(user.identity_url).to eq 'google:123545'
end

aggregate_failures 'Message expectations' do
expect(OpenProject::OmniAuth::Authorization)
.to(have_received(:after_login!))

expect(Users::RegisterUserService).not_to have_received(:new)
end
end
end

expect(call).to be_success
expect(call.result).to eq user
expect(call.result.firstname).to eq 'foo'
expect(call.result.lastname).to eq 'bar'
expect(call.result.mail).to eq 'foo@bar.com'
context 'with an all lower case login on the IdP side' do
it_behaves_like 'a successful remapping of foo'
end

user.reload
expect(user.firstname).to eq 'foo'
expect(user.lastname).to eq 'bar'
expect(user.identity_url).to eq 'google:123545'
context 'with a partially upper case login on the IdP side' do
let(:auth_email) { 'FoO@Bar.COM' }

expect(Users::RegisterUserService).not_to have_received(:new)
it_behaves_like 'a successful remapping of foo'
end
end

Expand Down

0 comments on commit 6b4c8b1

Please sign in to comment.