Skip to content

G
gitlab-ce
Commit c1865b69 authored by Jan-Willem van der Meer's avatar Jan-Willem van der Meer
Browse files
Options

Fix merge conflicts 

Due to the complicated merge, part of the new functionalilty is inserted here
parent 4ace6dbf
Show whitespace changes
Inline Side-by-side
Showing with 115 additions and 136 deletions
lib/api/ldap.rb
View file @ c1865b69
......@@ -10,7 +10,10 @@ module API
# Example Request:
# GET /ldap/groups
get 'groups' do
@groups = Gitlab::LDAP::Adapter.new.groups("#{params[:search]}*", 20)
# NOTE: this should be deprecated in favour of /ldap/PROVIDER_NAME/groups
# for now we just select the first LDAP server
provider = Gitlab::LDAP::Config.servers.first['provider_name']
@groups = Gitlab::LDAP::Adapter.new(provider).groups("#{params[:search]}*", 20)
present @groups, with: Entities::LdapGroup
end
end
......
lib/gitlab/ldap/access.rb
View file @ c1865b69
# LDAP authorization model
#
# * Check if we are allowed access (not blocked)
# * Update authorizations and associations
#
module Gitlab
module LDAP
class Access
attr_reader :adapter, :provider, :user
attr_reader :adapter, :provider, :user, :ldap_user
def self.open(user, &block)
Gitlab::LDAP::Adapter.open(user.provider) do |adapter|
......@@ -14,7 +15,7 @@ module Gitlab
end
def self.allowed?(user)
self.open do |access|
self.open(user) do |access|
if access.allowed?
access.update_permissions
access.update_email
......@@ -52,31 +53,28 @@ module Gitlab
Gitlab::LDAP::Config.new(provider)
end
def get_ldap_user(user)
@ldap_user ||= Gitlab::LDAP::Person.find_by_dn(user.extern_uid)
def ldap_user
@ldap_user ||= Gitlab::LDAP::Person.find_by_dn(user.extern_uid, adapter)
end
def update_permissions(user)
if Gitlab.config.ldap['sync_ssh_keys']
update_ssh_keys(user)
def update_permissions
if sync_ssh_keys?
update_ssh_keys
end
# Skip updating group permissions
# if instance does not use group_base setting
return true unless Gitlab.config.ldap['group_base'].present?
return true unless group_base.present?
update_ldap_group_links(user)
update_ldap_group_links
if Gitlab.config.ldap['admin_group'].present?
update_admin_status(user)
if admin_group.present?
update_admin_status
end
end
# Update user ssh keys if they changed in LDAP
def update_ssh_keys(user)
# Get LDAP user entry
ldap_user = get_ldap_user(user)
def update_ssh_keys
user.keys.ldap.where.not(key: ldap_user.ssh_keys).each do |deleted_key|
Rails.logger.info "#{self.class.name}: removing LDAP SSH key #{deleted_key.key} from #{user.name} (#{user.id})"
unless deleted_key.destroy
......@@ -86,7 +84,7 @@ module Gitlab
(ldap_user.ssh_keys - user.keys.ldap.pluck(:key)).each do |key|
Rails.logger.info "#{self.class.name}: adding LDAP SSH key #{key.inspect} to #{user.name} (#{user.id})"
new_key = LDAPKey.new(title: "LDAP - #{Gitlab.config.ldap['sync_ssh_keys']}", key: key)
new_key = LDAPKey.new(title: "LDAP - #{ldap_config.sync_ssh_keys}", key: key)
new_key.user = user
unless new_key.save
Rails.logger.error "#{self.class.name}: failed to add LDAP SSH key #{key.inspect} to #{user.name} (#{user.id})\n"\
......@@ -96,16 +94,12 @@ module Gitlab
end
# Update user email if it changed in LDAP
def update_email(user)
uid = user.extern_uid
ldap_user = get_ldap_user(user)
gitlab_user = ::User.where(provider: 'ldap', extern_uid: uid).last
if gitlab_user && ldap_user && ldap_user.email
def update_email
if ldap_user.try(:email)
ldap_email = ldap_user.email.last.to_s.downcase
if (gitlab_user.email != ldap_email)
gitlab_user.update(email: ldap_email)
if (user.email != ldap_email)
user.update(email: ldap_email)
else
false
end
......@@ -114,8 +108,8 @@ module Gitlab
end
end
def update_admin_status(user)
admin_group = Gitlab::LDAP::Group.find_by_cn(Gitlab.config.ldap['admin_group'], adapter)
def update_admin_status
admin_group = Gitlab::LDAP::Group.find_by_cn(ldap_config.admin_group, adapter)
if admin_group.has_member?(Gitlab::LDAP::Person.find_by_dn(user.extern_uid, adapter))
unless user.admin?
user.admin = true
......@@ -130,9 +124,9 @@ module Gitlab
end
# Loop throug all ldap conneted groups, and update the users link with it
def update_ldap_group_links(user)
def update_ldap_group_links
gitlab_groups_with_ldap_link.each do |group|
active_group_links = group.ldap_group_links.where(cn: cns_with_access(get_ldap_user(user)))
active_group_links = group.ldap_group_links.where(cn: cns_with_access)
if active_group_links.any?
group.add_users([user.id], fetch_group_access(group, user, active_group_links))
......@@ -149,12 +143,24 @@ module Gitlab
end
# returns a collection of cn strings to which the user has access
def cns_with_access(ldap_user)
def cns_with_access
@ldap_groups_with_access ||= ldap_groups.select do |ldap_group|
ldap_group.has_member?(ldap_user)
end.map(&:cn)
end
def sync_ssh_keys?
ldap_config.sync_ssh_keys?
end
def group_base
ldap_config.group_base
end
def admin_group
ldap_config.admin_group
end
private
def gitlab_groups_with_ldap_link
::Group.includes(:ldap_group_links).references(:ldap_group_links).
......
lib/gitlab/ldap/person.rb
View file @ c1865b69
......@@ -47,9 +47,8 @@ module Gitlab
end
def ssh_keys
ssh_keys_attribute = Gitlab.config.ldap['sync_ssh_keys'].to_sym
if entry.respond_to?(ssh_keys_attribute)
entry[ssh_keys_attribute]
if config.sync_ssh_keys? && entry.respond_to?(config.sync_ssh_keys)
entry[config.sync_ssh_keys.to_sym]
else
[]
end
......
spec/lib/gitlab/ldap/access_spec.rb
View file @ c1865b69
......@@ -4,7 +4,6 @@ describe Gitlab::LDAP::Access do
let(:access) { Gitlab::LDAP::Access.new user }
let(:user) { create(:user, :ldap) }
describe :allowed? do
subject { access.allowed? }
......@@ -29,7 +28,7 @@ describe Gitlab::LDAP::Access do
it { should be_true }
end
context 'without ActiveDirectory enabled' do
context 'withoud ActiveDirectory enabled' do
before do
Gitlab::LDAP::Config.stub(enabled?: true)
Gitlab::LDAP::Config.any_instance.stub(active_directory: false)
......@@ -41,161 +40,131 @@ describe Gitlab::LDAP::Access do
end
describe :update_permissions do
subject { access.update_permissions(user) }
subject { access.update_permissions }
before do
Gitlab.config.ldap['enabled'] = true
Gitlab.config.ldap['sync_ssh_keys'] = false
Gitlab.config.ldap['group_base'] = 'something'
Gitlab.config.ldap['admin_group'] = ''
end
it "syncs ssh keys if enabled by configuration" do
access.stub sync_ssh_keys?: 'sshpublickey'
expect(access).to receive(:update_ssh_keys).once
after do
Gitlab.config.ldap['enabled'] = false
subject
end
it "syncs ssh keys if enabled by configuration" do
Gitlab.config.ldap['sync_ssh_keys'] = true
expect(access).to receive(:update_ssh_keys).with(user).once
it "does update group permissions with a group base configured" do
access.stub group_base: 'my-group-base'
expect(access).to receive(:update_ldap_group_links)
subject
end
it "does not update group permissions without a group base configured" do
Gitlab.config.ldap['group_base'] = ''
expect(access).not_to receive(:update_ldap_group_links).with(user)
access.stub group_base: ''
expect(access).not_to receive(:update_ldap_group_links)
subject
end
it "does update admin group permissions if admin group is configured" do
Gitlab.config.ldap['admin_group'] = 'NSA'
access.stub(:update_ldap_group_links)
expect(access).to receive(:update_admin_status).with(user)
access.stub admin_group: 'my-admin-group'
access.stub :update_ldap_group_links
expect(access).to receive(:update_admin_status)
subject
end
end
describe :update_ssh_keys do
let(:user_ldap) { create(:user, provider: 'ldap', extern_uid: "66049")}
let(:ssh_key) { 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCrSQHff6a1rMqBdHFt+FwIbytMZ+hJKN3KLkTtOWtSvNIriGhnTdn4rs+tjD/w+z+revytyWnMDM9dS7J8vQi006B16+hc9Xf82crqRoPRDnBytgAFFQY1G/55ql2zdfsC5yvpDOFzuwIJq5dNGsojS82t6HNmmKPq130fzsenFnj5v1pl3OJvk513oduUyKiZBGTroWTn7H/eOPtu7s9MD7pAdEjqYKFLeaKmyidiLmLqQlCRj3Tl2U9oyFg4PYNc0bL5FZJ/Z6t0Ds3i/a2RanQiKxrvgu3GSnUKMx7WIX373baL4jeM7cprRGiOY/1NcS+1cAjfJ8oaxQF/1dYj' }
let(:key_ldap) { LDAPKey.new(title: 'used to be a ldap key', key: ssh_key) }
let(:ssh_key_attribute_name) { 'sshpublickey' }
let(:entry) {
Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{ssh_key_attribute_name}: #{ssh_key}") }
before do
@old_value = Gitlab.config.ldap['sync_ssh_keys']
key_attribute_name = 'sshpublickey'
Gitlab.config.ldap['sync_ssh_keys'] = key_attribute_name
end
after do
Gitlab.config.ldap['sync_ssh_keys'] = @old_value
Gitlab::LDAP::Config.any_instance.stub(sync_ssh_keys: ssh_key_attribute_name)
access.stub sync_ssh_keys?: true
end
it "should add a SSH key if it is in LDAP but not in gitlab" do
entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{Gitlab.config.ldap['sync_ssh_keys']}: #{ssh_key}")
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{ssh_key_attribute_name}: #{ssh_key}")
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
expect(user_ldap.keys.size).to be(0)
access.update_ssh_keys(user_ldap)
user_ldap.reload
expect(user_ldap.keys.size).to be(1)
expect{ access.update_ssh_keys }.to change(user.keys, :count).from(0).to(1)
end
it "should add a SSH key and give it a proper name" do
entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{Gitlab.config.ldap['sync_ssh_keys']}: #{ssh_key}")
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{ssh_key_attribute_name}: #{ssh_key}")
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
access.update_ssh_keys(user_ldap)
expect(user_ldap.keys.last.title).to match(/LDAP/)
expect(user_ldap.keys.last.title).to match(/#{Gitlab.config.ldap['sync_ssh_keys']}/)
access.update_ssh_keys
expect(user.keys.last.title).to match(/LDAP/)
expect(user.keys.last.title).to match(/#{access.ldap_config.sync_ssh_keys}/)
end
it "should not add a SSH key if it is invalid" do
entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{Gitlab.config.ldap['sync_ssh_keys']}: I am not a valid key")
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{ssh_key_attribute_name}: I am not a valid key")
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
expect(user_ldap.keys.size).to be(0)
access.update_ssh_keys(user_ldap)
expect(user_ldap.keys.size).to be(0)
expect{ access.update_ssh_keys }.to_not change(user.keys, :count)
end
context 'user has at least one LDAPKey' do
before { user.keys.ldap.create key: ssh_key, title: 'to be removed' }
it "should remove a SSH key if it is no longer in LDAP" do
entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{Gitlab.config.ldap['sync_ssh_keys']}:\n")
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
key_ldap.save
user_ldap.keys << key_ldap
entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{ssh_key_attribute_name}:\n")
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
expect(user_ldap.keys.size).to be(1)
access.update_ssh_keys(user_ldap)
expect(user_ldap.keys.size).to be(0)
expect{ access.update_ssh_keys }.to change(user.keys, :count).from(1).to(0)
end
it "should remove a SSH key if the ldap attribute was removes" do
it "should remove a SSH key if the ldap attribute was removed" do
entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com")
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
key_ldap.save
user_ldap.keys << key_ldap
expect(user_ldap.keys.size).to be(1)
access.update_ssh_keys(user_ldap)
expect(user_ldap.keys.size).to be(0)
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') }
expect{ access.update_ssh_keys }.to change(user.keys, :count).from(1).to(0)
end
end
end
describe :update_user_email do
let(:user_ldap) { create(:user, provider: 'ldap', extern_uid: "66048")}
let(:entry) { Net::LDAP::Entry.new }
before do
access.stub ldap_user: Gitlab::LDAP::Person.new(entry, user.provider)
end
it "should not update email if email attribute is not set" do
entry = Net::LDAP::Entry.new
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
updated = access.update_email(user_ldap)
updated.should == false
expect{ access.update_email }.to_not change(user, :unconfirmed_email)
end
it "should not update the email if the user has the same email in GitLab and in LDAP" do
entry = Net::LDAP::Entry.new
entry['mail'] = [user_ldap.email]
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
updated = access.update_email(user_ldap)
updated.should == false
entry['mail'] = [user.email]
expect{ access.update_email }.to_not change(user, :unconfirmed_email)
end
it "should not update the email if the user has the same email GitLab and in LDAP, but with upper case in LDAP" do
entry = Net::LDAP::Entry.new
entry['mail'] = [user_ldap.email.upcase]
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
updated = access.update_email(user_ldap)
updated.should == false
entry['mail'] = [user.email.upcase]
expect{ access.update_email }.to_not change(user, :unconfirmed_email)
end
it "should update the email if the user email is different" do
entry = Net::LDAP::Entry.new
entry['mail'] = ["new_email@example.com"]
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
updated = access.update_email(user_ldap)
updated.should == true
expect{ access.update_email }.to change(user, :unconfirmed_email)
end
end
describe :update_admin_status do
let(:gitlab_user) { create(:user, provider: 'ldap', extern_uid: "admin2")}
let(:gitlab_admin) { create(:admin, provider: 'ldap', extern_uid: "admin2")}
before do
Gitlab.config.ldap['admin_group'] = "GLAdmins"
access.stub(admin_group: "GLAdmins")
ldap_user_entry = Net::LDAP::Entry.new
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(ldap_user_entry) }
Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(ldap_user_entry, user.provider) }
Gitlab::LDAP::Person.any_instance.stub(:uid) { 'admin2' }
end
it "should give admin privileges to an User" do
admin_group = Net::LDAP::Entry.from_single_ldif_string(
%Q{dn: cn=#{Gitlab.config.ldap['admin_group']},ou=groups,dc=bar,dc=com
cn: #{Gitlab.config.ldap['admin_group']}
%Q{dn: cn=#{access.admin_group},ou=groups,dc=bar,dc=com
cn: #{access.admin_group}
description: GitLab admins
gidnumber: 42
memberuid: admin1
......@@ -205,15 +174,15 @@ objectclass: top
objectclass: posixGroup
})
Gitlab::LDAP::Adapter.any_instance.stub(:group) { Gitlab::LDAP::Group.new(admin_group) }
expect(gitlab_user.admin?).to be false
access.update_admin_status(gitlab_user)
expect(gitlab_user.admin?).to be true
expect{ access.update_admin_status }.to change(user, :admin?).to(true)
end
it "should remove admin privileges from an User" do
user.update_attribute(:admin, true)
admin_group = Net::LDAP::Entry.from_single_ldif_string(
%Q{dn: cn=#{Gitlab.config.ldap['admin_group']},ou=groups,dc=bar,dc=com
cn: #{Gitlab.config.ldap['admin_group']}
%Q{dn: cn=#{access.admin_group},ou=groups,dc=bar,dc=com
cn: #{access.admin_group}
description: GitLab admins
gidnumber: 42
memberuid: admin1
......@@ -222,9 +191,7 @@ objectclass: top
objectclass: posixGroup
})
Gitlab::LDAP::Adapter.any_instance.stub(:group) { Gitlab::LDAP::Group.new(admin_group) }
expect(gitlab_admin.admin?).to be true
access.update_admin_status(gitlab_admin)
expect(gitlab_admin.admin?).to be false
expect{ access.update_admin_status }.to change(user, :admin?).to(false)
end
end
......@@ -235,17 +202,17 @@ objectclass: posixGroup
let(:gitlab_group_2) { create :group }
before do
access.stub(:get_ldap_user)
access.stub(cns_with_access: cns_with_access)
end
context "non existing access for group-1, allowed via ldap-group1 as MASTER" do
before do
gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER
gitlab_group_1.ldap_group_links.create({
cn: 'ldap-group1', group_access: Gitlab::Access::MASTER })
end
it "gives the user master access for group 1" do
access.update_ldap_group_links(user)
access.update_ldap_group_links
expect( gitlab_group_1.has_master?(user) ).to be_true
end
end
......@@ -257,7 +224,7 @@ objectclass: posixGroup
end
it "upgrades the users access to master for group 1" do
expect { access.update_ldap_group_links(user) }.to \
expect { access.update_ldap_group_links }.to \
change{ gitlab_group_1.has_master?(user) }.from(false).to(true)
end
end
......@@ -269,7 +236,7 @@ objectclass: posixGroup
end
it "keeps the users master access for group 1" do
expect { access.update_ldap_group_links(user) }.not_to \
expect { access.update_ldap_group_links }.not_to \
change{ gitlab_group_1.has_master?(user) }
end
end
......@@ -282,7 +249,7 @@ objectclass: posixGroup
end
it "removes user from gitlab_group_1" do
expect { access.update_ldap_group_links(user) }.to \
expect { access.update_ldap_group_links }.to \
change{ gitlab_group_1.members.where(user_id: user).any? }.from(true).to(false)
end
end
......@@ -348,13 +315,17 @@ objectclass: posixGroup
Gitlab::LDAP::Group.new(ldap_group_response_2)
]
end
let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new) }
let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new, user.provider) }
before { ldap_user.stub(:uid) { 'user42' } }
before do
access.stub(ldap_user: ldap_user)
ldap_user.stub(:uid) { 'user42' }
end
it "only returns ldap cns to which the user has access" do
access.stub(ldap_groups: ldap_groups)
expect(access.cns_with_access(ldap_user)).to eql ['group1']
expect(access.cns_with_access).to eql ['group1']
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7