From c593154cb4f0215851fbbae1dde753dacbaa6713 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Tue, 7 Jun 2016 11:19:19 -0500 Subject: Moved `find_or_create_ldap_user` method to parent class and added logging. --- lib/gitlab/o_auth/user.rb | 18 ++++++++++++------ lib/gitlab/saml/user.rb | 24 ------------------------ spec/lib/gitlab/saml/user_spec.rb | 2 +- 3 files changed, 13 insertions(+), 31 deletions(-) diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 268ee115028..5e52979093d 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -69,13 +69,19 @@ module Gitlab return unless ldap_person # If a corresponding person exists with same uid in a LDAP server, - # set up a Gitlab user with dual LDAP and Omniauth identities. - if user = Gitlab::LDAP::User.find_by_uid_and_provider(ldap_person.dn, ldap_person.provider) - # Case when a LDAP user already exists in Gitlab. Add the Omniauth identity to existing account. + # check if the user already has a GitLab account. + if (user = Gitlab::LDAP::User.find_by_uid_and_provider(ldap_person.dn, ldap_person.provider)) + # Case when a LDAP user already exists in Gitlab. Add the OAuth identity to existing account. + log.info "LDAP account found for user #{user.username}. Building new identity." user.identities.build(extern_uid: auth_hash.uid, provider: auth_hash.provider) else - # No account in Gitlab yet: create it and add the LDAP identity - user = build_new_user + log.info 'No existing LDAP account was found in GitLab. Checking for OAuth account.' + user = find_by_uid_and_provider + if user.nil? + log.info 'No user found with the specified OAuth provider. Creating a new one.' + user = build_new_user + end + log.info "Correct account has been found. Adding LDAP identity to user: #{user.username}." user.identities.new(provider: ldap_person.provider, extern_uid: ldap_person.dn) end @@ -96,7 +102,7 @@ module Gitlab # Look for a corresponding person with same uid in any of the configured LDAP providers Gitlab::LDAP::Config.providers.each do |provider| adapter = Gitlab::LDAP::Adapter.new(provider) - @ldap_person = Gitlab::LDAP::Person.find_by_dn(auth_hash.uid, adapter) + @ldap_person = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter) break if @ldap_person end @ldap_person diff --git a/lib/gitlab/saml/user.rb b/lib/gitlab/saml/user.rb index 6f7d4825ae5..8943022612c 100644 --- a/lib/gitlab/saml/user.rb +++ b/lib/gitlab/saml/user.rb @@ -62,30 +62,6 @@ module Gitlab !Gitlab::Saml::Config.external_groups.nil? end - def find_or_create_ldap_user - return unless ldap_person - - # If a corresponding person exists with same uid in a LDAP server, - # check if the user already has a GitLab account - user = Gitlab::LDAP::User.find_by_uid_and_provider(ldap_person.dn, ldap_person.provider) - if user - # Case when a LDAP user already exists in Gitlab. Add the SAML identity to existing account. - user.identities.build(extern_uid: auth_hash.uid, provider: auth_hash.provider) - else - # No account found using LDAP in Gitlab yet: check if there is a SAML account with - # the passed uid and provider - user = find_by_uid_and_provider - if user.nil? - # No SAML account found, build a new user. - user = build_new_user - end - # Correct account is present, add the LDAP Identity to the user. - user.identities.new(provider: ldap_person.provider, extern_uid: ldap_person.dn) - end - - user - end - def auth_hash=(auth_hash) @auth_hash = Gitlab::Saml::AuthHash.new(auth_hash) end diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb index f0a17244ff6..5957998e0f3 100644 --- a/spec/lib/gitlab/saml/user_spec.rb +++ b/spec/lib/gitlab/saml/user_spec.rb @@ -182,7 +182,7 @@ describe Gitlab::Saml::User, lib: true do context 'user has SAML user, and wants to add their LDAP identity' do it 'adds the LDAP identity to the existing SAML user' do create(:omniauth_user, email: 'john@mail.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'saml', username: 'john') - local_hash = OmniAuth::AuthHash.new(uid: 'uid=user1,ou=People,dc=example', provider: provider, info: info_hash, extra: { raw_info: OneLogin::RubySaml::Attributes.new({ 'groups' => %w(Developers Freelancers Designers) }) }) + local_hash = OmniAuth::AuthHash.new(uid: 'uid=user1,ou=People,dc=example', provider: provider, info: info_hash) local_saml_user = described_class.new(local_hash) local_saml_user.save -- cgit v1.2.1