Merge branch 'ldap_migration'

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

Conflicts:
	db/schema.rb

7 files changed, 47 insertions, 19 deletions

diff --git a/app/models/identity.rb b/app/models/identity.rb
index 440fcd0d052..756d19adec7 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -15,4 +15,5 @@ class Identity < ActiveRecord::Base
   belongs_to :user
 
   validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider }
+  validates :user_id, uniqueness: { scope: :provider }
 end
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index a85db10e019..416e4e59465 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -128,6 +128,15 @@ production: &base
   ldap:
     enabled: false
     servers:
+      ##########################################################################
+      #
+      # Since GitLab 7.4, LDAP servers get ID's (below the ID is 'main'). GitLab
+      # Enterprise Edition now supports connecting to multiple LDAP servers.
+      #
+      # If you are updating from the old (pre-7.4) syntax, you MUST give your
+      # old server the ID 'main'.
+      #
+      ##########################################################################
       main: # 'main' is the GitLab 'provider ID' of this LDAP server
         ## label
         #
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index 70af7a829c4..f51cace896f 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -64,10 +64,11 @@ Settings.ldap['enabled'] = false if Settings.ldap['enabled'].nil?
 # backwards compatibility, we only have one host
 if Settings.ldap['enabled'] || Rails.env.test?
   if Settings.ldap['host'].present?
+    # We detected old LDAP configuration syntax. Update the config to make it
+    # look like it was entered with the new syntax.
     server = Settings.ldap.except('sync_time')
-    server['provider_name'] = 'ldap'
     Settings.ldap['servers'] = {
-      'ldap' => server
+      'main' => server
     }
   end
 
@@ -80,6 +81,7 @@ if Settings.ldap['enabled'] || Rails.env.test?
   end
 end
 
+
 Settings['omniauth'] ||= Settingslogic.new({})
 Settings.omniauth['enabled']      = false if Settings.omniauth['enabled'].nil?
 Settings.omniauth['providers']  ||= []
diff --git a/db/migrate/20150411000035_fix_identities.rb b/db/migrate/20150411000035_fix_identities.rb
new file mode 100644
index 00000000000..8f11a96ab01
--- /dev/null
+++ b/db/migrate/20150411000035_fix_identities.rb
@@ -0,0 +1,32 @@
+class FixIdentities < ActiveRecord::Migration
+  def up
+    # Up until now, legacy 'ldap' references in the database were charitably
+    # interpreted to point to the first LDAP server specified in the GitLab
+    # configuration. So if the database said 'provider: ldap' but the first
+    # LDAP server was called 'ldapmain', then we would try to interpret
+    # 'provider: ldap' as if it said 'provider: ldapmain'. This migration (and
+    # accompanying changes in the GitLab LDAP code) get rid of this complicated
+    # behavior. Any database references to 'provider: ldap' get rewritten to
+    # whatever the code would have interpreted it as, i.e. as a reference to
+    # the first LDAP server specified in gitlab.yml / gitlab.rb.
+    new_provider = if Gitlab.config.ldap.enabled
+                     first_ldap_server = Gitlab.config.ldap.servers.values.first
+                     first_ldap_server['provider_name']
+                   else
+                     'ldapmain'
+                   end
+
+    # Delete duplicate identities
+    execute "DELETE FROM identities WHERE provider = 'ldap' AND user_id IN (SELECT user_id FROM identities WHERE provider = '#{new_provider}')"
+
+    # Update legacy identities
+    execute "UPDATE identities SET provider = '#{new_provider}' WHERE provider = 'ldap';"
+
+    if table_exists?('ldap_group_links')
+      execute "UPDATE ldap_group_links SET provider = '#{new_provider}' WHERE provider IS NULL OR provider = 'ldap';"
+    end
+  end
+
+  def down
+  end
+end
diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb
index 0cb24d0ccc1..fa5b6c1e230 100644
--- a/lib/gitlab/ldap/config.rb
+++ b/lib/gitlab/ldap/config.rb
@@ -27,8 +27,6 @@ module Gitlab
       def initialize(provider)
         if self.class.valid_provider?(provider)
           @provider = provider
-        elsif provider == 'ldap'
-          @provider = self.class.providers.first
         else
           self.class.invalid_provider(provider)
         end
diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index cfa8692659d..fcc0936a41b 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -13,7 +13,7 @@ module Gitlab
         def find_by_uid_and_provider(uid, provider)
           # LDAP distinguished name is case-insensitive
           identity = ::Identity.
-            where(provider: [provider, :ldap]).
+            where(provider: provider).
             where('lower(extern_uid) = ?', uid.downcase).last
           identity && identity.user
         end
diff --git a/spec/lib/gitlab/ldap/config_spec.rb b/spec/lib/gitlab/ldap/config_spec.rb
index 2df2beca7a6..00e9076c787 100644
--- a/spec/lib/gitlab/ldap/config_spec.rb
+++ b/spec/lib/gitlab/ldap/config_spec.rb
@@ -16,19 +16,5 @@ describe Gitlab::LDAP::Config do
     it "raises an error if a unknow provider is used" do
       expect{ Gitlab::LDAP::Config.new 'unknown' }.to raise_error
     end
-
-    context "if 'ldap' is the provider name" do
-      let(:provider) { 'ldap' }
-
-      context "and 'ldap' is not in defined as a provider" do
-        before { Gitlab::LDAP::Config.stub(providers: %w{ldapmain}) }
-
-        it "uses the first provider" do
-          # Fetch the provider_name attribute from 'options' so that we know
-          # that the 'options' Hash is not empty/nil.
-          expect(config.options['provider_name']).to eq('ldapmain')
-        end
-      end
-    end
   end
 end