diff options
author | James Edwards-Jones <jedwardsjones@gitlab.com> | 2018-04-22 21:08:08 +0300 |
---|---|---|
committer | James Edwards-Jones <jedwardsjones@gitlab.com> | 2018-04-23 01:50:56 +0300 |
commit | f8d54913bb6f846d1204704d3a6e457956461b35 (patch) | |
tree | 7541e607f0e77638411448cf42482ee3ba778cf7 | |
parent | f10c999bca2b5b37b068ff3680a6e35a6707828d (diff) |
Show error on failed OAuth account link
-rw-r--r-- | app/controllers/omniauth_callbacks_controller.rb | 6 | ||||
-rw-r--r-- | lib/gitlab/auth/o_auth/identity_linker.rb | 20 | ||||
-rw-r--r-- | lib/gitlab/auth/omniauth_identity_linker_base.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/o_auth/identity_linker_spec.rb | 20 |
4 files changed, 46 insertions, 4 deletions
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 + + def error_message + identity.validate + + identity.errors.full_messages.join(', ') + end + + private - @created = true + 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 |