From 77c35d5d001a0ce0626bc8aeec574eca36c2233b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 5 Jul 2019 11:14:56 +0100 Subject: [PATCH] Create private merge requests in forks https://gitlab.com/gitlab-org/gitlab-ce/issues/58583 --- .../components/dropdown.vue | 58 ++++++++ .../components/project_form_group.vue | 136 ++++++++++++++++++ .../confidential_merge_request/index.js | 30 ++++ .../confidential_merge_request/state.js | 5 + .../create_merge_request_dropdown.js | 47 +++++- .../stylesheets/framework/dropdowns.scss | 4 +- .../projects/branches_controller.rb | 2 +- app/controllers/projects/issues_controller.rb | 2 +- app/helpers/issues_helper.rb | 2 +- .../projects/issues/_new_branch.html.haml | 13 +- locale/gitlab.pot | 18 +++ ...creates_confidential_merge_request_spec.rb | 54 +++++++ .../project_form_group_spec.js.snap | 101 +++++++++++++ .../components/dropdown_spec.js | 56 ++++++++ .../components/project_form_group_spec.js | 77 ++++++++++ .../create_merge_request_dropdown_spec.js | 9 +- 16 files changed, 599 insertions(+), 15 deletions(-) create mode 100644 app/assets/javascripts/confidential_merge_request/components/dropdown.vue create mode 100644 app/assets/javascripts/confidential_merge_request/components/project_form_group.vue create mode 100644 app/assets/javascripts/confidential_merge_request/index.js create mode 100644 app/assets/javascripts/confidential_merge_request/state.js create mode 100644 spec/features/issues/user_creates_confidential_merge_request_spec.rb create mode 100644 spec/frontend/confidential_merge_request/components/__snapshots__/project_form_group_spec.js.snap create mode 100644 spec/frontend/confidential_merge_request/components/dropdown_spec.js create mode 100644 spec/frontend/confidential_merge_request/components/project_form_group_spec.js rename spec/{javascripts => frontend}/create_merge_request_dropdown_spec.js (92%) diff --git a/app/assets/javascripts/confidential_merge_request/components/dropdown.vue b/app/assets/javascripts/confidential_merge_request/components/dropdown.vue new file mode 100644 index 00000000000..444640980af --- /dev/null +++ b/app/assets/javascripts/confidential_merge_request/components/dropdown.vue @@ -0,0 +1,58 @@ + + + diff --git a/app/assets/javascripts/confidential_merge_request/components/project_form_group.vue b/app/assets/javascripts/confidential_merge_request/components/project_form_group.vue new file mode 100644 index 00000000000..b89729375be --- /dev/null +++ b/app/assets/javascripts/confidential_merge_request/components/project_form_group.vue @@ -0,0 +1,136 @@ + + + diff --git a/app/assets/javascripts/confidential_merge_request/index.js b/app/assets/javascripts/confidential_merge_request/index.js new file mode 100644 index 00000000000..9672821d30e --- /dev/null +++ b/app/assets/javascripts/confidential_merge_request/index.js @@ -0,0 +1,30 @@ +import Vue from 'vue'; +import { parseBoolean } from '../lib/utils/common_utils'; +import ProjectFormGroup from './components/project_form_group.vue'; +import state from './state'; + +export function isConfidentialIssue() { + return parseBoolean(document.querySelector('.js-create-mr').dataset.isConfidential); +} + +export function canCreateConfidentialMergeRequest() { + return isConfidentialIssue() && Object.keys(state.selectedProject).length > 0; +} + +export function init() { + const el = document.getElementById('js-forked-project'); + + return new Vue({ + el, + render(h) { + return h(ProjectFormGroup, { + props: { + namespacePath: el.dataset.namespacePath, + projectPath: el.dataset.projectPath, + newForkPath: el.dataset.newForkPath, + helpPagePath: el.dataset.helpPagePath, + }, + }); + }, + }); +} diff --git a/app/assets/javascripts/confidential_merge_request/state.js b/app/assets/javascripts/confidential_merge_request/state.js new file mode 100644 index 00000000000..95b0580f4b9 --- /dev/null +++ b/app/assets/javascripts/confidential_merge_request/state.js @@ -0,0 +1,5 @@ +import Vue from 'vue'; + +export default Vue.observable({ + selectedProject: {}, +}); diff --git a/app/assets/javascripts/create_merge_request_dropdown.js b/app/assets/javascripts/create_merge_request_dropdown.js index 8f5cece0788..052168bb21c 100644 --- a/app/assets/javascripts/create_merge_request_dropdown.js +++ b/app/assets/javascripts/create_merge_request_dropdown.js @@ -5,6 +5,12 @@ import Flash from './flash'; import DropLab from './droplab/drop_lab'; import ISetter from './droplab/plugins/input_setter'; import { __, sprintf } from './locale'; +import { + init as initConfidentialMergeRequest, + isConfidentialIssue, + canCreateConfidentialMergeRequest, +} from './confidential_merge_request'; +import confidentialMergeRequestState from './confidential_merge_request/state'; // Todo: Remove this when fixing issue in input_setter plugin const InputSetter = Object.assign({}, ISetter); @@ -12,6 +18,17 @@ const InputSetter = Object.assign({}, ISetter); const CREATE_MERGE_REQUEST = 'create-mr'; const CREATE_BRANCH = 'create-branch'; +function createEndpoint(projectPath, endpoint) { + if (canCreateConfidentialMergeRequest()) { + return endpoint.replace( + projectPath, + confidentialMergeRequestState.selectedProject.pathWithNamespace, + ); + } + + return endpoint; +} + export default class CreateMergeRequestDropdown { constructor(wrapperEl) { this.wrapperEl = wrapperEl; @@ -42,6 +59,8 @@ export default class CreateMergeRequestDropdown { this.refIsValid = true; this.refsPath = this.wrapperEl.dataset.refsPath; this.suggestedRef = this.refInput.value; + this.projectPath = this.wrapperEl.dataset.projectPath; + this.projectId = this.wrapperEl.dataset.projectId; // These regexps are used to replace // a backend generated new branch name and its source (ref) @@ -58,6 +77,14 @@ export default class CreateMergeRequestDropdown { }; this.init(); + + if (isConfidentialIssue()) { + this.createMergeRequestButton.setAttribute( + 'data-dropdown-trigger', + '#create-merge-request-dropdown', + ); + initConfidentialMergeRequest(); + } } available() { @@ -113,7 +140,9 @@ export default class CreateMergeRequestDropdown { this.isCreatingBranch = true; return axios - .post(this.createBranchPath) + .post(createEndpoint(this.projectPath, this.createBranchPath), { + confidential_issue_project_id: canCreateConfidentialMergeRequest() ? this.projectId : null, + }) .then(({ data }) => { this.branchCreated = true; window.location.href = data.url; @@ -125,7 +154,11 @@ export default class CreateMergeRequestDropdown { this.isCreatingMergeRequest = true; return axios - .post(this.createMrPath) + .post(this.createMrPath, { + target_project_id: canCreateConfidentialMergeRequest() + ? confidentialMergeRequestState.selectedProject.id + : null, + }) .then(({ data }) => { this.mergeRequestCreated = true; window.location.href = data.url; @@ -149,6 +182,8 @@ export default class CreateMergeRequestDropdown { } enable() { + if (!canCreateConfidentialMergeRequest()) return; + this.createMergeRequestButton.classList.remove('disabled'); this.createMergeRequestButton.removeAttribute('disabled'); @@ -205,7 +240,7 @@ export default class CreateMergeRequestDropdown { if (!ref) return false; return axios - .get(`${this.refsPath}${encodeURIComponent(ref)}`) + .get(`${createEndpoint(this.projectPath, this.refsPath)}${encodeURIComponent(ref)}`) .then(({ data }) => { const branches = data[Object.keys(data)[0]]; const tags = data[Object.keys(data)[1]]; @@ -325,6 +360,12 @@ export default class CreateMergeRequestDropdown { let xhr = null; event.preventDefault(); + if (isConfidentialIssue() && !event.target.classList.contains('js-create-target')) { + this.droplab.hooks.forEach(hook => hook.list.toggle()); + + return; + } + if (this.isBusy()) { return; } diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index cd951f67293..a12029d2419 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -287,8 +287,8 @@ list-style: none; padding: 0 1px; - a, - button, + a:not(.help-link), + button:not(.btn), .menu-item { @include dropdown-link; } diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index d77f64a84f5..141a7dfb923 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -169,7 +169,7 @@ class Projects::BranchesController < Projects::ApplicationController end def confidential_issue_project - return unless Feature.enabled?(:create_confidential_merge_request, @project) + return unless helpers.create_confidential_merge_request_enabled? return if params[:confidential_issue_project_id].blank? confidential_issue_project = Project.find(params[:confidential_issue_project_id]) diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index e275b417784..b866f574f67 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -172,7 +172,7 @@ class Projects::IssuesController < Projects::ApplicationController def create_merge_request create_params = params.slice(:branch_name, :ref).merge(issue_iid: issue.iid) - create_params[:target_project_id] = params[:target_project_id] if Feature.enabled?(:create_confidential_merge_request, @project) + create_params[:target_project_id] = params[:target_project_id] if helpers.create_confidential_merge_request_enabled? result = ::MergeRequests::CreateFromIssueService.new(project, current_user, create_params).execute if result[:status] == :success diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index dfadcfc33b2..5476a7cdff6 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -137,7 +137,7 @@ module IssuesHelper end def create_confidential_merge_request_enabled? - Feature.enabled?(:create_confidential_merge_request, @project) + Feature.enabled?(:create_confidential_merge_request, @project, default_enabled: true) end def show_new_branch_button? diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml index 52bb797b5b3..8d3e54dc455 100644 --- a/app/views/projects/issues/_new_branch.html.haml +++ b/app/views/projects/issues/_new_branch.html.haml @@ -3,13 +3,14 @@ - data_action = can_create_merge_request ? 'create-mr' : 'create-branch' - value = can_create_merge_request ? 'Create merge request' : 'Create branch' - value = can_create_confidential_merge_request? ? _('Create confidential merge request') : value + - create_mr_text = can_create_confidential_merge_request? ? _('Create confidential merge request') : _('Create merge request') - can_create_path = can_create_branch_project_issue_path(@project, @issue) - create_mr_path = create_merge_request_project_issue_path(@project, @issue, branch_name: @issue.to_branch_name, ref: @project.default_branch) - create_branch_path = project_branches_path(@project, branch_name: @issue.to_branch_name, ref: @project.default_branch, issue_iid: @issue.iid) - refs_path = refs_namespace_project_path(@project.namespace, @project, search: '') - .create-mr-dropdown-wrap.d-inline-block.full-width-mobile{ data: { can_create_path: can_create_path, create_mr_path: create_mr_path, create_branch_path: create_branch_path, refs_path: refs_path } } + .create-mr-dropdown-wrap.d-inline-block.full-width-mobile.js-create-mr{ data: { project_path: @project.full_path, project_id: @project.id, can_create_path: can_create_path, create_mr_path: create_mr_path, create_branch_path: create_branch_path, refs_path: refs_path, is_confidential: can_create_confidential_merge_request?.to_s } } .btn-group.btn-group-sm.unavailable %button.btn.btn-grouped{ type: 'button', disabled: 'disabled' } = icon('spinner', class: 'fa-spin') @@ -26,7 +27,7 @@ .droplab-dropdown %ul#create-merge-request-dropdown.create-merge-request-dropdown-menu.dropdown-menu.dropdown-menu-right.gl-show-field-errors{ class: ("create-confidential-merge-request-dropdown-menu" if can_create_confidential_merge_request?), data: { dropdown: true } } - if can_create_merge_request - %li.droplab-item-selected{ role: 'button', data: { value: 'create-mr', text: _('Create merge request') } } + %li.droplab-item-selected{ role: 'button', data: { value: 'create-mr', text: create_mr_text } } .menu-item = icon('check', class: 'icon') - if can_create_confidential_merge_request? @@ -41,6 +42,8 @@ %li.divider.droplab-item-ignore %li.droplab-item-ignore.prepend-left-8.append-right-8.prepend-top-16 + - if can_create_confidential_merge_request? + #js-forked-project{ data: { namespace_path: @project.namespace.full_path, project_path: @project.full_path, new_fork_path: new_project_fork_path(@project), help_page_path: help_page_path('user/project/merge_requests') } } .form-group %label{ for: 'new-branch-name' } = _('Branch name') @@ -55,4 +58,8 @@ .form-group %button.btn.btn-success.js-create-target{ type: 'button', data: { action: 'create-mr' } } - = _('Create merge request') + = create_mr_text + + - if can_create_confidential_merge_request? + %p.text-warning.js-exposed-info-warning.hidden + = _('This may expose confidential information as the selected fork is in another namespace that can have other members.') diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8a4f57c5b13..05d291022dc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4169,6 +4169,9 @@ msgstr "" msgid "Error fetching diverging counts for branches. Please try again." msgstr "" +msgid "Error fetching forked projects. Please try again." +msgstr "" + msgid "Error fetching labels." msgstr "" @@ -6825,6 +6828,9 @@ msgstr "" msgid "No files found." msgstr "" +msgid "No forks available to you." +msgstr "" + msgid "No job trace" msgstr "" @@ -9192,6 +9198,9 @@ msgstr "" msgid "Select members to invite" msgstr "" +msgid "Select private project" +msgstr "" + msgid "Select project" msgstr "" @@ -10753,6 +10762,9 @@ msgstr "" msgid "This job will automatically run after its timer finishes. Often they are used for incremental roll-out deploys to production environments. When unscheduled it converts into a manual action." msgstr "" +msgid "This may expose confidential information as the selected fork is in another namespace that can have other members." +msgstr "" + msgid "This means you can not push code until you create an empty repository or import existing one." msgstr "" @@ -11084,6 +11096,12 @@ msgstr "" msgid "To preserve performance only %{display_size} of %{real_size} files are displayed." msgstr "" +msgid "To protect this issues confidentiality, %{link_start}fork the project%{link_end} and set the forks visiblity to private." +msgstr "" + +msgid "To protect this issues confidentiality, a private fork of this project was selected." +msgstr "" + msgid "To see all the user's personal access tokens you must impersonate them first." msgstr "" diff --git a/spec/features/issues/user_creates_confidential_merge_request_spec.rb b/spec/features/issues/user_creates_confidential_merge_request_spec.rb new file mode 100644 index 00000000000..7ae4af4667b --- /dev/null +++ b/spec/features/issues/user_creates_confidential_merge_request_spec.rb @@ -0,0 +1,54 @@ +require 'rails_helper' + +describe 'User creates confidential merge request on issue page', :js do + include ProjectForksHelper + + let(:user) { create(:user) } + let(:project) { create(:project, :repository, :public) } + let(:issue) { create(:issue, project: project, confidential: true) } + + def visit_confidential_issue + sign_in(user) + visit project_issue_path(project, issue) + wait_for_requests + end + + before do + project.add_developer(user) + end + + context 'user has no private fork' do + before do + fork_project(project, user, repository: true) + visit_confidential_issue + end + + it 'shows that user has no fork available' do + click_button 'Create confidential merge request' + + page.within '.create-confidential-merge-request-dropdown-menu' do + expect(page).to have_content('No forks available to you') + end + end + end + + describe 'user has private fork' do + let(:forked_project) { fork_project(project, user, repository: true) } + + before do + forked_project.update(visibility: Gitlab::VisibilityLevel::PRIVATE) + visit_confidential_issue + end + + it 'create merge request in fork' do + click_button 'Create confidential merge request' + + page.within '.create-confidential-merge-request-dropdown-menu' do + expect(page).to have_button(forked_project.name_with_namespace) + click_button 'Create confidential merge request' + end + + expect(page).to have_content(forked_project.namespace.name) + end + end +end diff --git a/spec/frontend/confidential_merge_request/components/__snapshots__/project_form_group_spec.js.snap b/spec/frontend/confidential_merge_request/components/__snapshots__/project_form_group_spec.js.snap new file mode 100644 index 00000000000..a241c764df7 --- /dev/null +++ b/spec/frontend/confidential_merge_request/components/__snapshots__/project_form_group_spec.js.snap @@ -0,0 +1,101 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Confidential merge request project form group component renders empty state when response is empty 1`] = ` +
+ + +
+ + +

+ + No forks available to you. +
+ + + To protect this issues confidentiality, + + fork the project + + and set the forks visiblity to private. + + + + + Read more + + +