Merge branch 'ee-38822-oauth-search-case-insensitive' into 'master'

Port from CE to EE: Changing OAuth lookup to be case insensitive

See merge request gitlab-org/gitlab-ee!3427

app/helpers/kerberos_spnego_helper.rb

@@ -45,7 +45,7 @@ module KerberosSpnegoHelper
     krb_principal = spnego_credentials!(spnego_token)
     return unless krb_principal
 
-    identity = ::Identity.find_by(provider: :kerberos, extern_uid: krb_principal)
+    identity = ::Identity.with_extern_uid(:kerberos, krb_principal).take
     identity&.user
   end
 

app/models/identity.rb

 class Identity < ActiveRecord::Base
+  prepend EE::Identity
+
   include Sortable
   include CaseSensitivity
+
   belongs_to :user
 
   validates :provider, presence: true
-  validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider }
+  validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider, case_sensitive: false }
   validates :user_id, uniqueness: { scope: :provider }
 
   scope :with_provider, ->(provider) { where(provider: provider) }
   scope :with_extern_uid, ->(provider, extern_uid) do
-    extern_uid = Gitlab::LDAP::Person.normalize_dn(extern_uid) if provider.starts_with?('ldap')
-    where(extern_uid: extern_uid, provider: provider)
+    iwhere(extern_uid: normalize_uid(provider, extern_uid)).with_provider(provider)
   end
 
   def ldap?
     provider.starts_with?('ldap')
   end
+
+  def self.normalize_uid(provider, uid)
+    if provider.to_s.starts_with?('ldap')
+      Gitlab::LDAP::Person.normalize_dn(uid)
+    else
+      uid.to_s
+    end
+  end
 end

app/models/user.rb

@@ -278,8 +278,7 @@ class User < ActiveRecord::Base
     end
 
     def for_github_id(id)
-      joins(:identities)
-        .where(identities: { provider: :github, extern_uid: id.to_s })
+      joins(:identities).merge(Identity.with_extern_uid(:github, id))
     end
 
     # Find a User by their primary email or any associated secondary email

changelogs/unreleased/38822-oauth-search-case-insensitive.yml

+---
+title: OAuth identity lookups case-insensitive
+merge_request: 15312
+author:
+type: fixed

ee/app/models/ee/identity.rb

+module EE
+  module Identity
+    extend ActiveSupport::Concern
+
+    prepended do
+      validates :secondary_extern_uid, allow_blank: true, uniqueness: { scope: :provider, case_sensitive: false }
+
+      scope :with_secondary_extern_uid, ->(provider, secondary_extern_uid) do
+        iwhere(secondary_extern_uid: normalize_uid(provider, secondary_extern_uid)).with_provider(provider)
+      end
+    end
+  end
+end

ee/lib/ee/gitlab/ldap/sync/proxy.rb

@@ -108,7 +108,7 @@ module EE
           end
 
           def member_uid_to_dn(uid)
-            identity = Identity.find_by(provider: provider, secondary_extern_uid: uid)
+            identity = ::Identity.with_secondary_extern_uid(provider, uid).take
 
             if identity.present?
               # Use the DN on record in GitLab when it's available
@@ -127,8 +127,7 @@ module EE
           end
 
           def update_identity(dn, uid)
-            identity =
-              Identity.find_by(provider: provider, extern_uid: dn)
+            identity = ::Identity.with_extern_uid(provider, dn).take
 
             # User may not exist in GitLab yet. Skip.
             return unless identity.present?

lib/gitlab/kerberos/authentication.rb

@@ -44,7 +44,7 @@ module Gitlab
       private
 
       def find_by_login(login)
-        identity = ::Identity.find_by(provider: :kerberos, extern_uid: login)
+        identity = ::Identity.with_extern_uid(:kerberos, login).take
         identity && identity.user
       end
     end

lib/gitlab/ldap/user.rb

@@ -11,11 +11,8 @@ module Gitlab
 
       class << self
         def find_by_uid_and_provider(uid, provider)
-          uid = Gitlab::LDAP::Person.normalize_dn(uid)
+          identity = ::Identity.with_extern_uid(provider, uid).take
 
-          identity = ::Identity
-            .where(provider: provider)
-            .where(extern_uid: uid).last
           identity && identity.user
         end
       end

lib/gitlab/o_auth/user.rb

@@ -159,7 +159,7 @@ module Gitlab
       end
 
       def find_by_uid_and_provider
-        identity = Identity.find_by(provider: auth_hash.provider, extern_uid: auth_hash.uid)
+        identity = Identity.with_extern_uid(auth_hash.provider, auth_hash.uid).take
         identity && identity.user
       end
 

spec/ee/spec/lib/ee/gitlab/ldap/sync/proxy_spec.rb

@@ -182,11 +182,9 @@ describe EE::Gitlab::LDAP::Sync::Proxy do
 
       it 'retrieves the DN from the identity' do
         expect(Identity)
-          .to receive(:find_by)
-                .with(
-                  provider: sync_proxy.provider,
-                  secondary_extern_uid: user.username
-                ).once.and_call_original
+          .to receive(:with_secondary_extern_uid)
+                .with(sync_proxy.provider, user.username)
+                .once.and_call_original
       end
     end
   end

spec/lib/gitlab/o_auth/user_spec.rb

@@ -662,4 +662,13 @@ describe Gitlab::OAuth::User do
       end
     end
   end
+
+  describe '.find_by_uid_and_provider' do
+    let!(:existing_user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'my-provider') }
+
+    it 'normalizes extern_uid' do
+      allow(oauth_user.auth_hash).to receive(:uid).and_return('MY-UID')
+      expect(oauth_user.find_user).to eql gl_user
+    end
+  end
 end

spec/models/identity_spec.rb

@@ -33,5 +33,15 @@ describe Identity do
         expect(identity).to eq(ldap_identity)
       end
     end
+
+    context 'any other provider' do
+      let!(:test_entity) { create(:identity, provider: 'test_provider', extern_uid: 'test_uid') }
+
+      it 'the extern_uid lookup is case insensitive' do
+        identity = described_class.with_extern_uid('test_provider', 'TEST_UID').first
+
+        expect(identity).to eq(test_entity)
+      end
+    end
   end
 end