From f979b78a9546584bb7146bd36123162246088ddd Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Mon, 18 Jan 2016 15:48:50 -0600 Subject: [PATCH 1/2] Remove user from groups when they are removed from LDAP --- lib/gitlab/ldap/access.rb | 28 ++++----- spec/lib/gitlab/ldap/access_spec.rb | 90 +++++++++++++++++++++-------- 2 files changed, 78 insertions(+), 40 deletions(-) diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index 1f4e95006cb2e6..b719689a8d1650 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -16,9 +16,11 @@ def self.open(user, &block) def self.allowed?(user) self.open(user) do |access| + # Whether user is allowed, or not, we should update + # permissions to keep things clean + access.update_permissions if access.allowed? - access.update_permissions - access.update_email + access.update_user user.last_credential_check_at = Time.now user.save true @@ -67,22 +69,15 @@ def ldap_user @ldap_user ||= Gitlab::LDAP::Person.find_by_dn(user.ldap_identity.extern_uid, adapter) end - def update_permissions - if sync_ssh_keys? - update_ssh_keys - end - + def update_user + update_email + update_ssh_keys if sync_ssh_keys? update_kerberos_identity if import_kerberos_identities? + end - # Skip updating group permissions - # if instance does not use group_base setting - return true unless group_base.present? - - update_ldap_group_links - - if admin_group.present? - update_admin_status - end + def update_permissions + update_ldap_group_links if group_base.present? + update_admin_status if admin_group.present? end # Update user ssh keys if they changed in LDAP @@ -191,6 +186,7 @@ def ldap_groups # returns a collection of cn strings to which the user has access def cns_with_access + return [] unless ldap_user.present? @ldap_groups_with_access ||= ldap_groups.select do |ldap_group| ldap_group.has_member?(ldap_user) end.map(&:cn) diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 1dbace9696e3f0..ee6141fceebeaa 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -4,7 +4,7 @@ let(:access) { Gitlab::LDAP::Access.new user } let(:user) { create(:omniauth_user) } - describe :allowed? do + describe '#allowed?' do subject { access.allowed? } context 'when the user cannot be found' do @@ -40,7 +40,7 @@ end end - context 'and has no disabled flag in active diretory' do + context 'and has no disabled flag in active directory' do before do allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false) end @@ -71,7 +71,7 @@ end end - context 'withoud ActiveDirectory enabled' do + context 'without ActiveDirectory enabled' do before do allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true) allow_any_instance_of(Gitlab::LDAP::Config).to receive(:active_directory).and_return(false) @@ -82,41 +82,67 @@ end end - describe :update_permissions do - subject { access.update_permissions } + describe '#update_user' do + subject { access.update_user } + let(:entry) do + Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com") + end + before do + allow(access).to( + receive_messages( + ldap_user: Gitlab::LDAP::Person.new(entry, user.ldap_identity.provider) + ) + ) + end + + it 'updates email address' do + expect(access).to receive(:update_email).once + + subject + end - it "syncs ssh keys if enabled by configuration" do - allow(access).to receive_messages(group_base: '', sync_ssh_keys?: 'sshpublickey', import_kerberos_identities?: false) + 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 subject end - it "does update group permissions with a group base configured" do - allow(access).to receive_messages(group_base: 'my-group-base', sync_ssh_keys?: false, import_kerberos_identities?: false) + it 'update_kerberos_identity' do + allow(access).to receive_messages(import_kerberos_identities?: true) + expect(access).to receive(:update_kerberos_identity).once + + subject + end + end + + describe '#update_permissions' do + subject { access.update_permissions } + + it 'does update group permissions with a group base configured' do + allow(access).to receive_messages(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 - allow(access).to receive_messages(group_base: '', sync_ssh_keys?: false, import_kerberos_identities?: false) + it 'does not update group permissions without a group base configured' do + allow(access).to receive_messages(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 - allow(access).to receive_messages(admin_group: 'my-admin-group', update_ldap_group_links: nil, sync_ssh_keys?: false, import_kerberos_identities?: false) + it 'does update admin group permissions if admin group is configured' do + allow(access).to receive_messages(admin_group: 'my-admin-group') expect(access).to receive(:update_admin_status) subject end - it "does update Kerberos identities if Kerberos is enabled and the LDAP server is Active Directory" do - allow(access).to receive_messages(group_base: '', sync_ssh_keys?: false, import_kerberos_identities?: true) - - expect(access).to receive(:update_kerberos_identity) + it 'does not update admin status when admin group is not configured' do + allow(access).to receive_messages(admin_group: '') + expect(access).not_to receive(:update_admin_status) subject end @@ -451,17 +477,33 @@ ] end - let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new, user.ldap_identity.provider) } - before do - allow(access).to receive_messages(ldap_user: ldap_user) - allow(ldap_user).to receive(:uid) { 'user1' } allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } + allow(access).to receive_messages(ldap_groups: ldap_groups) end - it "only returns ldap cns to which the user has access" do - allow(access).to receive_messages(ldap_groups: ldap_groups) - expect(access.cns_with_access).to eql ['group1'] + context 'when the LDAP user exists' do + let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new, user.ldap_identity.provider) } + + before do + allow(access).to receive_messages(ldap_user: ldap_user) + allow(ldap_user).to receive(:uid) { 'user1' } + end + + it 'only returns ldap cns to which the user has access' do + expect(access.cns_with_access).to eq(['group1']) + end + end + + context 'when the LADP user does not exist' do + let(:ldap_user) { nil } + before do + allow(access).to receive_messages(ldap_user: ldap_user) + end + + it 'returns an empty array' do + expect(access.cns_with_access).to eq([]) + end end end end -- GitLab From a024312db9b354fa2701e6ea83a2c2397836623f Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Thu, 21 Jan 2016 16:22:19 -0800 Subject: [PATCH 2/2] Add Changelog [ci skip] --- CHANGELOG | 2 ++ doc/workflow/groups.md | 32 ++++++++++++++++---------------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 0369b321e00e28..014c9642567436 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -12,6 +12,7 @@ v 8.4.0 (unreleased) - Add user's last used IP addresses to admin page (Stan Hu) - Add housekeeping function to project settings page - The default GitLab logo now acts as a loading indicator + - LDAP group sync: Remove user from group when they are removed from LDAP - Fix caching issue where build status was not updating in project dashboard (Stan Hu) - Accept 2xx status codes for successful Web hook triggers (Stan Hu) - Fix missing date of month in network graph when commits span a month (Stan Hu) @@ -27,6 +28,7 @@ v 8.4.0 (unreleased) - Add API support for looking up a user by username (Stan Hu) - Add project permissions to all project API endpoints (Stan Hu) - Link to milestone in "Milestone changed" system note + - LDAP Group Sync: Allow group role downgradegit - Only allow group/project members to mention `@all` - Expose Git's version in the admin area (Trey Davis) - Add "Frequently used" category to emoji picker diff --git a/doc/workflow/groups.md b/doc/workflow/groups.md index ad795d72471038..abf233c5a7a177 100644 --- a/doc/workflow/groups.md +++ b/doc/workflow/groups.md @@ -70,28 +70,28 @@ gitlab_rails['gitlab_default_can_create_group'] = false # line in /home/git/gitlab/config/gitlab.yml ``` -## Lock project membership to members of the group +## Lock project membership to members of this group -In GitLab Enterprise Edition it is possible to lock membership in project to the level of members in group. +In GitLab Enterprise Edition it is possible to lock membership in project to the +level of members in group. -This allows group owner to lock down any new project membership to any of the projects within the group allowing tighter control over project membership. +This allows group owner to lock down any new project membership to any of the +projects within the group allowing tighter control over project membership. -To enable this feature, navigate to group settings page, select `Member lock` and `Save group`. +To enable this feature, navigate to group settings page, select `Member lock` +and `Save group`. ![Checkbox for membership lock](groups/membership_lock.png) -This will disable the option for all users who previously had permissions to operate project memberships so no new users can be added. Furthermore, any request to add new user to project through API will not be possible. +This will disable the option for all users who previously had permissions to +operate project memberships so no new users can be added. Furthermore, any +request to add new user to project through API will not be possible. -## Namespaces in groups +## Prevent projects in this group from sharing a project with another group -By default, groups only get 20 namespaces at a time because the API results are paginated. +In GitLab Enterprise it is possible to prevent projects in a group from sharing +a project with another group. This allows for tighter control over project +access. -To get more (up to 100), pass the following as an argument to the API call: -``` -/groups?per_page=100 -``` - -And to switch pages add: -``` -/groups?per_page=100&page=2 -``` +To enable this feature, navigate to the group settings page. Select `Share with +group lock` and save the group. -- GitLab