Commit d3a8a074 authored by James Edwards-Jones's avatar James Edwards-Jones

Unify Saml::IdentityLinker and OAuth::IdentityLinker

parent f8d54913
...@@ -80,9 +80,9 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -80,9 +80,9 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
identity_linker ||= auth_module::IdentityLinker.new(current_user, oauth) 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 redirect_identity_linked
elsif identity_linker.error_message.present? elsif identity_linker.error_message.present?
redirect_identity_link_failed(identity_linker.error_message) redirect_identity_link_failed(identity_linker.error_message)
......
...@@ -2,25 +2,6 @@ module Gitlab ...@@ -2,25 +2,6 @@ module Gitlab
module Auth module Auth
module OAuth module OAuth
class IdentityLinker < OmniauthIdentityLinkerBase 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 end
end end
......
...@@ -6,19 +6,41 @@ module Gitlab ...@@ -6,19 +6,41 @@ module Gitlab
def initialize(current_user, oauth) def initialize(current_user, oauth)
@current_user = current_user @current_user = current_user
@oauth = oauth @oauth = oauth
@created = false @changed = false
end end
def created? def link
@created save if identity.new_record?
end
def changed?
@changed
end end
def error_message 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 end
def create_or_update def uid
raise NotImplementedError oauth['uid']
end end
end end
end end
......
...@@ -2,25 +2,6 @@ module Gitlab ...@@ -2,25 +2,6 @@ module Gitlab
module Auth module Auth
module Saml module Saml
class IdentityLinker < OmniauthIdentityLinkerBase 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 end
end end
......
...@@ -12,23 +12,23 @@ describe Gitlab::Auth::OAuth::IdentityLinker do ...@@ -12,23 +12,23 @@ describe Gitlab::Auth::OAuth::IdentityLinker do
let!(:identity) { user.identities.create!(provider: provider, extern_uid: uid) } let!(:identity) { user.identities.create!(provider: provider, extern_uid: uid) }
it "doesn't create new identity" do 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 end
it "#created? returns false" do it "sets #changed? to false" do
subject.create_or_update subject.link
expect(subject).not_to be_created expect(subject).not_to be_changed
end end
end end
context 'identity already linked to different user' do context 'identity already linked to different user' do
let!(:identity) { create(:identity, provider: provider, extern_uid: uid) } let!(:identity) { create(:identity, provider: provider, extern_uid: uid) }
it "#created? returns false" do it "#changed? returns false" do
subject.create_or_update subject.link
expect(subject).not_to be_created expect(subject).not_to be_changed
end end
it 'exposes error message' do it 'exposes error message' do
...@@ -38,25 +38,25 @@ describe Gitlab::Auth::OAuth::IdentityLinker do ...@@ -38,25 +38,25 @@ describe Gitlab::Auth::OAuth::IdentityLinker do
context 'identity needs to be created' do context 'identity needs to be created' do
it 'creates linked identity' 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 end
it 'sets identity provider' do it 'sets identity provider' do
subject.create_or_update subject.link
expect(user.identities.last.provider).to eq provider expect(user.identities.last.provider).to eq provider
end end
it 'sets identity extern_uid' do it 'sets identity extern_uid' do
subject.create_or_update subject.link
expect(user.identities.last.extern_uid).to eq uid expect(user.identities.last.extern_uid).to eq uid
end end
it 'sets #created? to true' do it 'sets #changed? to true' do
subject.create_or_update subject.link
expect(subject).to be_created expect(subject).to be_changed
end end
end end
end end
...@@ -12,37 +12,37 @@ describe Gitlab::Auth::Saml::IdentityLinker do ...@@ -12,37 +12,37 @@ describe Gitlab::Auth::Saml::IdentityLinker do
let!(:identity) { user.identities.create!(provider: provider, extern_uid: uid) } let!(:identity) { user.identities.create!(provider: provider, extern_uid: uid) }
it "doesn't create new identity" do 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 end
it 'sets #created? to false' do it "sets #changed? to false" do
subject.create_or_update subject.link
expect(subject).not_to be_created expect(subject).not_to be_changed
end end
end end
context 'identity needs to be created' do context 'identity needs to be created' do
it 'creates linked identity' 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 end
it 'sets identity provider' do it 'sets identity provider' do
subject.create_or_update subject.link
expect(user.identities.last.provider).to eq provider expect(user.identities.last.provider).to eq provider
end end
it 'sets identity extern_uid' do it 'sets identity extern_uid' do
subject.create_or_update subject.link
expect(user.identities.last.extern_uid).to eq uid expect(user.identities.last.extern_uid).to eq uid
end end
it 'sets #created? to true' do it 'sets #changed? to true' do
subject.create_or_update subject.link
expect(subject).to be_created expect(subject).to be_changed
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment