diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 190ad55859b..df75693e840 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -84,6 +84,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController if identity_linker.created? redirect_identity_linked + elsif identity_linker.error_message.present? + redirect_identity_link_failed(identity_linker.error_message) else redirect_identity_exists end @@ -96,6 +98,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController redirect_to after_sign_in_path_for(current_user) end + def redirect_identity_link_failed(error_message) + redirect_to profile_account_path, notice: "Authentication failed: #{error_message}" + end + def redirect_identity_linked redirect_to profile_account_path, notice: 'Authentication method updated' end diff --git a/lib/gitlab/auth/o_auth/identity_linker.rb b/lib/gitlab/auth/o_auth/identity_linker.rb index cfa83ba2a55..704a7e1b3c2 100644 --- a/lib/gitlab/auth/o_auth/identity_linker.rb +++ b/lib/gitlab/auth/o_auth/identity_linker.rb @@ -3,11 +3,23 @@ module Gitlab module OAuth class IdentityLinker < OmniauthIdentityLinkerBase def create_or_update - current_user.identities - .with_extern_uid(oauth['provider'], oauth['uid']) - .first_or_create(extern_uid: oauth['uid']) + if identity.new_record? + @created = identity.save + end + end - @created = true + 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 diff --git a/lib/gitlab/auth/omniauth_identity_linker_base.rb b/lib/gitlab/auth/omniauth_identity_linker_base.rb index c60d9f70a99..4ed28d6a8be 100644 --- a/lib/gitlab/auth/omniauth_identity_linker_base.rb +++ b/lib/gitlab/auth/omniauth_identity_linker_base.rb @@ -13,6 +13,10 @@ module Gitlab @created end + def error_message + '' + end + def create_or_update raise NotImplementedError 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 dc6aa5de53a..8d1b0a3cd4b 100644 --- a/spec/lib/gitlab/auth/o_auth/identity_linker_spec.rb +++ b/spec/lib/gitlab/auth/o_auth/identity_linker_spec.rb @@ -14,6 +14,26 @@ describe Gitlab::Auth::OAuth::IdentityLinker do it "doesn't create new identity" do expect { subject.create_or_update }.not_to change { Identity.count } end + + it "#created? returns false" do + subject.create_or_update + + expect(subject).not_to be_created + 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 + + expect(subject).not_to be_created + end + + it 'exposes error message' do + expect(subject.error_message).to eq 'Extern uid has already been taken' + end end context 'identity needs to be created' do