Skip to content

Commit 4ab717e

Browse files
committed
Merge branch 'ldap_migration'
Signed-off-by: Dmitriy Zaporozhets <[email protected]> Conflicts: db/schema.rb
2 parents ecb58da + f39b150 commit 4ab717e

File tree

8 files changed

+48
-20
lines changed

8 files changed

+48
-20
lines changed

app/models/identity.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,5 @@ class Identity < ActiveRecord::Base
1515
belongs_to :user
1616

1717
validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider }
18+
validates :user_id, uniqueness: { scope: :provider }
1819
end

config/gitlab.yml.example

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,15 @@ production: &base
105105
ldap:
106106
enabled: false
107107
servers:
108+
##########################################################################
109+
#
110+
# Since GitLab 7.4, LDAP servers get ID's (below the ID is 'main'). GitLab
111+
# Enterprise Edition now supports connecting to multiple LDAP servers.
112+
#
113+
# If you are updating from the old (pre-7.4) syntax, you MUST give your
114+
# old server the ID 'main'.
115+
#
116+
##########################################################################
108117
main: # 'main' is the GitLab 'provider ID' of this LDAP server
109118
## label
110119
#

config/initializers/1_settings.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,11 @@ def verify_constant(modul, current, default)
6666
# backwards compatibility, we only have one host
6767
if Settings.ldap['enabled'] || Rails.env.test?
6868
if Settings.ldap['host'].present?
69+
# We detected old LDAP configuration syntax. Update the config to make it
70+
# look like it was entered with the new syntax.
6971
server = Settings.ldap.except('sync_time')
70-
server['provider_name'] = 'ldap'
7172
Settings.ldap['servers'] = {
72-
'ldap' => server
73+
'main' => server
7374
}
7475
end
7576

@@ -82,6 +83,7 @@ def verify_constant(modul, current, default)
8283
end
8384
end
8485

86+
8587
Settings['omniauth'] ||= Settingslogic.new({})
8688
Settings.omniauth['enabled'] = false if Settings.omniauth['enabled'].nil?
8789
Settings.omniauth['providers'] ||= []
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
class FixIdentities < ActiveRecord::Migration
2+
def up
3+
# Up until now, legacy 'ldap' references in the database were charitably
4+
# interpreted to point to the first LDAP server specified in the GitLab
5+
# configuration. So if the database said 'provider: ldap' but the first
6+
# LDAP server was called 'ldapmain', then we would try to interpret
7+
# 'provider: ldap' as if it said 'provider: ldapmain'. This migration (and
8+
# accompanying changes in the GitLab LDAP code) get rid of this complicated
9+
# behavior. Any database references to 'provider: ldap' get rewritten to
10+
# whatever the code would have interpreted it as, i.e. as a reference to
11+
# the first LDAP server specified in gitlab.yml / gitlab.rb.
12+
new_provider = if Gitlab.config.ldap.enabled
13+
first_ldap_server = Gitlab.config.ldap.servers.values.first
14+
first_ldap_server['provider_name']
15+
else
16+
'ldapmain'
17+
end
18+
19+
# Delete duplicate identities
20+
execute "DELETE FROM identities WHERE provider = 'ldap' AND user_id IN (SELECT user_id FROM identities WHERE provider = '#{new_provider}')"
21+
22+
# Update legacy identities
23+
execute "UPDATE identities SET provider = '#{new_provider}' WHERE provider = 'ldap';"
24+
25+
if table_exists?('ldap_group_links')
26+
execute "UPDATE ldap_group_links SET provider = '#{new_provider}' WHERE provider IS NULL OR provider = 'ldap';"
27+
end
28+
end
29+
30+
def down
31+
end
32+
end

db/schema.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,14 +460,14 @@
460460
t.integer "notification_level", default: 1, null: false
461461
t.datetime "password_expires_at"
462462
t.integer "created_by_id"
463+
t.datetime "last_credential_check_at"
463464
t.string "avatar"
464465
t.string "confirmation_token"
465466
t.datetime "confirmed_at"
466467
t.datetime "confirmation_sent_at"
467468
t.string "unconfirmed_email"
468469
t.boolean "hide_no_ssh_key", default: false
469470
t.string "website_url", default: "", null: false
470-
t.datetime "last_credential_check_at"
471471
t.string "github_access_token"
472472
t.string "gitlab_access_token"
473473
t.string "notification_email"

lib/gitlab/ldap/config.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ def self.invalid_provider(provider)
2727
def initialize(provider)
2828
if self.class.valid_provider?(provider)
2929
@provider = provider
30-
elsif provider == 'ldap'
31-
@provider = self.class.providers.first
3230
else
3331
self.class.invalid_provider(provider)
3432
end

lib/gitlab/ldap/user.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class << self
1313
def find_by_uid_and_provider(uid, provider)
1414
# LDAP distinguished name is case-insensitive
1515
identity = ::Identity.
16-
where(provider: [provider, :ldap]).
16+
where(provider: provider).
1717
where('lower(extern_uid) = ?', uid.downcase).last
1818
identity && identity.user
1919
end

spec/lib/gitlab/ldap/config_spec.rb

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,5 @@
1616
it "raises an error if a unknow provider is used" do
1717
expect{ Gitlab::LDAP::Config.new 'unknown' }.to raise_error
1818
end
19-
20-
context "if 'ldap' is the provider name" do
21-
let(:provider) { 'ldap' }
22-
23-
context "and 'ldap' is not in defined as a provider" do
24-
before { Gitlab::LDAP::Config.stub(providers: %w{ldapmain}) }
25-
26-
it "uses the first provider" do
27-
# Fetch the provider_name attribute from 'options' so that we know
28-
# that the 'options' Hash is not empty/nil.
29-
expect(config.options['provider_name']).to eq('ldapmain')
30-
end
31-
end
32-
end
3319
end
3420
end

0 commit comments

Comments
 (0)