Merge branch 'fj-40279-normalize-ldap-dn-api' into 'master'

LDAP extern_uids are not normalized when updated via API

Closes #40279

See merge request gitlab-org/gitlab-ce!16004

app/models/identity.rb

@@ -8,6 +8,8 @@ class Identity < ActiveRecord::Base
   validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider, case_sensitive: false }
   validates :user_id, uniqueness: { scope: :provider }
 
+  before_save :ensure_normalized_extern_uid, if: :extern_uid_changed?
+
   scope :with_provider, ->(provider) { where(provider: provider) }
   scope :with_extern_uid, ->(provider, extern_uid) do
     iwhere(extern_uid: normalize_uid(provider, extern_uid)).with_provider(provider)
@@ -24,4 +26,12 @@ class Identity < ActiveRecord::Base
       uid.to_s
     end
   end
+
+  private
+
+  def ensure_normalized_extern_uid
+    return if extern_uid.nil?
+
+    self.extern_uid = Identity.normalize_uid(self.provider, self.extern_uid)
+  end
 end

changelogs/unreleased/fj-40279-normalize-ldap-dn-api.yml

+---
+title: Normalizing Identity extern_uid when saving the record
+merge_request:
+author:
+type: fixed

db/post_migrate/20171219121201_normalize_extern_uid_from_identities.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class NormalizeExternUidFromIdentities < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  MIGRATION = 'NormalizeLdapExternUidsRange'.freeze
+  DELAY_INTERVAL = 10.seconds
+
+  disable_ddl_transaction!
+
+  class Identity < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'identities'
+  end
+
+  def up
+    ldap_identities = Identity.where("provider like 'ldap%'")
+
+    if ldap_identities.any?
+      queue_background_migration_jobs_by_range_at_intervals(Identity, MIGRATION, DELAY_INTERVAL)
+    end
+  end
+
+  def down
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20171213160445) do
+ActiveRecord::Schema.define(version: 20171219121201) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

spec/models/identity_spec.rb

@@ -44,4 +44,31 @@ describe Identity do
       end
     end
   end
+
+  context 'callbacks' do
+    context 'before_save' do
+      describe 'normalizes extern uid' do
+        let!(:ldap_identity) { create(:identity, provider: 'ldapmain', extern_uid: 'uid=john smith,ou=people,dc=example,dc=com') }
+
+        it 'if extern_uid changes' do
+          expect(ldap_identity).not_to receive(:ensure_normalized_extern_uid)
+          ldap_identity.save
+        end
+
+        it 'if current_uid is nil' do
+          expect(ldap_identity).to receive(:ensure_normalized_extern_uid)
+
+          ldap_identity.update(extern_uid: nil)
+
+          expect(ldap_identity.extern_uid).to be_nil
+        end
+
+        it 'if extern_uid changed and not nil' do
+          ldap_identity.update(extern_uid: 'uid=john1,ou=PEOPLE,dc=example,dc=com')
+
+          expect(ldap_identity.extern_uid).to eq 'uid=john1,ou=people,dc=example,dc=com'
+        end
+      end
+    end
+  end
 end