Add latest changes from gitlab-org/security/gitlab@13-12-stable-ee

GITLAB_SHELL_VERSION

-13.18.0
+13.18.1

app/controllers/invites_controller.rb

@@ -20,7 +20,7 @@ class InvitesController < ApplicationController
   end
 
   def accept
-    if member.accept_invite!(current_user)
+    if current_user_matches_invite? && member.accept_invite!(current_user)
       redirect_to invite_details[:path], notice: helpers.invite_accepted_notice(member)
     else
       redirect_back_or_default(options: { alert: _("The invitation could not be accepted.") })
@@ -52,7 +52,7 @@ class InvitesController < ApplicationController
   end
 
   def current_user_matches_invite?
-    @member.invite_email == current_user.email
+    current_user.verified_emails.include?(@member.invite_email)
   end
 
   def member?

app/controllers/projects/pipelines_controller.rb

@@ -8,7 +8,7 @@ class Projects::PipelinesController < Projects::ApplicationController
   before_action :pipeline, except: [:index, :new, :create, :charts, :config_variables]
   before_action :set_pipeline_path, only: [:show]
   before_action :authorize_read_pipeline!
-  before_action :authorize_read_build!, only: [:index]
+  before_action :authorize_read_build!, only: [:index, :show]
   before_action :authorize_read_analytics!, only: [:charts]
   before_action :authorize_create_pipeline!, only: [:new, :create, :config_variables]
   before_action :authorize_update_pipeline!, only: [:retry, :cancel]

app/policies/personal_access_token_policy.rb

 # frozen_string_literal: true
 
 class PersonalAccessTokenPolicy < BasePolicy
-  condition(:is_owner) { user && subject.user_id == user.id }
+  condition(:is_owner) { user && subject.user_id == user.id && !subject.impersonation }
 
   rule { (is_owner | admin) & ~blocked }.policy do
     enable :read_token

app/views/invites/show.html.haml

 - page_title _("Invitation")
 %h3.page-title= _("Invitation")
 
-%p
-  = _("You have been invited")
-  - inviter = @member.created_by
-  - if inviter
-    = _("by")
-    = link_to inviter.name, user_url(inviter)
-  = _("to join %{source_name}") % { source_name: @invite_details[:title] }
-  %strong
-    = link_to @invite_details[:name], @invite_details[:url]
-  = _("as %{role}.") % { role: @member.human_access }
+- if current_user_matches_invite?
+  - if member?
+    %p
+      = _("You are already a member of this %{member_source}.") % { member_source: @invite_details[:title] }
+    .actions
+      = link_to _("Go to %{source_name}") % { source_name: @invite_details[:title] }, @invite_details[:url], class: "btn gl-button btn-confirm"
 
-- if member?
-  %p
-    = _("However, you are already a member of this %{member_source}. Sign in using a different account to accept the invitation.") % { member_source: @invite_details[:title] }
+  - else
+    %p
+      - inviter = @member.created_by
+      - link_to_inviter = link_to(inviter.name, user_url(inviter))
+      - link_to_source = link_to(@invite_details[:name], @invite_details[:url])
+
+      = html_escape(_("You have been invited by %{link_to_inviter} to join %{source_name} %{strong_open}%{link_to_source}%{strong_close} as %{role}")) % { link_to_inviter: link_to_inviter, source_name: @invite_details[:title], strong_open: '<strong>'.html_safe, link_to_source: link_to_source, strong_close: '</strong>'.html_safe, role: @member.human_access }
+
+    .actions
+      = link_to _("Accept invitation"), accept_invite_url(@token), method: :post, class: "btn gl-button btn-confirm"
+      = link_to _("Decline"), decline_invite_url(@token), method: :post, class: "btn gl-button btn-danger gl-ml-3"
 
-- if !current_user_matches_invite?
+- else
   %p
     - mail_to_invite_email = mail_to(@member.invite_email)
     - mail_to_current_user = mail_to(current_user.email)
     - link_to_current_user = link_to(current_user.to_reference, user_url(current_user))
-    = _("Note that this invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}.").html_safe % { mail_to_invite_email: mail_to_invite_email, mail_to_current_user: mail_to_current_user, link_to_current_user: link_to_current_user }
-
-- if !member?
-  .actions
-    = link_to _("Accept invitation"), accept_invite_url(@token), method: :post, class: "btn gl-button btn-confirm"
-    = link_to _("Decline"), decline_invite_url(@token), method: :post, class: "btn gl-button btn-danger gl-ml-3"
+    = _("This invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}.").html_safe % { mail_to_invite_email: mail_to_invite_email, mail_to_current_user: mail_to_current_user, link_to_current_user: link_to_current_user }
+  %p
+    = _("Sign in as a user with the matching email address, add the email to this account, or sign-up for a new account using the matching email.")

lib/api/personal_access_tokens.rb

@@ -23,7 +23,7 @@ module API
 
     helpers do
       def finder_params(current_user)
-        current_user.admin? ? { user: user(params[:user_id]) } : { user: current_user }
+        current_user.admin? ? { user: user(params[:user_id]) } : { user: current_user, impersonation: false }
       end
 
       def user(user_id)

locale/gitlab.pot

@@ -15374,6 +15374,9 @@ msgstr ""
 msgid "Go full screen"
 msgstr ""
 
+msgid "Go to %{source_name}"
+msgstr ""
+
 msgid "Go to Webhooks"
 msgstr ""
 
@@ -16470,9 +16473,6 @@ msgstr ""
 msgid "How many users will be evaluating the trial?"
 msgstr ""
 
-msgid "However, you are already a member of this %{member_source}. Sign in using a different account to accept the invitation."
-msgstr ""
-
 msgid "I accept the %{terms_link}"
 msgstr ""
 
@@ -22499,9 +22499,6 @@ msgstr ""
 msgid "Note that pushing to GitLab requires write access to this repository."
 msgstr ""
 
-msgid "Note that this invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}."
-msgstr ""
-
 msgid "Note: As an administrator you may like to configure %{github_integration_link}, which will allow login via GitHub and allow connecting repositories without generating a Personal Access Token."
 msgstr ""
 
@@ -30052,6 +30049,9 @@ msgstr ""
 msgid "Sign in / Register"
 msgstr ""
 
+msgid "Sign in as a user with the matching email address, add the email to this account, or sign-up for a new account using the matching email."
+msgstr ""
+
 msgid "Sign in preview"
 msgstr ""
 
@@ -33166,6 +33166,9 @@ msgstr ""
 msgid "This group, its subgroups and projects will be removed on %{date} since its parent group '%{parent_group_name}'' has been scheduled for removal."
 msgstr ""
 
+msgid "This invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}."
+msgstr ""
+
 msgid "This is a \"Ghost User\", created to hold all issues authored by users that have since been deleted. This user cannot be removed."
 msgstr ""
 
@@ -36958,6 +36961,9 @@ msgstr ""
 msgid "You are about to transfer the control of your account to %{group_name} group. This action is NOT reversible, you won't be able to access any of your groups and projects outside of %{group_name} once this transfer is complete."
 msgstr ""
 
+msgid "You are already a member of this %{member_source}."
+msgstr ""
+
 msgid "You are an admin, which means granting access to %{client_name} will allow them to interact with GitLab as an admin as well. Proceed with caution."
 msgstr ""
 
@@ -37291,7 +37297,7 @@ msgstr ""
 msgid "You have been granted %{member_human_access} access to project %{name}."
 msgstr ""
 
-msgid "You have been invited"
+msgid "You have been invited by %{link_to_inviter} to join %{source_name} %{strong_open}%{link_to_source}%{strong_close} as %{role}"
 msgstr ""
 
 msgid "You have been redirected to the only result; see the %{a_start}search results%{a_end} instead."
@@ -37887,9 +37893,6 @@ msgstr ""
 msgid "archived:"
 msgstr ""
 
-msgid "as %{role}."
-msgstr ""
-
 msgid "assign yourself"
 msgstr ""
 
@@ -38343,14 +38346,14 @@ msgstr ""
 msgid "element is not a hierarchy"
 msgstr ""
 
-msgid "email '%{email}' does not match the allowed domain of %{email_domains}"
-msgid_plural "email '%{email}' does not match the allowed domains: %{email_domains}"
-msgstr[0] ""
-msgstr[1] ""
-
 msgid "email '%{email}' is not a verified email."
 msgstr ""
 
+msgid "email does not match the allowed domain of %{email_domains}"
+msgid_plural "email does not match the allowed domains: %{email_domains}"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "enabled"
 msgstr ""
 
@@ -39383,9 +39386,6 @@ msgstr ""
 msgid "to help your contributors communicate effectively!"
 msgstr ""
 
-msgid "to join %{source_name}"
-msgstr ""
-
 msgid "toggle collapse"
 msgstr ""
 

spec/controllers/invites_controller_spec.rb

@@ -24,9 +24,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' } }
 
@@ -58,34 +113,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
@@ -239,6 +266,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
 

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
 

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

spec/features/projects/pipelines/pipeline_spec.rb

@@ -361,9 +361,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

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 },

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

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