diff --git a/CHANGELOG b/CHANGELOG index 0369b321e00e282dae97e7d3836acc4ac4139e8d..b9399e4acc41f7466ca641f512ded30eb3bb0483 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/config/gitlab.yml.example b/config/gitlab.yml.example index 14c822b0ce4b0fe7d49824e6aeaa74f5bc240526..ee8f71c22037e7477d45d7cd0c8ae31993b9115a 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -496,11 +496,6 @@ production: &base # If you use non-standard ssh port you need to specify it # ssh_port: 22 - # git-annex support (EE only) - # If this setting is set to true, the same setting in config.yml of - # gitlab-shell needs to be set to true - # git_annex_enabled: false - ## Git settings # CAUTION! # Use the default values unless you really know what you are doing diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index b9f0c3fd4ab92a03b266ac1d9a7513f1f3ae36f7..1f97862b79b210ee53db6751ef94742f729fe5e1 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -342,7 +342,6 @@ def on_standard_port?(config) Settings.gitlab_shell['ssh_user'] ||= Settings.gitlab.user Settings.gitlab_shell['owner_group'] ||= Settings.gitlab.user Settings.gitlab_shell['ssh_path_prefix'] ||= Settings.send(:build_gitlab_shell_ssh_path_prefix) -Settings.gitlab_shell['git_annex_enabled'] ||= false # # Backup diff --git a/doc/integration/ldap.md b/doc/integration/ldap.md index fc9441d112f76c38bc618183038d025f057e116d..026e327baa9504286b7bba1dcbf071efec2434da 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) diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index fc020be52773dca08b6bebb680b72a91e676933a..b578f0841b870d0307215d47626872e4b1480464 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -85,8 +85,6 @@ def download_access_check end def push_access_check(changes) - return build_status_object(true) if git_annex_branch_sync?(changes) - if user user_push_access_check(changes) elsif deploy_key @@ -327,23 +325,5 @@ def git_annex_access_check(project, changes) build_status_object(false, "You don't have permission") end end - - def git_annex_branch_sync?(changes) - return false unless Gitlab.config.gitlab_shell.git_annex_enabled - return false if changes.blank? - - changes = changes.lines if changes.kind_of?(String) - - # Iterate over all changes to find if user allowed all of them to be applied - # 0000000000000000000000000000000000000000 3073696294ddd52e9e6b6fc3f429109cac24626f refs/heads/synced/git-annex - # 0000000000000000000000000000000000000000 65be9df0e995d36977e6d76fc5801b7145ce19c9 refs/heads/synced/master - changes.map(&:strip).reject(&:blank?).each do |change| - unless change.end_with?("refs/heads/synced/git-annex") || change.include?("refs/heads/synced/") - return false - end - end - - true - end end end diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index 1f4e95006cb2e6002cdc5e160c69ad311bddbb2c..7b036a40853861e1798176ac8d0f7251ed719ef1 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/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index ff4a1c4ecebe1d98d350804d375f81d165c34be0..c462a70230ccb786954968a295e091ae9459b306 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -5,10 +5,6 @@ let(:project) { create(:project) } let(:user) { create(:user) } let(:actor) { user } - let(:git_annex_changes) do - ["6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/synced/git-annex", - "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/synced/named-branch"] - end describe 'can_push_to_branch?' do describe 'push to none protected branch' do @@ -248,65 +244,6 @@ def self.updated_permissions_matrix end end end - - context "when using git annex" do - before { project.team << [user, :master] } - - describe 'and git hooks unset' do - describe 'git annex enabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(true) } - - it { expect(access.push_access_check(git_annex_changes)).to be_allowed } - end - - describe 'git annex disabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(false) } - - it { expect(access.push_access_check(git_annex_changes)).to be_allowed } - end - end - - describe 'and git hooks set' do - before { project.create_git_hook } - - describe 'check commit author email' do - before do - project.git_hook.update(author_email_regex: "@only.com") - end - - describe 'git annex enabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(true) } - - it { expect(access.push_access_check(git_annex_changes)).to be_allowed } - end - - describe 'git annex disabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(false) } - - it { expect(access.push_access_check(git_annex_changes)).to_not be_allowed } - end - end - - describe 'check commit author email' do - before do - allow_any_instance_of(Gitlab::Git::Blob).to receive(:size).and_return(5.megabytes.to_i) - project.git_hook.update(max_file_size: 2) - end - - describe 'git annex enabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(true) } - - it { expect(access.push_access_check(git_annex_changes)).to be_allowed } - end - - describe 'git annex disabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(false) } - - it { expect(access.push_access_check(git_annex_changes)).to_not be_allowed } - end - end - end - end end describe "git_hook_check" do diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 1dbace9696e3f0a11c51512707958234d505f7cf..8045d1107a02ca5f04a6dbede4d2e3c10983292d 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