Respect group membership lock when importing a member from another group

app/controllers/projects/project_members_controller.rb

@@ -7,6 +7,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
 
   # Authorize
   before_action :authorize_admin_project_member!, except: [:index, :leave, :request_access]
+  before_action :check_membership_lock!, only: [:create, :import, :apply_import]
 
   # rubocop: disable CodeReuse/ActiveRecord
   def index
@@ -50,4 +51,8 @@ class Projects::ProjectMembersController < Projects::ApplicationController
 
   # MembershipActions concern
   alias_method :membershipable, :project
+
+  def check_membership_lock!
+    access_denied!('Membership is locked by group settings') if membership_locked?
+  end
 end

ee/changelogs/unreleased/security-maintainer-can-ignore-membership-lock.yml

+---
+title: Respect group membership lock when importing a member from another group
+merge_request:
+author:
+type: security

spec/controllers/projects/project_members_controller_spec.rb

@@ -2,7 +2,8 @@ require('spec_helper')
 
 describe Projects::ProjectMembersController do
   let(:user) { create(:user) }
-  let(:project) { create(:project, :public, :access_requestable) }
+  let(:project) { create(:project, :public, :access_requestable, namespace: namespace) }
+  let(:namespace) { create :group }
 
   describe 'GET index' do
     it 'should have the project_members address with a 200 status code' do
@@ -313,6 +314,13 @@ describe Projects::ProjectMembersController do
   end
 
   describe 'POST apply_import' do
+    subject(:apply_import) do
+      post(:apply_import, params: {
+        namespace_id: project.namespace,
+        project_id: project,
+        source_project_id: another_project.id
+      })
+    end
     let(:another_project) { create(:project, :private) }
     let(:member) { create(:user) }
 
@@ -322,40 +330,44 @@ describe Projects::ProjectMembersController do
       sign_in(user)
     end
 
-    shared_context 'import applied' do
-      before do
-        post(:apply_import, params: {
-                              namespace_id: project.namespace,
-                              project_id: project,
-                              source_project_id: another_project.id
-                            })
-      end
-    end
-
     context 'when user can access source project members' do
       before do
         another_project.add_guest(user)
       end
 
-      include_context 'import applied'
-
       it 'imports source project members' do
+        apply_import
+
         expect(project.team_members).to include member
         expect(response).to set_flash.to 'Successfully imported'
         expect(response).to redirect_to(
           project_project_members_path(project)
         )
       end
+
+      context 'and the project group has membership lock enabled' do
+        before do
+          project.namespace.update(membership_lock: true)
         end
 
-    context 'when user is not member of a source project' do
-      include_context 'import applied'
+        it 'responds with 403' do
+          apply_import
 
+          expect(response.status).to eq 403
+        end
+      end
+    end
+
+    context 'when user is not member of a source project' do
       it 'does not import team members' do
+        apply_import
+
         expect(project.team_members).not_to include member
       end
 
       it 'responds with not found' do
+        apply_import
+
         expect(response.status).to eq 404
       end
     end
@@ -363,40 +375,78 @@ describe Projects::ProjectMembersController do
 
   describe 'POST create' do
     let(:stranger) { create(:user) }
+    subject(:create_member) do
+      post :create, params: {
+        user_ids: stranger.id,
+        namespace_id: project.namespace,
+        access_level: access_level,
+        project_id: project
+      }
+    end
+    let(:access_level) { nil }
 
-    context 'when creating owner' do
     before do
       project.add_maintainer(user)
       sign_in(user)
     end
 
+    context 'when creating owner' do
+      let(:access_level) { Member::OWNER }
+
       it 'does not create a member' do
-        expect do
-          post :create, params: {
-                          user_ids: stranger.id,
-                          namespace_id: project.namespace,
-                          access_level: Member::OWNER,
-                          project_id: project
-                        }
-        end.to change { project.members.count }.by(0)
+        expect { create_member }.not_to change { project.members.count }
       end
     end
 
     context 'when create maintainer' do
+      let(:access_level) { Member::MAINTAINER }
+
+      it 'creates a member' do
+        expect { create_member }.to change { project.members.count }.by(1)
+      end
+    end
+
+    context 'when project group has membership lock enabled' do
       before do
-        project.add_maintainer(user)
-        sign_in(user)
+        project.namespace.update(membership_lock: true)
       end
 
-      it 'creates a member' do
-        expect do
-          post :create, params: {
-                          user_ids: stranger.id,
+      it 'responds with 403' do
+        create_member
+
+        expect(response.status).to eq 403
+      end
+    end
+  end
+
+  describe 'GET import' do
+    subject(:import) do
+      get :import, params: {
         namespace_id: project.namespace,
-                          access_level: Member::MAINTAINER,
         project_id: project
       }
-        end.to change { project.members.count }.by(1)
+    end
+
+    before do
+      project.add_maintainer(user)
+      sign_in(user)
+    end
+
+    it 'responds with 200' do
+      import
+
+      expect(response.status).to eq 200
+    end
+
+    context 'when project group has membership lock enabled' do
+      before do
+        project.namespace.update(membership_lock: true)
+      end
+
+      it 'responds with 403' do
+        import
+
+        expect(response.status).to eq 403
       end
     end
   end