Issue #993: Fixed login failure when extern_uid changes
This commit is contained in:
parent
f5b3bf84ad
commit
4d2f36118a
|
@ -75,6 +75,7 @@ v 7.14.0 (unreleased)
|
|||
- Update Flowdock integration to support new Flowdock API (Boyan Tabakov)
|
||||
- Remove author from files view (Sven Strickroth)
|
||||
- Fix infinite loop when SAML was incorrectly configured.
|
||||
- Fixed login failure when extern_uid changes (Joel Koglin)
|
||||
|
||||
v 7.13.5
|
||||
- Satellites reverted
|
||||
|
|
|
@ -104,7 +104,7 @@ class User < ActiveRecord::Base
|
|||
# Profile
|
||||
has_many :keys, dependent: :destroy
|
||||
has_many :emails, dependent: :destroy
|
||||
has_many :identities, dependent: :destroy
|
||||
has_many :identities, dependent: :destroy, autosave: true
|
||||
|
||||
# Groups
|
||||
has_many :members, dependent: :destroy
|
||||
|
|
|
@ -0,0 +1,14 @@
|
|||
class DeduplicateUserIdentities < ActiveRecord::Migration
|
||||
def change
|
||||
execute 'DROP TABLE IF EXISTS tt_migration_DeduplicateUserIdentities;'
|
||||
execute 'CREATE TEMPORARY TABLE tt_migration_DeduplicateUserIdentities AS SELECT id,provider,user_id FROM identities;'
|
||||
execute 'DELETE FROM identities WHERE id NOT IN ( SELECT MIN(id) FROM tt_migration_DeduplicateUserIdentities GROUP BY user_id, provider);'
|
||||
execute 'DROP TABLE IF EXISTS tt_migration_DeduplicateUserIdentities;'
|
||||
end
|
||||
|
||||
def down
|
||||
# This is an irreversible migration;
|
||||
# If someone is trying to rollback for other reasons, we should not throw an Exception.
|
||||
# raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
|
@ -11,7 +11,7 @@
|
|||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema.define(version: 20150812080800) do
|
||||
ActiveRecord::Schema.define(version: 20150817163600) do
|
||||
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "plpgsql"
|
||||
|
|
|
@ -44,9 +44,13 @@ module Gitlab
|
|||
gl_user.skip_reconfirmation!
|
||||
gl_user.email = auth_hash.email
|
||||
|
||||
# Build new identity only if we dont have have same one
|
||||
gl_user.identities.find_or_initialize_by(provider: auth_hash.provider,
|
||||
extern_uid: auth_hash.uid)
|
||||
# If we don't have an identity for this provider yet, create one
|
||||
if gl_user.identities.find_by(provider: auth_hash.provider).nil?
|
||||
gl_user.identities.new(extern_uid: auth_hash.uid, provider: auth_hash.provider)
|
||||
else # Update the UID attribute for this provider in case it has changed
|
||||
identity = gl_user.identities.select { |identity| identity.provider == auth_hash.provider }
|
||||
identity.first.extern_uid = auth_hash.uid
|
||||
end
|
||||
|
||||
gl_user
|
||||
end
|
||||
|
|
|
@ -47,6 +47,28 @@ describe Gitlab::LDAP::User do
|
|||
expect(existing_user.ldap_identity.provider).to eql 'ldapmain'
|
||||
end
|
||||
|
||||
it 'connects to existing ldap user if the extern_uid changes' do
|
||||
existing_user = create(:omniauth_user, email: 'john@example.com', extern_uid: 'old-uid', provider: 'ldapmain')
|
||||
expect{ ldap_user.save }.not_to change{ User.count }
|
||||
|
||||
existing_user.reload
|
||||
expect(existing_user.ldap_identity.extern_uid).to eql 'my-uid'
|
||||
expect(existing_user.ldap_identity.provider).to eql 'ldapmain'
|
||||
expect(existing_user.id).to eql ldap_user.gl_user.id
|
||||
end
|
||||
|
||||
it 'maintains an identity per provider' do
|
||||
existing_user = create(:omniauth_user, email: 'john@example.com', provider: 'twitter')
|
||||
expect(existing_user.identities.count).to eql(1)
|
||||
|
||||
ldap_user.save
|
||||
expect(ldap_user.gl_user.identities.count).to eql(2)
|
||||
|
||||
# Expect that find_by provider only returns a single instance of an identity and not an Enumerable
|
||||
expect(ldap_user.gl_user.identities.find_by(provider: 'twitter')).to be_instance_of Identity
|
||||
expect(ldap_user.gl_user.identities.find_by(provider: auth_hash.provider)).to be_instance_of Identity
|
||||
end
|
||||
|
||||
it "creates a new user if not found" do
|
||||
expect{ ldap_user.save }.to change{ User.count }.by(1)
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue