Merge branch 'if-215376-project_bot_exclusivity' into 'master'

Disallow project bot in multiple projects / groups

See merge request gitlab-org/gitlab!36827

app/models/member.rb

@@ -38,6 +38,11 @@ class Member < ApplicationRecord
       scope: [:source_type, :source_id],
       allow_nil: true
     }
+  validates :user_id,
+    uniqueness: {
+      message: _('project bots cannot be added to other groups / projects')
+    },
+    if: :project_bot?
 
   # This scope encapsulates (most of) the conditions a row in the member table
   # must satisfy if it is a valid permission. Of particular note:
@@ -473,6 +478,10 @@ class Member < ApplicationRecord
   def update_highest_role_attribute
     user_id
   end
+
+  def project_bot?
+    user&.project_bot?
+  end
 end
 
 Member.prepend_if_ee('EE::Member')

app/services/members/create_service.rb

@@ -22,7 +22,7 @@ module Members
       errors = []
 
       members.each do |member|
-        if member.errors.any?
+        if member.invalid?
           current_error =
             # Invited users may not have an associated user
             if member.user.present?

lib/api/members.rb

@@ -107,7 +107,7 @@ module API
 
           if !member
             not_allowed! # This currently can only be reached in EE
-          elsif member.persisted? && member.valid?
+          elsif member.valid? && member.persisted?
             present_members(member)
           else
             render_validation_error!(member)

locale/gitlab.pot

@@ -28648,6 +28648,9 @@ msgstr ""
 msgid "project avatar"
 msgstr ""
 
+msgid "project bots cannot be added to other groups / projects"
+msgstr ""
+
 msgid "project is read-only"
 msgstr ""
 

spec/controllers/projects/project_members_controller_spec.rb

@@ -106,6 +106,29 @@ RSpec.describe Projects::ProjectMembersController do
         expect(response).to redirect_to(project_project_members_path(project))
       end
     end
+
+    context 'adding project bot' do
+      let_it_be(:project_bot) { create(:user, :project_bot) }
+
+      before do
+        project.add_maintainer(user)
+
+        unrelated_project = create(:project)
+        unrelated_project.add_maintainer(project_bot)
+      end
+
+      it 'returns error' do
+        post :create, params: {
+          namespace_id: project.namespace,
+          project_id: project,
+          user_ids: project_bot.id,
+          access_level: Gitlab::Access::GUEST
+        }
+
+        expect(flash[:alert]).to include('project bots cannot be added to other groups / projects')
+        expect(response).to redirect_to(project_project_members_path(project))
+      end
+    end
   end
 
   describe 'PUT update' do

spec/models/member_spec.rb

@@ -88,6 +88,28 @@ RSpec.describe Member do
         expect(child_member).to be_valid
       end
     end
+
+    context 'project bots' do
+      let_it_be(:project_bot) { create(:user, :project_bot) }
+      let(:new_member) { build(:project_member, user_id: project_bot.id) }
+
+      context 'not a member of any group or project' do
+        it 'is valid' do
+          expect(new_member).to be_valid
+        end
+      end
+
+      context 'already member of a project' do
+        before do
+          unrelated_project = create(:project)
+          unrelated_project.add_maintainer(project_bot)
+        end
+
+        it 'is not valid' do
+          expect(new_member).not_to be_valid
+        end
+      end
+    end
   end
 
   describe 'Scopes & finders' do

spec/requests/api/members_spec.rb

@@ -321,6 +321,26 @@ RSpec.describe API::Members do
         expect(response).to have_gitlab_http_status(:bad_request)
       end
     end
+
+    context 'adding project bot' do
+      let_it_be(:project_bot) { create(:user, :project_bot) }
+
+      before do
+        unrelated_project = create(:project)
+        unrelated_project.add_maintainer(project_bot)
+      end
+
+      it 'returns 400' do
+        expect do
+          post api("/#{source_type.pluralize}/#{source.id}/members", maintainer),
+               params: { user_id: project_bot.id, access_level: Member::DEVELOPER }
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response['message']['user_id']).to(
+            include('project bots cannot be added to other groups / projects'))
+        end.not_to change { project.members.count }
+      end
+    end
   end
 
   shared_examples 'PUT /:source_type/:id/members/:user_id' do |source_type|
@@ -461,8 +481,34 @@ RSpec.describe API::Members do
     end
   end
 
-  it_behaves_like 'POST /:source_type/:id/members', 'project' do
-    let(:source) { project }
+  describe 'POST /projects/:id/members' do
+    it_behaves_like 'POST /:source_type/:id/members', 'project' do
+      let(:source) { project }
+    end
+
+    context 'adding owner to project' do
+      it 'returns 403' do
+        expect do
+          post api("/projects/#{project.id}/members", maintainer),
+               params: { user_id: stranger.id, access_level: Member::OWNER }
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end.not_to change { project.members.count }
+      end
+    end
+
+    context 'remove bot from project' do
+      it 'returns a 403 forbidden' do
+        project_bot = create(:user, :project_bot)
+        create(:project_member, project: project, user: project_bot)
+
+        expect do
+          delete api("/projects/#{project.id}/members/#{project_bot.id}", maintainer)
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end.not_to change { project.members.count }
+      end
+    end
   end
 
   it_behaves_like 'POST /:source_type/:id/members', 'group' do
@@ -484,28 +530,4 @@ RSpec.describe API::Members do
   it_behaves_like 'DELETE /:source_type/:id/members/:user_id', 'group' do
     let(:source) { group }
   end
-
-  context 'Adding owner to project' do
-    it 'returns 403' do
-      expect do
-        post api("/projects/#{project.id}/members", maintainer),
-             params: { user_id: stranger.id, access_level: Member::OWNER }
-
-        expect(response).to have_gitlab_http_status(:bad_request)
-      end.to change { project.members.count }.by(0)
-    end
-  end
-
-  context 'remove bot from project' do
-    it 'returns a 403 forbidden' do
-      project_bot = create(:user, :project_bot)
-      create(:project_member, project: project, user: project_bot)
-
-      expect do
-        delete api("/projects/#{project.id}/members/#{project_bot.id}", maintainer)
-
-        expect(response).to have_gitlab_http_status(:forbidden)
-      end.to change { project.members.count }.by(0)
-    end
-  end
 end