Add latest changes from gitlab-org/security/gitlab@14-1-stable-ee

10 files changed, 209 insertions, 101 deletions

diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb
index 0d9cde88eca..fd7631edbbb 100644
--- a/spec/controllers/invites_controller_spec.rb
+++ b/spec/controllers/invites_controller_spec.rb
@@ -25,9 +25,64 @@ RSpec.describe InvitesController do
     end
   end
 
+  shared_examples 'invite email match enforcement' do |error_status:, flash_alert: nil|
+    it 'accepts user if invite email matches signed in user' do
+      expect do
+        request
+      end.to change { project_members.include?(user) }.from(false).to(true)
+
+      expect(response).to have_gitlab_http_status(:found)
+      expect(flash[:notice]).to include 'You have been granted'
+    end
+
+    it 'accepts invite if invite email matches confirmed secondary email' do
+      secondary_email = create(:email, :confirmed, user: user)
+      member.update!(invite_email: secondary_email.email)
+
+      expect do
+        request
+      end.to change { project_members.include?(user) }.from(false).to(true)
+
+      expect(response).to have_gitlab_http_status(:found)
+      expect(flash[:notice]).to include 'You have been granted'
+    end
+
+    it 'does not accept if invite email matches unconfirmed secondary email' do
+      secondary_email = create(:email, user: user)
+      member.update!(invite_email: secondary_email.email)
+
+      expect do
+        request
+      end.not_to change { project_members.include?(user) }
+
+      expect(response).to have_gitlab_http_status(error_status)
+      expect(flash[:alert]).to eq(flash_alert)
+    end
+
+    it 'does not accept if invite email does not match signed in user' do
+      member.update!(invite_email: 'bogus@email.com')
+
+      expect do
+        request
+      end.not_to change { project_members.include?(user) }
+
+      expect(response).to have_gitlab_http_status(error_status)
+      expect(flash[:alert]).to eq(flash_alert)
+    end
+  end
+
   describe 'GET #show' do
     subject(:request) { get :show, params: params }
 
+    context 'when logged in' do
+      before do
+        sign_in(user)
+      end
+
+      it_behaves_like 'invite email match enforcement', error_status: :ok
+      it_behaves_like 'invalid token'
+    end
+
     context 'when it is part of our invite email experiment' do
       let(:extra_params) { { invite_type: 'initial_email' } }
 
@@ -59,34 +114,6 @@ RSpec.describe InvitesController do
       end
     end
 
-    context 'when logged in' do
-      before do
-        sign_in(user)
-      end
-
-      it 'accepts user if invite email matches signed in user' do
-        expect do
-          request
-        end.to change { project_members.include?(user) }.from(false).to(true)
-
-        expect(response).to have_gitlab_http_status(:found)
-        expect(flash[:notice]).to include 'You have been granted'
-      end
-
-      it 'forces re-confirmation if email does not match signed in user' do
-        member.update!(invite_email: 'bogus@email.com')
-
-        expect do
-          request
-        end.not_to change { project_members.include?(user) }
-
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(flash[:notice]).to be_nil
-      end
-
-      it_behaves_like 'invalid token'
-    end
-
     context 'when not logged in' do
       context 'when invite token belongs to a valid member' do
         context 'when instance allows sign up' do
@@ -213,6 +240,7 @@ RSpec.describe InvitesController do
 
     subject(:request) { post :accept, params: params }
 
+    it_behaves_like 'invite email match enforcement', error_status: :redirect, flash_alert: 'The invitation could not be accepted.'
     it_behaves_like 'invalid token'
   end
 
diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb
index 2379ff9fd98..65a563fac7c 100644
--- a/spec/controllers/projects/pipelines_controller_spec.rb
+++ b/spec/controllers/projects/pipelines_controller_spec.rb
@@ -302,35 +302,46 @@ RSpec.describe Projects::PipelinesController do
   end
 
   describe 'GET #show' do
-    render_views
-
-    let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
-
-    subject { get_pipeline_html }
-
     def get_pipeline_html
       get :show, params: { namespace_id: project.namespace, project_id: project, id: pipeline }, format: :html
     end
 
-    def create_build_with_artifacts(stage, stage_idx, name)
-      create(:ci_build, :artifacts, :tags, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name)
-    end
+    context 'when the project is public' do
+      render_views
 
-    before do
-      create_build_with_artifacts('build', 0, 'job1')
-      create_build_with_artifacts('build', 0, 'job2')
+      let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
+
+      def create_build_with_artifacts(stage, stage_idx, name)
+        create(:ci_build, :artifacts, :tags, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name)
+      end
+
+      before do
+        create_build_with_artifacts('build', 0, 'job1')
+        create_build_with_artifacts('build', 0, 'job2')
+      end
+
+      it 'avoids N+1 database queries', :request_store do
+        control_count = ActiveRecord::QueryRecorder.new { get_pipeline_html }.count
+        expect(response).to have_gitlab_http_status(:ok)
+
+        create_build_with_artifacts('build', 0, 'job3')
+
+        expect { get_pipeline_html }.not_to exceed_query_limit(control_count)
+        expect(response).to have_gitlab_http_status(:ok)
+      end
     end
 
-    it 'avoids N+1 database queries', :request_store do
-      get_pipeline_html
+    context 'when the project is private' do
+      let(:project) { create(:project, :private, :repository) }
+      let(:pipeline) { create(:ci_pipeline, project: project) }
 
-      control_count = ActiveRecord::QueryRecorder.new { get_pipeline_html }.count
-      expect(response).to have_gitlab_http_status(:ok)
+      it 'returns `not_found` when the user does not have access' do
+        sign_in(create(:user))
 
-      create_build_with_artifacts('build', 0, 'job3')
+        get_pipeline_html
 
-      expect { get_pipeline_html }.not_to exceed_query_limit(control_count)
-      expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
     end
   end
 
diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb
index 93602033d73..b7e1004aef5 100644
--- a/spec/features/invites_spec.rb
+++ b/spec/features/invites_spec.rb
@@ -90,48 +90,17 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
         end
 
         context 'when signed in and an invite link is clicked' do
-          context 'when an invite email is a secondary email for the user' do
-            let(:invite_email) { 'user_secondary@example.com' }
-
-            before do
-              sign_in(user)
-              visit invite_path(group_invite.raw_invite_token)
-            end
-
-            it 'sends user to the invite url and allows them to decline' do
-              expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
-              expect(page).to have_content("Note that this invitation was sent to #{invite_email}")
-              expect(page).to have_content("but you are signed in as #{user.to_reference} with email #{user.email}")
-
-              click_link('Decline')
-
-              expect(page).to have_content('You have declined the invitation')
-              expect(current_path).to eq(dashboard_projects_path)
-              expect { group_invite.reload }.to raise_error ActiveRecord::RecordNotFound
-            end
-
-            it 'sends uer to the invite url and allows them to accept' do
-              expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
-              expect(page).to have_content("Note that this invitation was sent to #{invite_email}")
-              expect(page).to have_content("but you are signed in as #{user.to_reference} with email #{user.email}")
-
-              click_link('Accept invitation')
-
-              expect(page).to have_content('You have been granted')
-              expect(current_path).to eq(activity_group_path(group))
-            end
-          end
-
           context 'when user is an existing member' do
             before do
-              sign_in(owner)
+              group.add_developer(user)
+              sign_in(user)
               visit invite_path(group_invite.raw_invite_token)
             end
 
             it 'shows message user already a member' do
               expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
-              expect(page).to have_link(owner.name, href: user_url(owner))
-              expect(page).to have_content('However, you are already a member of this group.')
+              expect(page).to have_link(user.name, href: user_path(user))
+              expect(page).to have_content('You are already a member of this group.')
             end
           end
         end
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index 0958e1d1891..ce2083b397a 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -365,9 +365,8 @@ RSpec.describe 'Pipeline', :js do
         let(:project) { create(:project, :public, :repository, public_builds: false) }
         let(:role) { :guest }
 
-        it 'does not show failed jobs tab pane' do
-          expect(page).to have_link('Pipeline')
-          expect(page).not_to have_content('Failed Jobs')
+        it 'does not show the pipeline details page' do
+          expect(page).to have_content('Not Found')
         end
       end
     end
diff --git a/spec/frontend/members/store/mutations_spec.js b/spec/frontend/members/store/mutations_spec.js
index 7ad7034eb6d..78bbad394a0 100644
--- a/spec/frontend/members/store/mutations_spec.js
+++ b/spec/frontend/members/store/mutations_spec.js
@@ -44,8 +44,7 @@ describe('Vuex members mutations', () => {
       describe('when error has a message', () => {
         it('shows error message', () => {
           const error = new Error('Request failed with status code 422');
-          const message =
-            'User email "john.smith@gmail.com" does not match the allowed domain of example.com';
+          const message = 'User email does not match the allowed domain of example.com';
 
           error.response = {
             data: { message },
@@ -88,8 +87,7 @@ describe('Vuex members mutations', () => {
       describe('when error has a message', () => {
         it('shows error message', () => {
           const error = new Error('Request failed with status code 422');
-          const message =
-            'User email "john.smith@gmail.com" does not match the allowed domain of example.com';
+          const message = 'User email does not match the allowed domain of example.com';
 
           error.response = {
             data: { message },
diff --git a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb b/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb
index e8470657181..2ef3b324db8 100644
--- a/spec/lib/gitlab/email/handler/service_desk_handler_spec.rb
+++ b/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
 
diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb
index bc09191f6ec..8ff936d5a35 100644
--- a/spec/policies/issue_policy_spec.rb
+++ b/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
diff --git a/spec/policies/personal_access_token_policy_spec.rb b/spec/policies/personal_access_token_policy_spec.rb
index b5e8d40b133..e146133429b 100644
--- a/spec/policies/personal_access_token_policy_spec.rb
+++ b/spec/policies/personal_access_token_policy_spec.rb
@@ -41,6 +41,13 @@ RSpec.describe PersonalAccessTokenPolicy do
       it { is_expected.to be_allowed(:read_token) }
       it { is_expected.to be_allowed(:revoke_token) }
     end
+
+    context 'subject of the impersonated token' do
+      let_it_be(:token) { build_stubbed(:personal_access_token, user: current_user, impersonation: true) }
+
+      it { is_expected.to be_disallowed(:read_token) }
+      it { is_expected.to be_disallowed(:revoke_token) }
+    end
   end
 
   context 'current_user is a blocked administrator', :enable_admin_mode do
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 59123c3695a..77214814b7f 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/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
diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb
index ccc5f322ff9..0ff2c46e693 100644
--- a/spec/requests/api/personal_access_tokens_spec.rb
+++ b/spec/requests/api/personal_access_tokens_spec.rb
@@ -6,6 +6,7 @@ RSpec.describe API::PersonalAccessTokens do
   let_it_be(:path) { '/personal_access_tokens' }
   let_it_be(:token1) { create(:personal_access_token) }
   let_it_be(:token2) { create(:personal_access_token) }
+  let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: token1.user) }
   let_it_be(:current_user) { create(:user) }
 
   describe 'GET /personal_access_tokens' do
@@ -24,8 +25,9 @@ RSpec.describe API::PersonalAccessTokens do
           get api(path, current_user), params: { user_id: token1.user.id }
 
           expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response.count).to eq(1)
+          expect(json_response.count).to eq(2)
           expect(json_response.first['user_id']).to eq(token1.user.id)
+          expect(json_response.last['id']).to eq(token_impersonated.id)
         end
       end
 
@@ -34,6 +36,7 @@ RSpec.describe API::PersonalAccessTokens do
         let_it_be(:user) { create(:user) }
         let_it_be(:token) { create(:personal_access_token, user: current_user)}
         let_it_be(:other_token) { create(:personal_access_token, user: user) }
+        let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: current_user) }
 
         it 'returns all PATs belonging to the signed-in user' do
           get api(path, current_user, personal_access_token: token)
@@ -95,6 +98,7 @@ RSpec.describe API::PersonalAccessTokens do
     context 'when current_user is not an administrator' do
       let_it_be(:user_token) { create(:personal_access_token, user: current_user) }
       let_it_be(:user_token_path) { "/personal_access_tokens/#{user_token.id}" }
+      let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: current_user) }
 
       it 'fails revokes a different users token' do
         delete api(path, current_user)
@@ -107,6 +111,12 @@ RSpec.describe API::PersonalAccessTokens do
 
         expect(response).to have_gitlab_http_status(:no_content)
       end
+
+      it 'cannot revoke impersonation token' do
+        delete api("/personal_access_tokens/#{token_impersonated.id}", current_user)
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+      end
     end
   end
 end