Commit 10f1a1f2 authored by Bob Van Landuyt's avatar Bob Van Landuyt Committed by Drew Blessing

Add a user's memberships when logging in

parent 1552a182
......@@ -34,6 +34,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
prompt_for_two_factor(@user)
else
log_audit_event(@user, with: :ldap)
flash[:notice] = 'LDAP sync in progress. This could take a few minutes. '\
'Refresh the page to see the changes.'
sign_in_and_redirect(@user)
end
else
......
class LdapAllGroupsSyncWorker
include Sidekiq::Worker
include CronjobQueue
def perform
logger.info 'Started LDAP group sync'
EE::Gitlab::LDAP::Sync::Groups.execute
logger.info 'Finished LDAP group sync'
end
end
class LdapGroupSyncWorker
include Sidekiq::Worker
include CronjobQueue
include DedicatedSidekiqQueue
def perform(group_id = nil)
if group_id
group = Group.find_by(id: group_id)
unless group
logger.warn "Could not find group #{group_id} for LDAP group sync"
return
def perform(group_ids, provider = nil)
groups = Group.where(id: Array(group_ids))
if provider
EE::Gitlab::LDAP::Sync::Proxy.open(provider) do |proxy|
sync_groups(groups, proxy: proxy)
end
else
sync_groups(groups)
end
end
logger.info "Started LDAP group sync for group #{group.name} (#{group.id})"
EE::Gitlab::LDAP::Sync::Group.execute_all_providers(group)
logger.info "Finished LDAP group sync for group #{group.name} (#{group.id})"
def sync_groups(groups, proxy: nil)
groups.each { |group| sync_group(group, proxy: proxy) }
end
def sync_group(group, proxy: nil)
logger.info "Started LDAP group sync for group #{group.name} (#{group.id})"
if proxy
EE::Gitlab::LDAP::Sync::Group.execute(group, proxy)
else
logger.info 'Started LDAP group sync'
EE::Gitlab::LDAP::Sync::Groups.execute
logger.info 'Finished LDAP group sync'
EE::Gitlab::LDAP::Sync::Group.execute_all_providers(group)
end
logger.info "Finished LDAP group sync for group #{group.name} (#{group.id})"
end
end
---
title: Add a user's memberships when logging in through LDAP
merge_request: 1819
author:
......@@ -392,7 +392,7 @@ Settings.cron_jobs['ldap_sync_worker']['cron'] ||= '30 1 * * *'
Settings.cron_jobs['ldap_sync_worker']['job_class'] = 'LdapSyncWorker'
Settings.cron_jobs['ldap_group_sync_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['ldap_group_sync_worker']['cron'] ||= '0 * * * *'
Settings.cron_jobs['ldap_group_sync_worker']['job_class'] = 'LdapGroupSyncWorker'
Settings.cron_jobs['ldap_group_sync_worker']['job_class'] = 'LdapAllGroupsSyncWorker'
Settings.cron_jobs['geo_bulk_notify_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['geo_bulk_notify_worker']['cron'] ||= '*/10 * * * * *'
Settings.cron_jobs['geo_bulk_notify_worker']['job_class'] ||= 'GeoBulkNotifyWorker'
......
......@@ -55,6 +55,7 @@
- [update_user_activity, 1]
- [propagate_service_template, 1]
# EE specific queues
- [ldap_group_sync, 2]
- [geo, 1]
- [project_mirror, 1]
- [project_update_repository_storage, 1]
......
......@@ -29,6 +29,11 @@ The process will also update the following user information:
## Group Sync
If your LDAP supports the `memberof` property, GitLab will add the user to any
new groups they might be added to when the user logs in. That way they don't need
to wait for the hourly sync to be granted access to the groups that they are in
in LDAP.
If `group_base` is set in LDAP configuration, a group sync process will run
every hour, on the hour. This allows GitLab group membership to be automatically
updated based on LDAP group members.
......
......@@ -89,6 +89,7 @@ module Gitlab
def update_user
update_email
update_memberships
update_ssh_keys if sync_ssh_keys?
update_kerberos_identity if import_kerberos_identities?
end
......@@ -162,6 +163,16 @@ module Gitlab
ldap_config.active_directory && (Gitlab.config.kerberos.enabled || AuthHelper.kerberos_enabled? )
end
def update_memberships
return if ldap_user.nil? || ldap_user.group_cns.empty?
group_ids = LdapGroupLink.where(cn: ldap_user.group_cns, provider: provider)
.distinct(:group_id)
.pluck(:group_id)
LdapGroupSyncWorker.perform_async(group_ids, provider) if group_ids.any?
end
private
def logger
......
......@@ -107,7 +107,7 @@ module Gitlab
end
def user_attributes
%W(#{config.uid} cn mail dn)
%W(#{config.uid} cn mail dn memberof)
end
end
end
......
......@@ -45,6 +45,22 @@ module Gitlab
attribute_value(:email)
end
def memberof
return [] unless entry.attribute_names.include?(:memberof)
entry.memberof
end
def group_cns
memberof.map { |memberof_value| cn_from_memberof(memberof_value) }
end
def cn_from_memberof(memberof)
# Only get the first CN value of the string, that's the one that contains
# the group name
memberof.match(/(?:cn=([\w\s]+))/i)&.captures&.first
end
delegate :dn, to: :entry
private
......
......@@ -43,7 +43,7 @@ describe Gitlab::LDAP::Adapter, lib: true do
describe '#user_attributes' do
it 'appends EE-specific attributes' do
stub_ldap_config(uid: 'uid', sync_ssh_keys: 'sshPublicKey')
expect(adapter.user_attributes).to match_array(%w(uid dn cn mail sshPublicKey))
expect(adapter.user_attributes).to match_array(%w(uid dn cn mail sshPublicKey memberof))
end
end
end
......@@ -65,10 +65,11 @@ describe EE::Gitlab::LDAP::Sync::Group, lib: true do
end
before do
allow(Gitlab::LDAP::Config)
.to receive(:providers).and_return(%w(main secondary))
allow(EE::Gitlab::LDAP::Sync::Proxy)
.to receive(:open).and_yield(double('proxy').as_null_object)
stub_ldap_config(providers: %w[main secundary])
adapter = ldap_adapter('main')
proxy = proxy(adapter, 'main')
allow(EE::Gitlab::LDAP::Sync::Proxy).to receive(:open).and_yield(proxy)
end
let(:group) do
......
require 'spec_helper'
describe Gitlab::LDAP::Access, lib: true do
include LdapHelpers
let(:access) { Gitlab::LDAP::Access.new user }
let(:user) { create(:omniauth_user) }
......@@ -173,6 +174,12 @@ describe Gitlab::LDAP::Access, lib: true do
subject
end
it 'updates the group memberships' do
expect(access).to receive(:update_memberships).once
subject
end
it 'syncs ssh keys if enabled by configuration' do
allow(access).to receive_messages(group_base: '', sync_ssh_keys?: true)
expect(access).to receive(:update_ssh_keys).once
......@@ -305,4 +312,50 @@ describe Gitlab::LDAP::Access, lib: true do
expect{ access.update_email }.to change(user, :email)
end
end
describe '#update_memberships' do
let(:provider) { user.ldap_identity.provider }
let(:entry) { ldap_user_entry(user.ldap_identity.extern_uid) }
let(:person_with_memberof) do
entry['memberof'] = ['CN=Group1,CN=Users,DC=The dc,DC=com',
'CN=Group2,CN=Builtin,DC=The dc,DC=com']
Gitlab::LDAP::Person.new(entry, provider)
end
it 'triggers a sync for all groups found in `memberof`' do
group_link_1 = create(:ldap_group_link, cn: 'Group1', provider: provider)
group_link_2 = create(:ldap_group_link, cn: 'Group2', provider: provider)
group_ids = [group_link_1, group_link_2].map(&:group_id)
allow(access).to receive(:ldap_user).and_return(person_with_memberof)
expect(LdapGroupSyncWorker).to receive(:perform_async)
.with(group_ids, provider)
access.update_memberships
end
it "doesn't continue when there is no `memberOf` param" do
allow(access).to receive(:ldap_user)
.and_return(Gitlab::LDAP::Person.new(entry, provider))
expect(LdapGroupLink).not_to receive(:where)
expect(LdapGroupSyncWorker).not_to receive(:perform_async)
access.update_memberships
end
it "doesn't trigger a sync when there are no links for the provider" do
_another_provider = create(:ldap_group_link,
cn: 'Group1',
provider: 'not-this-ldap')
allow(access).to receive(:ldap_user).and_return(person_with_memberof)
expect(LdapGroupSyncWorker).not_to receive(:perform_async)
access.update_memberships
end
end
end
......@@ -5,6 +5,11 @@ describe Gitlab::LDAP::Adapter, lib: true do
let(:ldap) { double(:ldap) }
let(:adapter) { ldap_adapter('ldapmain', ldap) }
let(:default_user_search_attributes) { }
def user_search_attributes(id_name)
[id_name, 'cn', 'mail', 'dn', 'memberof']
end
describe '#users' do
before do
......@@ -16,7 +21,7 @@ describe Gitlab::LDAP::Adapter, lib: true do
expect(adapter).to receive(:ldap_search) do |arg|
expect(arg[:filter].to_s).to eq('(uid=johndoe)')
expect(arg[:base]).to eq('dc=example,dc=com')
expect(arg[:attributes]).to match(%w{uid cn mail dn})
expect(arg[:attributes]).to match(user_search_attributes('uid'))
end.and_return({})
adapter.users('uid', 'johndoe')
......@@ -26,7 +31,7 @@ describe Gitlab::LDAP::Adapter, lib: true do
expect(adapter).to receive(:ldap_search).with(
base: 'uid=johndoe,ou=users,dc=example,dc=com',
scope: Net::LDAP::SearchScope_BaseObject,
attributes: %w{uid cn mail dn},
attributes: user_search_attributes('uid'),
filter: nil
).and_return({})
......@@ -63,7 +68,7 @@ describe Gitlab::LDAP::Adapter, lib: true do
it 'uses the right uid attribute when non-default' do
stub_ldap_config(uid: 'sAMAccountName')
expect(adapter).to receive(:ldap_search).with(
hash_including(attributes: %w{sAMAccountName cn mail dn})
hash_including(attributes: user_search_attributes('sAMAccountName'))
).and_return({})
adapter.users('sAMAccountName', 'johndoe')
......
......@@ -43,4 +43,51 @@ describe Gitlab::LDAP::Person do
expect(person.email).to eq([user_principal_name])
end
end
describe '#memberof' do
it 'returns an empty array if the field was not present' do
person = described_class.new(entry, 'ldapmain')
expect(person.memberof).to eq([])
end
it 'returns the values of `memberof` if the field was present' do
example_memberof = ['CN=Group Policy Creator Owners,CN=Users,DC=Vosmaer,DC=com',
'CN=Domain Admins,CN=Users,DC=Vosmaer,DC=com',
'CN=Enterprise Admins,CN=Users,DC=Vosmaer,DC=com',
'CN=Schema Admins,CN=Users,DC=Vosmaer,DC=com',
'CN=Administrators,CN=Builtin,DC=Vosmaer,DC=com']
entry['memberof'] = example_memberof
person = described_class.new(entry, 'ldapmain')
expect(person.memberof).to eq(example_memberof)
end
end
describe '#cn_from_memberof' do
it 'gets the group cn from the memberof value' do
person = described_class.new(entry, 'ldapmain')
expect(person.cn_from_memberof('cN=Group Policy Creator Owners,CN=Users,DC=Vosmaer,DC=com'))
.to eq('Group Policy Creator Owners')
end
it "doesn't break when there is no CN property" do
person = described_class.new(entry, 'ldapmain')
expect(person.cn_from_memberof('DC=Vosmaer,DC=com'))
.to be_nil
end
end
describe '#group_cns' do
it 'returns only CNs from the memberof values' do
example_memberof = ['CN=Group Policy Creator Owners,CN=Users,DC=Vosmaer,DC=com',
'CN=Administrators,CN=Builtin,DC=Vosmaer,DC=com']
entry['memberof'] = example_memberof
person = described_class.new(entry, 'ldapmain')
expect(person.group_cns).to eq(['Group Policy Creator Owners', 'Administrators'])
end
end
end
require 'spec_helper'
describe LdapAllGroupsSyncWorker do
let(:subject) { described_class.new }
before do
allow(Sidekiq.logger).to receive(:info)
end
describe '#perform' do
it 'syncs all groups when group_id is nil' do
expect(EE::Gitlab::LDAP::Sync::Groups).to receive(:execute)
subject.perform
end
end
end
require 'spec_helper'
describe LdapGroupSyncWorker do
include LdapHelpers
let(:subject) { described_class.new }
let(:group) { create(:group) }
def expect_fake_proxy(provider)
fake = double
expect(EE::Gitlab::LDAP::Sync::Proxy)
.to receive(:open).with(provider).and_yield(fake)
fake
end
before do
allow(Sidekiq.logger).to receive(:info)
end
describe '#perform' do
it 'syncs all groups when group_id is nil' do
expect(EE::Gitlab::LDAP::Sync::Groups).to receive(:execute)
it 'syncs a single group when group_id is present' do
expect(subject).to receive(:sync_groups).with([group])
described_class.new.perform
subject.perform(group.id)
end
it 'syncs a single group when group_id is present' do
group = create(:group)
it 'creates a proxy for syncing a single provider' do
fake_proxy = expect_fake_proxy('the-provider')
expect(subject).to receive(:sync_groups).with([group], proxy: fake_proxy)
subject.perform(group.id, 'the-provider')
end
end
describe '#sync_groups' do
it 'syncs a group when it was found without a proxy' do
expect(subject).to receive(:sync_group).with(group, proxy: nil)
subject.sync_groups([group])
end
it 'syncs with an existing proxy when one was given' do
fake_proxy = double('proxy')
expect(subject).to receive(:sync_group).with(group, proxy: fake_proxy)
subject.sync_groups([group], proxy: fake_proxy)
end
end
describe '#sync_group' do
it 'syncs a single provider when a provider was given' do
proxy = EE::Gitlab::LDAP::Sync::Proxy.new('ldapmain', ldap_adapter)
expect(EE::Gitlab::LDAP::Sync::Group)
.to receive(:execute_all_providers).with(group)
expect(EE::Gitlab::LDAP::Sync::Group).to receive(:execute)
.with(group, proxy)
described_class.new.perform(group.id)
subject.sync_group(group, proxy: proxy)
end
it 'logs an error when group cannot be found' do
expect(EE::Gitlab::LDAP::Sync::Group).not_to receive(:execute_all_providers)
expect(Sidekiq.logger)
.to receive(:warn).with('Could not find group 9999 for LDAP group sync')
it 'syncs all providers when no proxy was given' do
expect(EE::Gitlab::LDAP::Sync::Group).to receive(:execute_all_providers)
.with(group)
described_class.new.perform(9999)
subject.sync_group(group)
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