From f9fd9372930a7819186112dfaa32ed6ed1eea6c9 Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Mon, 18 Jan 2016 14:19:05 -0600 Subject: [PATCH 1/2] Downgrade group member role if LDAP dictates --- lib/gitlab/ldap/access.rb | 14 ++------------ spec/lib/gitlab/ldap/access_spec.rb | 10 +++++----- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index 1f4e95006cb2e6..7b036a40853861 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -164,7 +164,7 @@ def update_admin_status end end - # Loop throug all ldap conneted groups, and update the users link with it + # Loop through all ldap connected groups, and update the users link with it # # We documented what sort of queries an LDAP server can expect from # GitLab EE in doc/integration/ldap.md. Please remember to update that @@ -174,7 +174,7 @@ def update_ldap_group_links 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), skip_notification: true) + group.add_users([user.id], active_group_links.maximum(:group_access), skip_notification: true) elsif group.last_owner?(user) logger.warn "#{self.class.name}: LDAP group sync cannot remove #{user.name} (#{user.id}) from group #{group.name} (#{group.id}) as this is the group's last owner" else @@ -221,16 +221,6 @@ def gitlab_groups_with_ldap_link where(ldap_group_links: { provider: provider }) end - # Get the group_access for a give user. - # Always respect the current level, never downgrade it. - def fetch_group_access(group, user, active_group_links) - current_access_level = group.group_members.where(user_id: user).maximum(:access_level) - max_group_access_level = active_group_links.maximum(:group_access) - - # TODO: Test if nil value of current_access_level in handled properly - [current_access_level, max_group_access_level].compact.max - end - def logger Rails.logger end diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 1dbace9696e3f0..8045d1107a02ca 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -328,19 +328,19 @@ end end - context "existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER" do + context 'existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER' do before do gitlab_group_1.group_members.masters.create(user_id: user.id) gitlab_group_1.ldap_group_links.create({ cn: 'ldap-group1', group_access: Gitlab::Access::DEVELOPER, provider: 'ldapmain' }) end - it "keeps the users master access for group 1" do - expect { access.update_ldap_group_links }.not_to \ - change{ gitlab_group_1.has_master?(user) } + it 'downgrades the users access' do + expect { access.update_ldap_group_links }.to \ + change{ gitlab_group_1.has_master?(user) }.from(true).to(false) end - it "doesn't send a notification email" do + it 'does not send a notification email' do expect { access.update_ldap_group_links }.not_to \ change { ActionMailer::Base.deliveries } end -- GitLab From 9965e743c9cff39d7d2bca1319b9cb888505a48d Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Wed, 20 Jan 2016 20:38:52 -0600 Subject: [PATCH 2/2] Update LDAP group sync documentation and add changelog [ci skip] --- CHANGELOG | 1 + doc/integration/ldap.md | 27 +++++++++++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 0369b321e00e28..b9399e4acc41f7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -27,6 +27,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/integration/ldap.md b/doc/integration/ldap.md index fc9441d112f76c..026e327baa9504 100644 --- a/doc/integration/ldap.md +++ b/doc/integration/ldap.md @@ -156,18 +156,22 @@ If multiple LDAP email attributes are present, e.g. `mail: foo@bar.com` and `ema ## LDAP group synchronization (GitLab Enterprise Edition) -LDAP group synchronization in GitLab Enterprise Edition allows you to synchronize the members of a GitLab group with one or more LDAP groups. +LDAP group synchronization in GitLab Enterprise Edition allows you to +synchronize the members of a GitLab group with one or more LDAP groups. ### Setting up LDAP group synchronization -Before enabling group synchronization, you need to make sure that the `group_base` field is set in your LDAP settings on -your `gitlab.rb` or `gitlab.yml` file. This setting will tell GitLab where to look for groups within your LDAP server. +Before enabling group synchronization, you need to make sure that the +`group_base` field is set in your LDAP settings on your `gitlab.rb` or +`gitlab.yml` file. This setting will tell GitLab where to look for groups +within your LDAP server. ``` group_base: 'OU=groups,DC=example,DC=com' ``` -Suppose we want to synchronize the GitLab group 'example group' with the LDAP group 'Engineering'. +Suppose we want to synchronize the GitLab group 'example group' with the LDAP +group 'Engineering'. 1. As an owner, go to the group settings page for 'example group'. @@ -179,18 +183,25 @@ As an admin you can also go to the group edit page in the admin area. 2. Enter 'Engineering' as the LDAP Common Name (CN) in the 'LDAP Group cn' field. -3. Enter a default group access level in the 'LDAP Access' field; let's say Developer. +3. Enter a default group access level in the 'LDAP Access' field; let's say +Developer. ![LDAP group settings filled in](ldap/select_group_cn_engineering.png) 4. Click 'Add synchronization' to add the new LDAP group link. -Now every time a member of the 'Engineering' LDAP group signs in, they automatically become a Developer-level member of the 'example group' GitLab group. Users who are already signed in will see the change in membership after up to one hour. +Now every time a member of the 'Engineering' LDAP group signs in, they +automatically become a Developer-level member of the 'example group' GitLab +group. Users who are already signed in will see the change in membership after +up to one hour. ### Synchronizing with more than one LDAP group (GitLab EE 7.3 and newer) -If you want to add the members of LDAP group to your GitLab group you can add an additional LDAP group link. -If you have two LDAP group links, e.g. 'cn=Engineering' at level 'Developer' and 'cn=QA' at level 'Reporter', and user Jane belongs to both the 'Engineering' and 'QA' LDAP groups, she will get the _highest_ access level of the two, namely 'Developer'. +If you have two LDAP group links, and a user belongs to both LDAP groups, they +will receive the _highest_ access level of the two. For example, suppose you +have configured group sync for the 'Engineering' group at level 'Developer' and +'QA' group at level 'Reporter'. User 'Jane' belongs to both the 'Engineering' and +'QA' LDAP groups so she will receive the 'Developer' role. ![Two linked LDAP groups](ldap/two_linked_ldap_groups.png) -- GitLab