Fallback to mirror_user for PR pipelines

Pipelines are created without an user if current_user is nil
and the pipeline triggerer is listed as API.
With this commit the pipeline triggerer will fallback to the
mirror_user that set up the project.

ee/changelogs/unreleased/32801-pipeline-by-external-pr-from-github-mirror-on-12-3-has-no-user-p.yml

+---
+title: Default current user to mirror user when creating pipelines for GitHub pull requests
+merge_request: 19072
+author:
+type: fixed

ee/lib/api/project_mirror.rb

@@ -20,7 +20,7 @@ module API
       def valid_github_signature?
         request.body.rewind
 
-        token        = project.external_webhook_token
+        token        = project.external_webhook_token.to_s
         payload_body = request.body.read
         signature    = 'sha1=' + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), token, payload_body)
 
@@ -43,7 +43,7 @@ module API
       end
 
       def process_pull_request
-        external_pull_request = ProcessGithubPullRequestEventService.new(project, current_user).execute(params)
+        external_pull_request = ProcessGithubPullRequestEventService.new(project, mirror_user).execute(params)
 
         if external_pull_request
           render_validation_error!(external_pull_request)
@@ -53,10 +53,14 @@ module API
       end
 
       def start_pull_mirroring
-        result = StartPullMirroringService.new(project, current_user).execute
+        result = StartPullMirroringService.new(project, mirror_user).execute
 
         render_api_error!(result[:message], result[:http_status]) if result[:status] == :error
       end
+
+      def mirror_user
+        current_user || project.mirror_user
+      end
     end
 
     params do

ee/spec/requests/api/project_mirror_spec.rb

@@ -3,295 +3,310 @@
 require 'spec_helper'
 
 describe API::ProjectMirror do
-  let(:user) { create(:user) }
-  let(:user2) { create(:user) }
-  let(:project) { create(:project, :repository, namespace: user.namespace) }
-
   describe 'POST /projects/:id/mirror/pull' do
-    context 'when the project is not mirrored' do
-      it 'returns error' do
-        allow(project).to receive(:mirror?).and_return(false)
+    let(:visibility) { Gitlab::VisibilityLevel::PUBLIC }
+    let(:project_mirrored) { create(:project, :repository, :mirror, :import_finished, visibility: visibility) }
 
-        post api("/projects/#{project.id}/mirror/pull", user)
+    def do_post(user: nil, params: {}, headers: { 'X-Hub-Signature' => 'signature' })
+      api_path = api("/projects/#{project_mirrored.id}/mirror/pull", user)
 
-        expect(response).to have_gitlab_http_status(400)
-      end
+      post api_path, params: params, headers: headers
     end
 
-    context 'when the project is mirrored' do
+    context 'when authenticated via GitHub signature' do
       before do
-        allow_any_instance_of(Projects::UpdateMirrorService).to receive(:execute).and_return(status: :success)
+        Grape::Endpoint.before_each do |endpoint|
+          allow(endpoint).to receive(:valid_github_signature?).and_return(true)
+          allow(endpoint).to receive(:project).and_return(project_mirrored)
+        end
       end
 
-      context 'when it receives a "push" event' do
-        shared_examples_for 'an API endpoint that triggers pull mirroring operation' do
-          it 'executes UpdateAllMirrorsWorker' do
-            expect(UpdateAllMirrorsWorker).to receive(:perform_async).once
+      after do
+        Grape::Endpoint.before_each nil
+      end
 
-            post api("/projects/#{project.id}/mirror/pull", user)
+      context 'when project is not mirrored' do
+        before do
+          allow(project_mirrored).to receive(:mirror?).and_return(false)
 
-            expect(response).to have_gitlab_http_status(200)
-          end
+          do_post
         end
 
-        shared_examples_for 'an API endpoint that does not trigger pull mirroring operation' do |status_code|
-          it "does not execute UpdateAllMirrorsWorker and returns #{status_code}" do
-            expect(UpdateAllMirrorsWorker).not_to receive(:perform_async)
+        it { expect(response).to have_gitlab_http_status(:bad_request) }
+      end
 
-            post api("/projects/#{project.id}/mirror/pull", user)
+      context 'when project is mirrored' do
+        before do
+          allow_any_instance_of(Projects::UpdateMirrorService).to receive(:execute).and_return(status: :success)
+        end
 
-            expect(response).to have_gitlab_http_status(status_code)
+        context 'when "pull_request" event is received' do
+          let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) }
+          let(:branch) { project_mirrored.repository.branches.first }
+          let(:source_branch) { branch.name }
+          let(:source_sha) { branch.target }
+          let(:action) { 'opened' }
+
+          let(:params) do
+            {
+              pull_request: {
+                number: 123,
+                head: {
+                  ref: source_branch,
+                  sha: source_sha,
+                  repo: { full_name: 'the-repo' }
+                },
+                base: {
+                  ref: 'master',
+                  sha: 'a09386439ca39abe575675ffd4b89ae824fec22f',
+                  repo: { full_name: 'the-repo' }
+                }
+              },
+              action: action
+            }
           end
-        end
 
-        let(:project) do
-          create(:project, :repository, namespace: user.namespace) do |project|
-            create(:import_state, :mirror, state, project: project) do |import_state|
-              import_state.update(next_execution_timestamp: 10.minutes.from_now)
-            end
+          let(:pipeline_params) do
+            {
+              ref: Gitlab::Git::BRANCH_REF_PREFIX + branch.name,
+              source_sha: branch.target,
+              target_sha: 'a09386439ca39abe575675ffd4b89ae824fec22f'
+            }
           end
-        end
 
-        context 'when import state is none' do
-          let(:state) { :none }
+          it 'triggers a pipeline for pull request' do
+            expect(Ci::CreatePipelineService)
+              .to receive(:new)
+              .with(project_mirrored, project_mirrored.mirror_user, pipeline_params)
+              .and_return(create_pipeline_service)
 
-          it_behaves_like 'an API endpoint that triggers pull mirroring operation'
-        end
+            expect(create_pipeline_service)
+              .to receive(:execute)
+              .with(:external_pull_request_event, any_args)
+
+            do_post(params: params)
 
-        context 'when import state is failed' do
-          let(:state) { :failed }
+            expect(response).to have_gitlab_http_status(:ok)
+          end
+
+          context 'when any param is missing' do
+            let(:source_sha) { nil }
 
-          it_behaves_like 'an API endpoint that triggers pull mirroring operation'
+            it 'returns the error message' do
+              do_post(params: params)
 
-          context "and retried more than #{Gitlab::Mirror::MAX_RETRY} times" do
-            before do
-              project.import_state.update(retry_count: Gitlab::Mirror::MAX_RETRY + 1)
+              expect(response).to have_gitlab_http_status(:bad_request)
             end
+          end
 
-            it_behaves_like 'an API endpoint that does not trigger pull mirroring operation', 403
+          context 'when action is not supported' do
+            let(:action) { 'assigned' }
+
+            it 'ignores it and return success status' do
+              expect(Ci::CreatePipelineService).not_to receive(:new)
+
+              do_post(params: params)
+
+              expect(response).to have_gitlab_http_status(:unprocessable_entity)
+            end
           end
-        end
 
-        context 'when import state is finished' do
-          let(:state) { :finished }
+          context 'when authenticated as user' do
+            let(:user) { create(:user) }
 
-          it_behaves_like 'an API endpoint that triggers pull mirroring operation'
-        end
+            it 'triggers a pipeline for pull request' do
+              project_member(:maintainer, user)
 
-        context 'when import state is scheduled' do
-          let(:state) { :scheduled }
+              expect(Ci::CreatePipelineService)
+                .to receive(:new)
+                .with(project_mirrored, user, pipeline_params)
+                .and_return(create_pipeline_service)
 
-          it_behaves_like 'an API endpoint that does not trigger pull mirroring operation', 200
-        end
+              expect(create_pipeline_service)
+                .to receive(:execute)
+                .with(:external_pull_request_event, any_args)
 
-        context 'when import state is started' do
-          let(:state) { :started }
+              do_post(params: params, user: user, headers: {})
 
-          it_behaves_like 'an API endpoint that does not trigger pull mirroring operation', 200
+              expect(response).to have_gitlab_http_status(:ok)
+            end
+          end
         end
-      end
 
-      context 'when it receives a "pull_request" event' do
-        let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) }
-        let(:branch) { project.repository.branches.first }
-        let(:source_branch) { branch.name }
-        let(:source_sha) { branch.target }
-        let(:action) { 'opened' }
-
-        let(:params) do
-          {
-            pull_request: {
-              number: 123,
-              head: {
-                ref: source_branch,
-                sha: source_sha,
-                repo: { full_name: 'the-repo' }
-              },
-              base: {
-                ref: 'master',
-                sha: 'a09386439ca39abe575675ffd4b89ae824fec22f',
-                repo: { full_name: 'the-repo' }
-              }
-            },
-            action: action
-          }
-        end
+        context 'when "push" event is received' do
+          shared_examples_for 'an API endpoint that triggers pull mirroring operation' do
+            it 'executes UpdateAllMirrorsWorker' do
+              expect(project_mirrored.import_state).to receive(:force_import_job!).and_call_original
+              expect(UpdateAllMirrorsWorker).to receive(:perform_async).once
 
-        before do
-          create(:import_state, :mirror, :finished, project: project)
-        end
+              do_post
 
-        it 'triggers a pipeline for pull request' do
-          pipeline_params = {
-            ref: Gitlab::Git::BRANCH_REF_PREFIX + branch.name,
-            source_sha: branch.target,
-            target_sha: 'a09386439ca39abe575675ffd4b89ae824fec22f'
-          }
-          expect(Ci::CreatePipelineService).to receive(:new).with(project, user, pipeline_params).and_return(create_pipeline_service)
-          expect(create_pipeline_service).to receive(:execute).with(:external_pull_request_event, any_args)
+              expect(response).to have_gitlab_http_status(:ok)
+            end
+          end
 
-          post api("/projects/#{project.id}/mirror/pull", user), params: params
+          shared_examples_for 'an API endpoint that does not trigger pull mirroring operation' do |status_code|
+            it "does not execute UpdateAllMirrorsWorker and returns #{status_code}" do
+              expect(UpdateAllMirrorsWorker).not_to receive(:perform_async)
+              do_post
 
-          expect(response).to have_gitlab_http_status(200)
-        end
+              expect(response).to have_gitlab_http_status(status_code)
+            end
+          end
 
-        context 'when any param is missing' do
-          let(:source_sha) { nil }
+          let(:state) { :none }
 
-          it 'returns the error message' do
-            post api("/projects/#{project.id}/mirror/pull", user), params: params
+          before do
+            project_mirrored
+              .import_state
+              .update!(status: state, next_execution_timestamp: 10.minutes.from_now)
+          end
 
-            expect(response).to have_gitlab_http_status(400)
+          context 'when import state is none' do
+            it_behaves_like 'an API endpoint that triggers pull mirroring operation'
           end
-        end
 
-        context 'when action is not supported' do
-          let(:action) { 'assigned' }
+          context 'when import state is failed' do
+            let(:state) { :failed }
 
-          it 'ignores it and return success status' do
-            expect(Ci::CreatePipelineService).not_to receive(:new)
+            it_behaves_like 'an API endpoint that triggers pull mirroring operation'
 
-            post api("/projects/#{project.id}/mirror/pull", user), params: params
+            context "and retried more than #{Gitlab::Mirror::MAX_RETRY} times" do
+              before do
+                project_mirrored
+                  .import_state
+                  .update!(retry_count: Gitlab::Mirror::MAX_RETRY + 1)
+              end
 
-            expect(response).to have_gitlab_http_status(422)
+              it_behaves_like 'an API endpoint that does not trigger pull mirroring operation', :forbidden
+            end
           end
-        end
-      end
 
-      context 'when user' do
-        let(:project_mirrored) { create(:project, :repository, :mirror, :import_finished, namespace: user.namespace) }
+          context 'when import state is finished' do
+            let(:state) { :finished }
 
-        def project_member(role, user)
-          create(:project_member, role, user: user, project: project_mirrored)
-        end
+            it_behaves_like 'an API endpoint that triggers pull mirroring operation'
+          end
 
-        context 'is unauthenticated' do
-          it 'returns authentication error' do
-            post api("/projects/#{project_mirrored.id}/mirror/pull")
+          context 'when import state is scheduled' do
+            let(:state) { :scheduled }
 
-            expect(response).to have_gitlab_http_status(401)
+            it_behaves_like 'an API endpoint that does not trigger pull mirroring operation', :ok
           end
-        end
 
-        context 'is authenticated as developer' do
-          it 'returns forbidden error' do
-            project_member(:developer, user2)
+          context 'when import state is started' do
+            let(:state) { :started }
 
-            post api("/projects/#{project_mirrored.id}/mirror/pull", user2)
-
-            expect(response).to have_gitlab_http_status(403)
+            it_behaves_like 'an API endpoint that does not trigger pull mirroring operation', :ok
           end
-        end
 
-        context 'is authenticated as reporter' do
-          it 'returns forbidden error' do
-            project_member(:reporter, user2)
+          context 'when authenticated as user' do
+            let(:user) { create(:user) }
 
-            post api("/projects/#{project_mirrored.id}/mirror/pull", user2)
+            context 'is authenticated as developer' do
+              it 'returns forbidden error' do
+                project_member(:developer, user)
 
-            expect(response).to have_gitlab_http_status(403)
-          end
-        end
+                do_post(user: user, headers: {})
 
-        context 'is authenticated as guest' do
-          it 'returns forbidden error' do
-            project_member(:guest, user2)
+                expect(response).to have_gitlab_http_status(:forbidden)
+              end
+            end
 
-            post api("/projects/#{project_mirrored.id}/mirror/pull", user2)
+            context 'is authenticated as reporter' do
+              it 'returns forbidden error' do
+                project_member(:reporter, user)
 
-            expect(response).to have_gitlab_http_status(403)
-          end
-        end
+                do_post(user: user, headers: {})
 
-        context 'is authenticated as maintainer' do
-          it 'triggers the pull mirroring operation' do
-            project_member(:maintainer, user2)
+                expect(response).to have_gitlab_http_status(:forbidden)
+              end
+            end
 
-            post api("/projects/#{project_mirrored.id}/mirror/pull", user2)
+            context 'is authenticated as guest' do
+              it 'returns forbidden error' do
+                project_member(:guest, user)
 
-            expect(response).to have_gitlab_http_status(200)
-          end
-        end
+                do_post(user: user, headers: {})
 
-        context 'is authenticated as owner' do
-          it 'triggers the pull mirroring operation' do
-            post api("/projects/#{project_mirrored.id}/mirror/pull", user)
+                expect(response).to have_gitlab_http_status(:forbidden)
+              end
+            end
 
-            expect(response).to have_gitlab_http_status(200)
-          end
-        end
-      end
+            context 'is authenticated as maintainer' do
+              it 'triggers the pull mirroring operation' do
+                project_member(:maintainer, user)
 
-      context 'authenticating from GitHub signature' do
-        let(:visibility) { Gitlab::VisibilityLevel::PUBLIC }
-        let(:project_mirrored) { create(:project, :repository, :mirror, :import_finished, visibility: visibility) }
+                expect(StartPullMirroringService)
+                  .to receive(:new)
+                  .with(project_mirrored, user)
+                  .and_call_original
 
-        def do_post
-          post api("/projects/#{project_mirrored.id}/mirror/pull"), params: {}, headers: { 'X-Hub-Signature' => 'signature' }
-        end
+                do_post(user: user, headers: {})
 
-        context "when it's valid" do
-          before do
-            Grape::Endpoint.before_each do |endpoint|
-              allow(endpoint).to receive(:project).and_return(project_mirrored)
-              allow(endpoint).to receive(:valid_github_signature?).and_return(true)
+                expect(response).to have_gitlab_http_status(:ok)
+              end
             end
-          end
 
-          it 'syncs the mirror' do
-            expect(project_mirrored.import_state).to receive(:force_import_job!)
+            context 'is authenticated as owner' do
+              it 'triggers the pull mirroring operation' do
+                expect(StartPullMirroringService)
+                  .to receive(:new)
+                  .with(project_mirrored, project_mirrored.creator)
+                  .and_call_original
 
-            do_post
-          end
-        end
+                do_post(user: project_mirrored.creator, headers: {})
 
-        context "when it's invalid" do
-          before do
-            Grape::Endpoint.before_each do |endpoint|
-              allow(endpoint).to receive(:project).and_return(project_mirrored)
-              allow(endpoint).to receive(:valid_github_signature?).and_return(false)
+                expect(response).to have_gitlab_http_status(:ok)
+              end
             end
           end
+        end
 
-          after do
-            Grape::Endpoint.before_each nil
-          end
+        def project_member(role, user)
+          create(:project_member, role, user: user, project: project_mirrored)
+        end
+      end
+    end
 
-          it "doesn't sync the mirror" do
-            expect(project_mirrored.import_state).not_to receive(:force_import_job!)
+    context 'when not authenticated' do
+      before do
+        Grape::Endpoint.before_each do |endpoint|
+          allow(endpoint).to receive(:valid_github_signature?).and_return(false)
+        end
+      end
 
-            post api("/projects/#{project_mirrored.id}/mirror/pull"), params: {}, headers: { 'X-Hub-Signature' => 'signature' }
-          end
+      after do
+        Grape::Endpoint.before_each nil
+      end
 
-          context 'with a public project' do
-            let(:visibility) { Gitlab::VisibilityLevel::PUBLIC }
+      context 'with public project' do
+        let(:visibility) { Gitlab::VisibilityLevel::PUBLIC }
 
-            it 'returns a 401 status' do
-              do_post
+        it 'returns a 401 status' do
+          do_post
 
-              expect(response).to have_gitlab_http_status(401)
-            end
-          end
+          expect(response).to have_gitlab_http_status(:unauthorized)
+        end
+      end
 
-          context 'with an internal project' do
-            let(:visibility) { Gitlab::VisibilityLevel::INTERNAL }
+      context 'with internal project' do
+        let(:visibility) { Gitlab::VisibilityLevel::INTERNAL }
 
-            it 'returns a 404 status' do
-              do_post
+        it 'returns a 404 status' do
+          do_post
 
-              expect(response).to have_gitlab_http_status(404)
-            end
-          end
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
 
-          context 'with a private project' do
-            let(:visibility) { Gitlab::VisibilityLevel::PRIVATE }
+      context 'with private project' do
+        let(:visibility) { Gitlab::VisibilityLevel::PRIVATE }
 
-            it 'returns a 404 status' do
-              do_post
+        it 'returns a 404 status' do
+          do_post
 
-              expect(response).to have_gitlab_http_status(404)
-            end
-          end
+          expect(response).to have_gitlab_http_status(:not_found)
         end
       end
     end