diff options
| -rw-r--r-- | app/controllers/omniauth_callbacks_controller.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/auth/o_auth/identity_linker.rb | 19 | ||||
| -rw-r--r-- | lib/gitlab/auth/omniauth_identity_linker_base.rb | 34 | ||||
| -rw-r--r-- | lib/gitlab/auth/saml/identity_linker.rb | 19 | ||||
| -rw-r--r-- | spec/lib/gitlab/auth/o_auth/identity_linker_spec.rb | 26 | ||||
| -rw-r--r-- | spec/lib/gitlab/auth/saml/identity_linker_spec.rb | 20 |
6 files changed, 53 insertions, 69 deletions
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index df75693e840..f7e7f04384a 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -80,9 +80,9 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController identity_linker ||= auth_module::IdentityLinker.new(current_user, oauth) - identity_linker.create_or_update + identity_linker.link - if identity_linker.created? + if identity_linker.changed? redirect_identity_linked elsif identity_linker.error_message.present? redirect_identity_link_failed(identity_linker.error_message) diff --git a/lib/gitlab/auth/o_auth/identity_linker.rb b/lib/gitlab/auth/o_auth/identity_linker.rb index 704a7e1b3c2..de92d7a214d 100644 --- a/lib/gitlab/auth/o_auth/identity_linker.rb +++ b/lib/gitlab/auth/o_auth/identity_linker.rb @@ -2,25 +2,6 @@ module Gitlab module Auth module OAuth class IdentityLinker < OmniauthIdentityLinkerBase - def create_or_update - if identity.new_record? - @created = identity.save - end - end - - def error_message - identity.validate - - identity.errors.full_messages.join(', ') - end - - private - - def identity - @identity ||= current_user.identities - .with_extern_uid(oauth['provider'], oauth['uid']) - .first_or_initialize(extern_uid: oauth['uid']) - end end end end diff --git a/lib/gitlab/auth/omniauth_identity_linker_base.rb b/lib/gitlab/auth/omniauth_identity_linker_base.rb index 4ed28d6a8be..ae365fcdfaa 100644 --- a/lib/gitlab/auth/omniauth_identity_linker_base.rb +++ b/lib/gitlab/auth/omniauth_identity_linker_base.rb @@ -6,19 +6,41 @@ module Gitlab def initialize(current_user, oauth) @current_user = current_user @oauth = oauth - @created = false + @changed = false end - def created? - @created + def link + save if identity.new_record? + end + + def changed? + @changed end def error_message - '' + identity.validate + + identity.errors.full_messages.join(', ') + end + + private + + def save + @changed = identity.save + end + + def identity + @identity ||= current_user.identities + .with_extern_uid(provider, uid) + .first_or_initialize(extern_uid: uid) + end + + def provider + oauth['provider'] end - def create_or_update - raise NotImplementedError + def uid + oauth['uid'] end end end diff --git a/lib/gitlab/auth/saml/identity_linker.rb b/lib/gitlab/auth/saml/identity_linker.rb index d5f97f01df3..7e4b191d512 100644 --- a/lib/gitlab/auth/saml/identity_linker.rb +++ b/lib/gitlab/auth/saml/identity_linker.rb @@ -2,25 +2,6 @@ module Gitlab module Auth module Saml class IdentityLinker < OmniauthIdentityLinkerBase - def create_or_update - if find_saml_identity.nil? - create_saml_identity - - @created = true - else - @created = false - end - end - - protected - - def find_saml_identity - current_user.identities.with_extern_uid(:saml, oauth['uid']).take - end - - def create_saml_identity - current_user.identities.create(extern_uid: oauth['uid'], provider: :saml) - end end end end diff --git a/spec/lib/gitlab/auth/o_auth/identity_linker_spec.rb b/spec/lib/gitlab/auth/o_auth/identity_linker_spec.rb index 8d1b0a3cd4b..528f1b4ec57 100644 --- a/spec/lib/gitlab/auth/o_auth/identity_linker_spec.rb +++ b/spec/lib/gitlab/auth/o_auth/identity_linker_spec.rb @@ -12,23 +12,23 @@ describe Gitlab::Auth::OAuth::IdentityLinker do let!(:identity) { user.identities.create!(provider: provider, extern_uid: uid) } it "doesn't create new identity" do - expect { subject.create_or_update }.not_to change { Identity.count } + expect { subject.link }.not_to change { Identity.count } end - it "#created? returns false" do - subject.create_or_update + it "sets #changed? to false" do + subject.link - expect(subject).not_to be_created + expect(subject).not_to be_changed end end context 'identity already linked to different user' do let!(:identity) { create(:identity, provider: provider, extern_uid: uid) } - it "#created? returns false" do - subject.create_or_update + it "#changed? returns false" do + subject.link - expect(subject).not_to be_created + expect(subject).not_to be_changed end it 'exposes error message' do @@ -38,25 +38,25 @@ describe Gitlab::Auth::OAuth::IdentityLinker do context 'identity needs to be created' do it 'creates linked identity' do - expect { subject.create_or_update }.to change { user.identities.count } + expect { subject.link }.to change { user.identities.count } end it 'sets identity provider' do - subject.create_or_update + subject.link expect(user.identities.last.provider).to eq provider end it 'sets identity extern_uid' do - subject.create_or_update + subject.link expect(user.identities.last.extern_uid).to eq uid end - it 'sets #created? to true' do - subject.create_or_update + it 'sets #changed? to true' do + subject.link - expect(subject).to be_created + expect(subject).to be_changed end end end diff --git a/spec/lib/gitlab/auth/saml/identity_linker_spec.rb b/spec/lib/gitlab/auth/saml/identity_linker_spec.rb index 11e8469e9f4..f3305d574cc 100644 --- a/spec/lib/gitlab/auth/saml/identity_linker_spec.rb +++ b/spec/lib/gitlab/auth/saml/identity_linker_spec.rb @@ -12,37 +12,37 @@ describe Gitlab::Auth::Saml::IdentityLinker do let!(:identity) { user.identities.create!(provider: provider, extern_uid: uid) } it "doesn't create new identity" do - expect { subject.create_or_update }.not_to change { Identity.count } + expect { subject.link }.not_to change { Identity.count } end - it 'sets #created? to false' do - subject.create_or_update + it "sets #changed? to false" do + subject.link - expect(subject).not_to be_created + expect(subject).not_to be_changed end end context 'identity needs to be created' do it 'creates linked identity' do - expect { subject.create_or_update }.to change { user.identities.count } + expect { subject.link }.to change { user.identities.count } end it 'sets identity provider' do - subject.create_or_update + subject.link expect(user.identities.last.provider).to eq provider end it 'sets identity extern_uid' do - subject.create_or_update + subject.link expect(user.identities.last.extern_uid).to eq uid end - it 'sets #created? to true' do - subject.create_or_update + it 'sets #changed? to true' do + subject.link - expect(subject).to be_created + expect(subject).to be_changed end end end |