From 8c6b5baba1d784b1615d023c63fb4531713ef8db Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= <fjlopez@gitlab.com>
Date: Thu, 21 Dec 2017 14:31:15 +0000
Subject: LDAP extern_uids are not normalized when updated via API

---
 app/models/identity.rb                             | 10 ++++++++
 .../unreleased/fj-40279-normalize-ldap-dn-api.yml  |  5 ++++
 ...9121201_normalize_extern_uid_from_identities.rb | 29 ++++++++++++++++++++++
 db/schema.rb                                       |  2 +-
 spec/models/identity_spec.rb                       | 27 ++++++++++++++++++++
 5 files changed, 72 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/fj-40279-normalize-ldap-dn-api.yml
 create mode 100644 db/post_migrate/20171219121201_normalize_extern_uid_from_identities.rb

diff --git a/app/models/identity.rb b/app/models/identity.rb
index 99d99bc6deb..b3fa7d8176a 100644
--- a/app/models/identity.rb
+++ b/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
diff --git a/changelogs/unreleased/fj-40279-normalize-ldap-dn-api.yml b/changelogs/unreleased/fj-40279-normalize-ldap-dn-api.yml
new file mode 100644
index 00000000000..3fd8b0eb988
--- /dev/null
+++ b/changelogs/unreleased/fj-40279-normalize-ldap-dn-api.yml
@@ -0,0 +1,5 @@
+---
+title: Normalizing Identity extern_uid when saving the record
+merge_request:
+author:
+type: fixed
diff --git a/db/post_migrate/20171219121201_normalize_extern_uid_from_identities.rb b/db/post_migrate/20171219121201_normalize_extern_uid_from_identities.rb
new file mode 100644
index 00000000000..286721a0894
--- /dev/null
+++ b/db/post_migrate/20171219121201_normalize_extern_uid_from_identities.rb
@@ -0,0 +1,29 @@
+# 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
diff --git a/db/schema.rb b/db/schema.rb
index 2048c50f892..2b20628bd53 100644
--- a/db/schema.rb
+++ b/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"
diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb
index a45a6088831..7c66c98231b 100644
--- a/spec/models/identity_spec.rb
+++ b/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
-- 
cgit v1.2.1