Disallow non-members to set issue metadata on issue create

When creating an issue a guest member is allowed to set issue
metadata, however in public projects authenticated non-members
get guest access as well, so we would restrict permission to
set issue metadata for non-members specifically.

Changelog: security

app/policies/issue_policy.rb

@@ -44,7 +44,18 @@ class IssuePolicy < IssuablePolicy
     enable :update_subscription
   end
 
-  rule { ~persisted & can?(:guest_access) }.policy do
+  # admin can set metadata on new issues
+  rule { ~persisted & admin }.policy do
+    enable :set_issue_metadata
+  end
+
+  # support bot needs to be able to set metadata on new issues when service desk is enabled
+  rule { ~persisted & support_bot & can?(:guest_access) }.policy do
+    enable :set_issue_metadata
+  end
+
+  # guest members need to be able to set issue metadata per https://gitlab.com/gitlab-org/gitlab/-/issues/300100
+  rule { ~persisted & is_project_member & can?(:guest_access) }.policy do
     enable :set_issue_metadata
   end
 

app/policies/project_policy.rb

@@ -673,6 +673,7 @@ class ProjectPolicy < BasePolicy
   rule { support_bot & ~service_desk_enabled }.policy do
     prevent :create_note
     prevent :read_project
+    prevent :guest_access
   end
 
   rule { project_bot }.enable :project_bot_access

spec/lib/gitlab/email/handler/service_desk_handler_spec.rb

@@ -138,6 +138,25 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do
               expect(issue.assignees).to be_empty
               expect(issue.milestone).to be_nil
             end
+
+            context 'when issues are set to private' do
+              before do
+                project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
+              end
+
+              it 'applies quick action commands present on templates' do
+                file_content = %(Text from service_desk2 template \n/label ~#{label.title} \n/milestone %"#{milestone.name}")
+                set_template_file('service_desk2', file_content)
+
+                receiver.execute
+
+                issue = Issue.last
+                expect(issue.description).to include('Text from service_desk2 template')
+                expect(issue.label_ids).to include(label.id)
+                expect(issue.author_id).to eq(User.support_bot.id)
+                expect(issue.milestone).to eq(milestone)
+              end
+            end
           end
         end
 

spec/policies/issue_policy_spec.rb

@@ -11,13 +11,37 @@ RSpec.describe IssuePolicy do
   let(:reporter) { create(:user) }
   let(:group) { create(:group, :public) }
   let(:reporter_from_group_link) { create(:user) }
+  let(:non_member) { create(:user) }
+  let(:support_bot) { User.support_bot }
 
   def permissions(user, issue)
     described_class.new(user, issue)
   end
 
+  shared_examples 'support bot with service desk enabled' do
+    before do
+      allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true }
+      allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }
+
+      project.update!(service_desk_enabled: true)
+    end
+
+    it 'allows support_bot to read issues, create and set metadata on new issues' do
+      expect(permissions(support_bot, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata)
+      expect(permissions(support_bot, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata)
+      expect(permissions(support_bot, new_issue)).to be_allowed(:create_issue, :set_issue_metadata)
+    end
+  end
+
+  shared_examples 'support bot with service desk disabled' do
+    it 'allows support_bot to read issues, create and set metadata on new issues' do
+      expect(permissions(support_bot, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata)
+      expect(permissions(support_bot, issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata)
+      expect(permissions(support_bot, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata)
+    end
+  end
+
   context 'a private project' do
-    let(:non_member) { create(:user) }
     let(:project) { create(:project, :private) }
     let(:issue) { create(:issue, project: project, assignees: [assignee], author: author) }
     let(:issue_no_assignee) { create(:issue, project: project) }
@@ -34,12 +58,6 @@ RSpec.describe IssuePolicy do
       create(:project_group_link, group: group, project: project)
     end
 
-    it 'does not allow non-members to read issues' do
-      expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata)
-      expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata)
-      expect(permissions(non_member, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata)
-    end
-
     it 'allows guests to read issues' do
       expect(permissions(guest, issue)).to be_allowed(:read_issue, :read_issue_iid)
       expect(permissions(guest, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata)
@@ -82,6 +100,15 @@ RSpec.describe IssuePolicy do
       expect(permissions(assignee, new_issue)).to be_allowed(:create_issue, :set_issue_metadata)
     end
 
+    it 'does not allow non-members to read, update or create issues' do
+      expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata)
+      expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata)
+      expect(permissions(non_member, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata)
+    end
+
+    it_behaves_like 'support bot with service desk disabled'
+    it_behaves_like 'support bot with service desk enabled'
+
     context 'with confidential issues' do
       let(:confidential_issue) { create(:issue, :confidential, project: project, assignees: [assignee], author: author) }
       let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) }
@@ -196,7 +223,8 @@ RSpec.describe IssuePolicy do
       expect(permissions(author, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue)
       expect(permissions(author, issue_locked)).to be_disallowed(:admin_issue, :reopen_issue, :set_issue_metadata)
 
-      expect(permissions(author, new_issue)).to be_allowed(:create_issue, :set_issue_metadata)
+      expect(permissions(author, new_issue)).to be_allowed(:create_issue)
+      expect(permissions(author, new_issue)).to be_disallowed(:set_issue_metadata)
     end
 
     it 'allows issue assignees to read, reopen and update their issues' do
@@ -208,14 +236,44 @@ RSpec.describe IssuePolicy do
 
       expect(permissions(assignee, issue_locked)).to be_allowed(:read_issue, :read_issue_iid, :update_issue)
       expect(permissions(assignee, issue_locked)).to be_disallowed(:admin_issue, :reopen_issue, :set_issue_metadata)
+    end
 
-      expect(permissions(author, new_issue)).to be_allowed(:create_issue, :set_issue_metadata)
+    it 'allows non-members to read and create issues' do
+      expect(permissions(non_member, issue)).to be_allowed(:read_issue, :read_issue_iid)
+      expect(permissions(non_member, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid)
+      expect(permissions(non_member, new_issue)).to be_allowed(:create_issue)
+    end
+
+    it 'allows non-members to read issues' do
+      expect(permissions(non_member, issue)).to be_allowed(:read_issue, :read_issue_iid)
+      expect(permissions(non_member, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid)
+    end
+
+    it 'does not allow non-members to update, admin or set metadata' do
+      expect(permissions(non_member, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata)
+      expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata)
+      expect(permissions(non_member, new_issue)).to be_disallowed(:set_issue_metadata)
     end
 
+    it 'allows support_bot to read issues' do
+      # support_bot is still allowed read access in public projects through :public_access permission,
+      # see project_policy public_access rules policy (rule { can?(:public_access) }.policy {...})
+      expect(permissions(support_bot, issue)).to be_allowed(:read_issue, :read_issue_iid)
+      expect(permissions(support_bot, issue)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata)
+
+      expect(permissions(support_bot, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid)
+      expect(permissions(support_bot, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata)
+
+      expect(permissions(support_bot, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata)
+    end
+
+    it_behaves_like 'support bot with service desk enabled'
+
     context 'when issues are private' do
       before do
         project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
       end
+
       let(:issue) { create(:issue, project: project, author: author) }
       let(:visitor) { create(:user) }
       let(:admin) { create(:user, :admin) }
@@ -258,6 +316,15 @@ RSpec.describe IssuePolicy do
           expect(permissions(admin, issue)).to be_disallowed(:create_note)
         end
       end
+
+      it 'does not allow non-members to update or create issues' do
+        expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue, :set_issue_metadata)
+        expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue, :set_issue_metadata)
+        expect(permissions(non_member, new_issue)).to be_disallowed(:create_issue, :set_issue_metadata)
+      end
+
+      it_behaves_like 'support bot with service desk disabled'
+      it_behaves_like 'support bot with service desk enabled'
     end
 
     context 'with confidential issues' do

spec/policies/project_policy_spec.rb

@@ -480,8 +480,8 @@ RSpec.describe ProjectPolicy do
     let(:current_user) { User.support_bot }
 
     context 'with service desk disabled' do
-      it { expect_allowed(:guest_access) }
-      it { expect_disallowed(:create_note, :read_project) }
+      it { expect_allowed(:public_access) }
+      it { expect_disallowed(:guest_access, :create_note, :read_project) }
     end
 
     context 'with service desk enabled' do