Merge branch 'ide-merge-requests-forks' into 'master'

Allow merge requests from forks to be opened in Web IDE

Closes #47460

See merge request gitlab-org/gitlab-ce!20521

12 files changed, 161 insertions, 20 deletions

diff --git a/app/assets/javascripts/ide/ide_router.js b/app/assets/javascripts/ide/ide_router.js
index cc8dbb942d8..44c35e9a5a5 100644
--- a/app/assets/javascripts/ide/ide_router.js
+++ b/app/assets/javascripts/ide/ide_router.js
@@ -101,6 +101,7 @@ router.beforeEach((to, from, next) => {
           store
             .dispatch('getMergeRequestData', {
               projectId: fullProjectId,
+              targetProjectId: to.query.target_project,
               mergeRequestId: to.params.mrid,
             })
             .then(mr => {
@@ -119,12 +120,14 @@ router.beforeEach((to, from, next) => {
             .then(() =>
               store.dispatch('getMergeRequestVersions', {
                 projectId: fullProjectId,
+                targetProjectId: to.query.target_project,
                 mergeRequestId: to.params.mrid,
               }),
             )
             .then(() =>
               store.dispatch('getMergeRequestChanges', {
                 projectId: fullProjectId,
+                targetProjectId: to.query.target_project,
                 mergeRequestId: to.params.mrid,
               }),
             )
diff --git a/app/assets/javascripts/ide/stores/actions/merge_request.js b/app/assets/javascripts/ide/stores/actions/merge_request.js
index 6bdf9dc3028..1887b77b00b 100644
--- a/app/assets/javascripts/ide/stores/actions/merge_request.js
+++ b/app/assets/javascripts/ide/stores/actions/merge_request.js
@@ -4,12 +4,14 @@ import * as types from '../mutation_types';
 
 export const getMergeRequestData = (
   { commit, dispatch, state },
-  { projectId, mergeRequestId, force = false } = {},
+  { projectId, mergeRequestId, targetProjectId = null, force = false } = {},
 ) =>
   new Promise((resolve, reject) => {
     if (!state.projects[projectId].mergeRequests[mergeRequestId] || force) {
       service
-        .getProjectMergeRequestData(projectId, mergeRequestId, { render_html: true })
+        .getProjectMergeRequestData(targetProjectId || projectId, mergeRequestId, {
+          render_html: true,
+        })
         .then(({ data }) => {
           commit(types.SET_MERGE_REQUEST, {
             projectPath: projectId,
@@ -38,12 +40,12 @@ export const getMergeRequestData = (
 
 export const getMergeRequestChanges = (
   { commit, dispatch, state },
-  { projectId, mergeRequestId, force = false } = {},
+  { projectId, mergeRequestId, targetProjectId = null, force = false } = {},
 ) =>
   new Promise((resolve, reject) => {
     if (!state.projects[projectId].mergeRequests[mergeRequestId].changes.length || force) {
       service
-        .getProjectMergeRequestChanges(projectId, mergeRequestId)
+        .getProjectMergeRequestChanges(targetProjectId || projectId, mergeRequestId)
         .then(({ data }) => {
           commit(types.SET_MERGE_REQUEST_CHANGES, {
             projectPath: projectId,
@@ -71,12 +73,12 @@ export const getMergeRequestChanges = (
 
 export const getMergeRequestVersions = (
   { commit, dispatch, state },
-  { projectId, mergeRequestId, force = false } = {},
+  { projectId, mergeRequestId, targetProjectId = null, force = false } = {},
 ) =>
   new Promise((resolve, reject) => {
     if (!state.projects[projectId].mergeRequests[mergeRequestId].versions.length || force) {
       service
-        .getProjectMergeRequestVersions(projectId, mergeRequestId)
+        .getProjectMergeRequestVersions(targetProjectId || projectId, mergeRequestId)
         .then(res => res.data)
         .then(data => {
           commit(types.SET_MERGE_REQUEST_VERSIONS, {
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue
index c18b74743e4..a4c2289c590 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue
@@ -1,7 +1,7 @@
 <script>
 import tooltip from '~/vue_shared/directives/tooltip';
 import { n__ } from '~/locale';
-import { webIDEUrl } from '~/lib/utils/url_utility';
+import { mergeUrlParams, webIDEUrl } from '~/lib/utils/url_utility';
 import Icon from '~/vue_shared/components/icon.vue';
 import clipboardButton from '~/vue_shared/components/clipboard_button.vue';
 
@@ -43,7 +43,10 @@ export default {
       return this.isBranchTitleLong(this.mr.targetBranch);
     },
     webIdePath() {
-      return webIDEUrl(this.mr.statusPath.replace('.json', ''));
+      return mergeUrlParams({
+        target_project: this.mr.sourceProjectFullPath !== this.mr.targetProjectFullPath ?
+          this.mr.targetProjectFullPath : '',
+      }, webIDEUrl(`/${this.mr.sourceProjectFullPath}/merge_requests/${this.mr.iid}`));
     },
   },
   methods: {
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
index c881cd496d1..e84c436905d 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
@@ -16,10 +16,11 @@ export default class MergeRequestStore {
     const pipelineStatus = data.pipeline ? data.pipeline.details.status : null;
 
     this.squash = data.squash;
-    this.squashBeforeMergeHelpPath = this.squashBeforeMergeHelpPath ||
-      data.squash_before_merge_help_path;
+    this.squashBeforeMergeHelpPath =
+      this.squashBeforeMergeHelpPath || data.squash_before_merge_help_path;
     this.enableSquashBeforeMerge = this.enableSquashBeforeMerge || true;
 
+    this.iid = data.iid;
     this.title = data.title;
     this.targetBranch = data.target_branch;
     this.sourceBranch = data.source_branch;
@@ -85,6 +86,8 @@ export default class MergeRequestStore {
     this.isMergeAllowed = data.mergeable || false;
     this.mergeOngoing = data.merge_ongoing;
     this.allowCollaboration = data.allow_collaboration;
+    this.targetProjectFullPath = data.target_project_full_path;
+    this.sourceProjectFullPath = data.source_project_full_path;
 
     // Cherry-pick and Revert actions related
     this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false;
@@ -97,7 +100,8 @@ export default class MergeRequestStore {
     this.hasCI = data.has_ci;
     this.ciStatus = data.ci_status;
     this.isPipelineFailed = this.ciStatus === 'failed' || this.ciStatus === 'canceled';
-    this.isPipelinePassing = this.ciStatus === 'success' || this.ciStatus === 'success_with_warnings';
+    this.isPipelinePassing =
+      this.ciStatus === 'success' || this.ciStatus === 'success_with_warnings';
     this.isPipelineSkipped = this.ciStatus === 'skipped';
     this.pipelineDetailedStatus = pipelineStatus;
     this.isPipelineActive = data.pipeline ? data.pipeline.active : false;
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
index 5d72ebdd7fd..a78bd77cf7c 100644
--- a/app/serializers/merge_request_widget_entity.rb
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -10,9 +10,15 @@ class MergeRequestWidgetEntity < IssuableEntity
   expose :merge_when_pipeline_succeeds
   expose :source_branch
   expose :source_project_id
+  expose :source_project_full_path do |merge_request|
+    merge_request.source_project&.full_path
+  end
   expose :squash
   expose :target_branch
   expose :target_project_id
+  expose :target_project_full_path do |merge_request|
+    merge_request.project&.full_path
+  end
   expose :allow_collaboration
 
   expose :should_be_rebased?, as: :should_be_rebased
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index 964780cba6a..92329465b2c 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -6,6 +6,18 @@ module API
 
     before { authorize! :download_code, user_project }
 
+    helpers do
+      def user_access
+        @user_access ||= Gitlab::UserAccess.new(current_user, project: user_project)
+      end
+
+      def authorize_push_to_branch!(branch)
+        unless user_access.can_push_to_branch?(branch)
+          forbidden!("You are not allowed to push into this branch")
+        end
+      end
+    end
+
     params do
       requires :id, type: String, desc: 'The ID of a project'
     end
@@ -67,7 +79,7 @@ module API
         optional :author_name, type: String, desc: 'Author name for commit'
       end
       post ':id/repository/commits' do
-        authorize! :push_code, user_project
+        authorize_push_to_branch!(params[:branch])
 
         attrs = declared_params
         attrs[:branch_name] = attrs.delete(:branch)
@@ -142,7 +154,7 @@ module API
         requires :branch, type: String, desc: 'The name of the branch'
       end
       post ':id/repository/commits/:sha/cherry_pick', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do
-        authorize! :push_code, user_project
+        authorize_push_to_branch!(params[:branch])
 
         commit = user_project.commit(params[:sha])
         not_found!('Commit') unless commit
diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json
index 38ce92a5dc7..3b741d51598 100644
--- a/spec/fixtures/api/schemas/entities/merge_request_widget.json
+++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json
@@ -29,8 +29,10 @@
     "merge_when_pipeline_succeeds": { "type": "boolean" },
     "source_branch": { "type": "string" },
     "source_project_id": { "type": "integer" },
+    "source_project_full_path": { "type": ["string", "null"]},
     "target_branch": { "type": "string" },
     "target_project_id": { "type": "integer" },
+    "target_project_full_path": { "type": ["string", "null"]},
     "allow_collaboration": { "type": "boolean"},
     "metrics": {
       "oneOf": [
diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js
index 61b7bd2c226..8ac2f26979b 100644
--- a/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js
@@ -119,6 +119,7 @@ describe('MRWidgetHeader', () => {
       beforeEach(() => {
         vm = mountComponent(Component, {
           mr: {
+            iid: 1,
             divergedCommitsCount: 12,
             sourceBranch: 'mr-widget-refactor',
             sourceBranchLink: '<a href="/foo/bar/mr-widget-refactor">mr-widget-refactor</a>',
@@ -130,6 +131,8 @@ describe('MRWidgetHeader', () => {
             emailPatchesPath: '/mr/email-patches',
             plainDiffPath: '/mr/plainDiffPath',
             statusPath: 'abc',
+            sourceProjectFullPath: 'root/gitlab-ce',
+            targetProjectFullPath: 'gitlab-org/gitlab-ce',
           },
         });
       });
@@ -146,16 +149,40 @@ describe('MRWidgetHeader', () => {
         const button = vm.$el.querySelector('.js-web-ide');
 
         expect(button.textContent.trim()).toEqual('Open in Web IDE');
-        expect(button.getAttribute('href')).toEqual('/-/ide/projectabc');
+        expect(button.getAttribute('href')).toEqual(
+          '/-/ide/project/root/gitlab-ce/merge_requests/1?target_project=gitlab-org%2Fgitlab-ce',
+        );
       });
 
-      it('renders web ide button with relative URL', () => {
+      it('renders web ide button with blank query string if target & source project branch', done => {
+        vm.mr.targetProjectFullPath = 'root/gitlab-ce';
+
+        vm.$nextTick(() => {
+          const button = vm.$el.querySelector('.js-web-ide');
+
+          expect(button.textContent.trim()).toEqual('Open in Web IDE');
+          expect(button.getAttribute('href')).toEqual(
+            '/-/ide/project/root/gitlab-ce/merge_requests/1?target_project=',
+          );
+
+          done();
+        });
+      });
+
+      it('renders web ide button with relative URL', done => {
         gon.relative_url_root = '/gitlab';
+        vm.mr.iid = 2;
 
-        const button = vm.$el.querySelector('.js-web-ide');
+        vm.$nextTick(() => {
+          const button = vm.$el.querySelector('.js-web-ide');
 
-        expect(button.textContent.trim()).toEqual('Open in Web IDE');
-        expect(button.getAttribute('href')).toEqual('/-/ide/projectabc');
+          expect(button.textContent.trim()).toEqual('Open in Web IDE');
+          expect(button.getAttribute('href')).toEqual(
+            '/gitlab/-/ide/project/root/gitlab-ce/merge_requests/2?target_project=gitlab-org%2Fgitlab-ce',
+          );
+
+          done();
+        });
       });
 
       it('renders download dropdown with links', () => {
diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js
index 9d2a15ff009..c0b5a7d4455 100644
--- a/spec/javascripts/vue_mr_widget/mock_data.js
+++ b/spec/javascripts/vue_mr_widget/mock_data.js
@@ -29,8 +29,10 @@ export default {
   source_branch: 'daaaa',
   source_branch_link: 'daaaa',
   source_project_id: 19,
+  source_project_full_path: '/group1/project1',
   target_branch: 'master',
   target_project_id: 19,
+  target_project_full_path: '/group2/project2',
   metrics: {
     merged_by: {
       name: 'Administrator',
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 113703fac38..246947e58a8 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -514,6 +514,38 @@ describe API::Commits do
         expect(response).to have_gitlab_http_status(400)
       end
     end
+
+    context 'when committing into a fork as a maintainer' do
+      include_context 'merge request allowing collaboration'
+
+      let(:project_id) { forked_project.id }
+
+      def push_params(branch_name)
+        {
+          branch: branch_name,
+          commit_message: 'Hello world',
+          actions: [
+            {
+              action: 'create',
+              file_path: 'foo/bar/baz.txt',
+              content: 'puts 8'
+            }
+          ]
+        }
+      end
+
+      it 'allows pushing to the source branch of the merge request' do
+        post api(url, user), push_params('feature')
+
+        expect(response).to have_gitlab_http_status(:created)
+      end
+
+      it 'denies pushing to another branch' do
+        post api(url, user), push_params('other-branch')
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
   end
 
   describe 'GET /projects/:id/repository/commits/:sha/refs' do
@@ -1065,11 +1097,29 @@ describe API::Commits do
         it 'returns 400 if you are not allowed to push to the target branch' do
           post api(route, current_user), branch: 'feature'
 
-          expect(response).to have_gitlab_http_status(400)
-          expect(json_response['message']).to eq('You are not allowed to push into this branch')
+          expect(response).to have_gitlab_http_status(:forbidden)
+          expect(json_response['message']).to match(/You are not allowed to push into this branch/)
         end
       end
     end
+
+    context 'when cherry picking to a fork as a maintainer' do
+      include_context 'merge request allowing collaboration'
+
+      let(:project_id) { forked_project.id }
+
+      it 'allows access from a maintainer that to the source branch' do
+        post api(route, user), branch: 'feature'
+
+        expect(response).to have_gitlab_http_status(:created)
+      end
+
+      it 'denies cherry picking to another branch' do
+        post api(route, user), branch: 'master'
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
   end
 
   describe 'POST /projects/:id/repository/commits/:sha/comments' do
diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb
index d2072198d83..0ba2539a717 100644
--- a/spec/serializers/merge_request_widget_entity_spec.rb
+++ b/spec/serializers/merge_request_widget_entity_spec.rb
@@ -11,6 +11,21 @@ describe MergeRequestWidgetEntity do
     described_class.new(resource, request: request).as_json
   end
 
+  describe 'source_project_full_path' do
+    it 'includes the full path of the source project' do
+      expect(subject[:source_project_full_path]).to be_present
+    end
+
+    context 'when the source project is missing' do
+      it 'returns `nil` for the source project' do
+        resource.allow_broken = true
+        resource.update!(source_project: nil)
+
+        expect(subject[:source_project_full_path]).to be_nil
+      end
+    end
+  end
+
   describe 'pipeline' do
     let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) }
 
diff --git a/spec/support/shared_contexts/merge_requests_allowing_collaboration.rb b/spec/support/shared_contexts/merge_requests_allowing_collaboration.rb
new file mode 100644
index 00000000000..05424d08b9d
--- /dev/null
+++ b/spec/support/shared_contexts/merge_requests_allowing_collaboration.rb
@@ -0,0 +1,15 @@
+shared_context 'merge request allowing collaboration' do
+  include ProjectForksHelper
+
+  let(:canonical) { create(:project, :public, :repository) }
+  let(:forked_project) { fork_project(canonical, nil, repository: true) }
+
+  before do
+    canonical.add_maintainer(user)
+    create(:merge_request,
+           target_project: canonical,
+           source_project: forked_project,
+           source_branch: 'feature',
+           allow_collaboration: true)
+  end
+end