Merge branch '4471-member-and-group-lock-fix-to-master' into 'master'

Merge branch '4471-member-and-group-lock-enabled-allows-project-to-be-shared-10-5' into 'master'

See merge request gitlab/gitlab-ee!607

app/controllers/projects/group_links_controller.rb

@@ -2,6 +2,7 @@ class Projects::GroupLinksController < Projects::ApplicationController
   layout 'project_settings'
   before_action :authorize_admin_project!
   before_action :authorize_admin_project_member!, only: [:update]
+  before_action :authorize_group_share!, only: [:create]
 
   def index
     redirect_to namespace_project_settings_members_path
@@ -42,6 +43,10 @@ class Projects::GroupLinksController < Projects::ApplicationController
 
   protected
 
+  def authorize_group_share!
+    access_denied! unless project.allowed_to_share_with_group?
+  end
+
   def group_link_params
     params.require(:group_link).permit(:group_access, :expires_at)
   end

app/helpers/projects_helper.rb

@@ -450,6 +450,10 @@ module ProjectsHelper
     end
   end
 
+  def project_can_be_shared?
+    !membership_locked? || @project.allowed_to_share_with_group?
+  end
+
   def membership_locked?
     if @project.group && @project.group.membership_lock
       true
@@ -458,6 +462,24 @@ module ProjectsHelper
     end
   end
 
+  def share_project_description
+    share_with_group   = @project.allowed_to_share_with_group?
+    share_with_members = !membership_locked?
+    project_name       = content_tag(:strong, @project.name)
+    member_message     = "You can add a new member to #{project_name}"
+
+    description =
+      if share_with_group && share_with_members
+        "#{member_message} or share it with another group."
+      elsif share_with_group
+        "You can share #{project_name} with another group."
+      elsif share_with_members
+        "#{member_message}."
+      end
+
+    description.to_s.html_safe
+  end
+
   def readme_cache_key
     sha = @project.commit.try(:sha) || 'nil'
     [@project.full_path, sha, "readme"].join('-')

app/views/projects/project_members/index.html.haml

 - page_title "Members"
+- can_admin_project_members = can?(current_user, :admin_project_member, @project)
 
 .row.prepend-top-default
   .col-lg-12
-    %h4
-      Project members
-    - if can?(current_user, :admin_project_member, @project)
-      %p
-        You can add a new member to
-        %strong= @project.name
-        or share it with another group.
-    - else
-      %p
-        Members can be added by project
-        %i Masters
-        or
-        %i Owners
+    - if project_can_be_shared?
+      %h4
+        Project members
+      - if can_admin_project_members
+        %p= share_project_description
+      - else
+        %p
+          Members can be added by project
+          %i Masters
+          or
+          %i Owners
+
     .light
-      - if can?(current_user, :admin_project_member, @project)
-        %ul.nav-links.gitlab-tabs{ role: 'tablist' }
-          - if !membership_locked?
+      - if can_admin_project_members && project_can_be_shared?
+        - if !membership_locked? && @project.allowed_to_share_with_group?
+          %ul.nav-links.gitlab-tabs{ role: 'tablist' }
             %li.active{ role: 'presentation' }
               %a{ href: '#add-member-pane', id: 'add-member-tab', data: { toggle: 'tab' }, role: 'tab' } Add member
-          - if @project.allowed_to_share_with_group?
             %li{ role: 'presentation', class: ('active' if membership_locked?) }
               %a{ href: '#share-with-group-pane', id: 'share-with-group-tab', data: { toggle: 'tab' }, role: 'tab' } Share with group
 
-        .tab-content.gitlab-tab-content
-          - if !membership_locked?
+          .tab-content.gitlab-tab-content
             .tab-pane.active{ id: 'add-member-pane', role: 'tabpanel' }
               = render 'projects/project_members/new_project_member', tab_title: 'Add member'
-          .tab-pane{ id: 'share-with-group-pane', role: 'tabpanel', class: ('active' if membership_locked?) }
-            = render 'projects/project_members/new_shared_group', tab_title: 'Share with group'
+            .tab-pane{ id: 'share-with-group-pane', role: 'tabpanel', class: ('active' if membership_locked?) }
+              = render 'projects/project_members/new_shared_group', tab_title: 'Share with group'
+        - elsif !membership_locked?
+          .add-member= render 'projects/project_members/new_project_member', tab_title: 'Add member'
+        - elsif @project.allowed_to_share_with_group?
+          .share-with-group= render 'projects/project_members/new_shared_group', tab_title: 'Share with group'
+
+
 
         = render 'shared/members/requests', membership_source: @project, requesters: @requesters
         .clearfix

changelogs/unreleased-ee/4471-member-and-group-lock-enabled-allows-project-to-be-shared.yml

+---
+title: Project can no longer be shared between groups when both member and group locks
+  are active
+merge_request:
+author:
+type: security

spec/controllers/projects/group_links_controller_spec.rb

@@ -21,6 +21,18 @@ describe Projects::GroupLinksController do
       end
     end
 
+    context 'when project is not allowed to be shared with a group' do
+      before do
+        group.update_attributes(share_with_group_lock: false)
+      end
+
+      include_context 'link project to group'
+
+      it 'responds with status 404' do
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
     context 'when user has access to group he want to link project to' do
       before do
         group.add_developer(user)

spec/features/projects/members/share_with_group_spec.rb → spec/features/projects/members/share_with_group_and_members_spec.rb

@@ -7,18 +7,47 @@ feature 'Project > Members > Share with Group', :js do
   let(:master) { create(:user) }
 
   describe 'Share with group lock' do
-    shared_examples 'the project can be shared with groups' do
-      scenario 'the "Share with group" tab exists' do
+    shared_examples 'the project cannot be shared with groups' do
+      scenario 'user is only able to share with members' do
         visit project_settings_members_path(project)
-        expect(page).to have_selector('#share-with-group-tab')
+
+        expect(page).not_to have_selector('#add-member-tab')
+        expect(page).not_to have_selector('#share-with-group-tab')
+        expect(page).not_to have_selector('.share-with-group')
+        expect(page).to have_selector('.add-member')
       end
     end
 
-    shared_examples 'the project cannot be shared with groups' do
-      scenario 'the "Share with group" tab does not exist' do
+    shared_examples 'the project cannot be shared with members' do
+      scenario 'user is only able to share with groups' do
         visit project_settings_members_path(project)
-        expect(page).to have_selector('#add-member-tab')
+
+        expect(page).not_to have_selector('#add-member-tab')
         expect(page).not_to have_selector('#share-with-group-tab')
+        expect(page).not_to have_selector('.add-member')
+        expect(page).to have_selector('.share-with-group')
+      end
+    end
+
+    shared_examples 'the project cannot be shared with groups and members' do
+      scenario 'no tabs or share content exists' do
+        visit project_settings_members_path(project)
+
+        expect(page).not_to have_selector('#add-member-tab')
+        expect(page).not_to have_selector('#share-with-group-tab')
+        expect(page).not_to have_selector('.add-member')
+        expect(page).not_to have_selector('.share-with-group')
+      end
+    end
+
+    shared_examples 'the project can be shared with groups and members' do
+      scenario 'both member and group tabs exist' do
+        visit project_settings_members_path(project)
+
+        expect(page).not_to have_selector('.add-member')
+        expect(page).not_to have_selector('.share-with-group')
+        expect(page).to have_selector('#add-member-tab')
+        expect(page).to have_selector('#share-with-group-tab')
       end
     end
 
@@ -31,8 +60,8 @@ feature 'Project > Members > Share with Group', :js do
         sign_in(master)
       end
 
-      context 'when the group has "Share with group lock" disabled' do
-        it_behaves_like 'the project can be shared with groups'
+      context 'when the group has "Share with group lock" and "Member lock" disabled' do
+        it_behaves_like 'the project can be shared with groups and members'
 
         scenario 'the project can be shared with another group' do
           visit project_settings_members_path(project)
@@ -56,10 +85,25 @@ feature 'Project > Members > Share with Group', :js do
 
         it_behaves_like 'the project cannot be shared with groups'
       end
+
+      context 'when the group has membership lock enabled' do
+        before do
+          project.namespace.update_column(:membership_lock, true)
+        end
+
+        it_behaves_like 'the project cannot be shared with members'
+      end
+
+      context 'when the group has membership lock and "Share with group lock" enabled' do
+        before do
+          project.namespace.update_attributes(share_with_group_lock: true, membership_lock: true)
+        end
+
+        it_behaves_like 'the project cannot be shared with groups and members'
+      end
     end
 
     context 'for a project in a subgroup', :nested_groups do
-      let!(:group_to_share_with) { create(:group) }
       let(:root_group) { create(:group) }
       let(:subgroup) { create(:group, parent: root_group) }
       let(:project) { create(:project, namespace: subgroup) }
@@ -69,9 +113,9 @@ feature 'Project > Members > Share with Group', :js do
         sign_in(master)
       end
 
-      context 'when the root_group has "Share with group lock" disabled' do
-        context 'when the subgroup has "Share with group lock" disabled' do
-          it_behaves_like 'the project can be shared with groups'
+      context 'when the root_group has "Share with group lock" and membership lock disabled' do
+        context 'when the subgroup has "Share with group lock" and membership lock disabled' do
+          it_behaves_like 'the project can be shared with groups and members'
         end
 
         context 'when the subgroup has "Share with group lock" enabled' do
@@ -81,24 +125,67 @@ feature 'Project > Members > Share with Group', :js do
 
           it_behaves_like 'the project cannot be shared with groups'
         end
+
+        context 'when the subgroup has membership lock enabled' do
+          before do
+            subgroup.update_column(:membership_lock, true)
+          end
+
+          it_behaves_like 'the project cannot be shared with members'
+        end
+
+        context 'when the group has membership lock and "Share with group lock" enabled' do
+          before do
+            subgroup.update_attributes(share_with_group_lock: true, membership_lock: true)
+          end
+
+          it_behaves_like 'the project cannot be shared with groups and members'
+        end
       end
 
-      context 'when the root_group has "Share with group lock" enabled' do
+      context 'when the root_group has "Share with group lock" and membership lock enabled' do
         before do
-          root_group.update_column(:share_with_group_lock, true)
+          root_group.update_attributes(share_with_group_lock: true, membership_lock: true)
+          subgroup.reload
+        end
+
+        # This behaviour should be changed to disable sharing with members as well
+        # See: https://gitlab.com/gitlab-org/gitlab-ce/issues/42093
+        it_behaves_like 'the project cannot be shared with groups'
+
+        context 'when the subgroup has "Share with group lock" and membership lock disabled (parent overridden)' do
+          before do
+            subgroup.update_attributes(share_with_group_lock: false, membership_lock: false)
+          end
+
+          it_behaves_like 'the project can be shared with groups and members'
         end
 
-        context 'when the subgroup has "Share with group lock" disabled (parent overridden)' do
-          it_behaves_like 'the project can be shared with groups'
+        # This behaviour should be changed to disable sharing with members as well
+        # See: https://gitlab.com/gitlab-org/gitlab-ce/issues/42093
+        context 'when the subgroup has membership lock enabled (parent overridden)' do
+          before do
+            subgroup.update_column(:membership_lock, true)
+          end
+
+          it_behaves_like 'the project cannot be shared with groups and members'
         end
 
-        context 'when the subgroup has "Share with group lock" enabled' do
+        context 'when the subgroup has "Share with group lock" enabled (parent overridden)' do
           before do
             subgroup.update_column(:share_with_group_lock, true)
           end
 
           it_behaves_like 'the project cannot be shared with groups'
         end
+
+        context 'when the subgroup has "Share with group lock" and membership lock enabled' do
+          before do
+            subgroup.update_attributes(membership_lock: true, share_with_group_lock: true)
+          end
+
+          it_behaves_like 'the project cannot be shared with groups and members'
+        end
       end
     end
   end