Commit 8bd32ec0 authored by Shinya Maeda's avatar Shinya Maeda

Show Security Warning for fork projects

This commit shows security warning for fork projects
parent fdaeabef
<script>
import { GlDeprecatedButton, GlLoadingIcon } from '@gitlab/ui';
import { GlDeprecatedButton, GlLoadingIcon, GlModal, GlModalDirective } from '@gitlab/ui';
import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
import PipelinesService from '~/pipelines/services/pipelines_service';
import PipelineStore from '~/pipelines/stores/pipelines_store';
......@@ -14,6 +14,10 @@ export default {
TablePagination,
GlDeprecatedButton,
GlLoadingIcon,
GlModal,
},
directives: {
GlModalDirective,
},
mixins: [pipelinesMixin, CIPaginationMixin],
props: {
......@@ -38,11 +42,21 @@ export default {
required: false,
default: 'child',
},
canRunPipeline: {
canCreatePipelineInTargetProject: {
type: Boolean,
required: false,
default: false,
},
sourceProjectFullPath: {
type: String,
required: false,
default: '',
},
targetProjectFullPath: {
type: String,
required: false,
default: '',
},
projectId: {
type: String,
required: false,
......@@ -63,6 +77,7 @@ export default {
state: store.state,
page: getParameterByName('page') || '1',
requestData: {},
modalId: 'create-pipeline-for-fork-merge-request-modal',
};
},
......@@ -75,13 +90,28 @@ export default {
},
/**
* The Run Pipeline button can only be rendered when:
* - In MR view - we use `canRunPipeline` for that purpose
* - In MR view - we use `canCreatePipelineInTargetProject` for that purpose
* - If the latest pipeline has the `detached_merge_request_pipeline` flag
*
* @returns {Boolean}
*/
canRenderPipelineButton() {
return this.canRunPipeline && this.latestPipelineDetachedFlag;
return this.latestPipelineDetachedFlag;
},
isForkMergeRequest() {
return this.sourceProjectFullPath !== this.targetProjectFullPath;
},
isLatestPipelineCreatedInTargetProject() {
const latest = this.state.pipelines[0];
return latest && latest.project.full_path === `/${this.targetProjectFullPath}`;
},
shouldShowSecurityWarning() {
return (
this.canCreatePipelineInTargetProject &&
this.isForkMergeRequest &&
!this.isLatestPipelineCreatedInTargetProject
);
},
/**
* Checks if either `detached_merge_request_pipeline` or
......@@ -172,7 +202,7 @@ export default {
<div v-else-if="shouldRenderTable" class="table-holder">
<div v-if="canRenderPipelineButton" class="nav justify-content-end">
<gl-deprecated-button
v-if="canRenderPipelineButton"
v-if="!shouldShowSecurityWarning"
variant="success"
class="js-run-mr-pipeline prepend-top-10 btn-wide-on-xs"
:disabled="state.isRunningMergeRequestPipeline"
......@@ -181,6 +211,48 @@ export default {
<gl-loading-icon v-if="state.isRunningMergeRequestPipeline" inline />
{{ s__('Pipelines|Run Pipeline') }}
</gl-deprecated-button>
<gl-deprecated-button
v-if="shouldShowSecurityWarning"
v-gl-modal-directive="modalId"
variant="success"
class="js-run-mr-pipeline prepend-top-10 btn-wide-on-xs"
:disabled="state.isRunningMergeRequestPipeline"
>
<gl-loading-icon v-if="state.isRunningMergeRequestPipeline" inline />
{{ s__('Pipelines|Run Pipeline') }}
</gl-deprecated-button>
<gl-modal
:id="$options.id"
:modal-id="modalId"
:title="s__('Pipelines|Are you sure you want to run this pipeline?')"
:ok-title="s__('Pipelines|Run Pipeline')"
ok-variant="danger"
@ok="onClickRunPipeline"
>
<p>
{{
s__(
'Pipelines|This pipeline will run code originating from a forked project merge request. This means that the code can potentially have security considerations like exposing CI variables.',
)
}}
</p>
<p>
{{
s__(
"Pipelines|It is recommended the code is reviewed thoroughly before running this pipeline with the parent project's CI resource.",
)
}}
</p>
<p>
{{
s__(
'Pipelines|If you are unsure, please ask a project maintainer to review it for you.',
)
}}
</p>
</gl-modal>
</div>
<pipelines-table-component
......
......@@ -358,7 +358,11 @@ export default class MergeRequestTabs {
emptyStateSvgPath: pipelineTableViewEl.dataset.emptyStateSvgPath,
errorStateSvgPath: pipelineTableViewEl.dataset.errorStateSvgPath,
autoDevopsHelpPath: pipelineTableViewEl.dataset.helpAutoDevopsPath,
canRunPipeline: true,
canCreatePipelineInTargetProject: mrWidgetData
? mrWidgetData.can_create_pipeline_in_target_project
: null,
sourceProjectFullPath: mrWidgetData ? mrWidgetData.source_project_full_path : null,
targetProjectFullPath: mrWidgetData ? mrWidgetData.target_project_full_path : null,
projectId: pipelineTableViewEl.dataset.projectId,
mergeRequestId: mrWidgetData ? mrWidgetData.iid : null,
},
......
......@@ -14,6 +14,10 @@ class MergeRequestWidgetEntity < Grape::Entity
merge_request.project&.full_path
end
expose :can_create_pipeline_in_target_project do |merge_request|
can?(current_user, :create_pipeline, merge_request.target_project)
end
expose :email_patches_path do |merge_request|
project_merge_request_path(merge_request.project, merge_request, format: :patch)
end
......
......@@ -389,7 +389,7 @@ export default {
>
{{
s__(
'mrWidget|Fork merge requests do not create merge request pipelines which validate a post merge result',
'mrWidget|Fork project merge requests do not create merge request pipelines that validate a post merge result unless invoked by a project member.',
)
}}
</mr-widget-alert-message>
......
......@@ -70,13 +70,14 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
let(:traits) { [:with_detached_merge_request_pipeline] }
let(:options) { {} }
it 'shows a warning that fork project cannot create merge request pipelines', :sidekiq_might_not_need_inline do
it 'shows a warning that fork project merge request does not create merge request pipelines by default', :sidekiq_might_not_need_inline do
visit project_merge_request_path(project, merge_request)
within('.warning_message') do
expect(page)
.to have_content('Fork merge requests do not create merge request' \
' pipelines which validate a post merge result')
.to have_content('Fork project merge requests do not create merge' \
' request pipelines that validate a post merge result' \
' unless invoked by a project member.')
end
end
end
......
......@@ -17158,6 +17158,9 @@ msgstr ""
msgid "Pipelines|API"
msgstr ""
msgid "Pipelines|Are you sure you want to run this pipeline?"
msgstr ""
msgid "Pipelines|Build with confidence"
msgstr ""
......@@ -17182,6 +17185,12 @@ msgstr ""
msgid "Pipelines|Group %{namespace_name} has exceeded its pipeline minutes quota. Unless you buy additional pipeline minutes, no new jobs or pipelines in its projects will run."
msgstr ""
msgid "Pipelines|If you are unsure, please ask a project maintainer to review it for you."
msgstr ""
msgid "Pipelines|It is recommended the code is reviewed thoroughly before running this pipeline with the parent project's CI resource."
msgstr ""
msgid "Pipelines|Loading Pipelines"
msgstr ""
......@@ -17206,6 +17215,9 @@ msgstr ""
msgid "Pipelines|This is a child pipeline within the parent pipeline"
msgstr ""
msgid "Pipelines|This pipeline will run code originating from a forked project merge request. This means that the code can potentially have security considerations like exposing CI variables."
msgstr ""
msgid "Pipelines|This project is not currently set up to run pipelines."
msgstr ""
......@@ -28472,6 +28484,9 @@ msgstr ""
msgid "mrWidget|Fork merge requests do not create merge request pipelines which validate a post merge result"
msgstr ""
msgid "mrWidget|Fork project merge requests do not create merge request pipelines that validate a post merge result unless invoked by a project member."
msgstr ""
msgid "mrWidget|If the %{branch} branch exists in your local repository, you can merge this merge request manually using the"
msgstr ""
......
......@@ -123,14 +123,24 @@ RSpec.describe 'Merge request > User sees pipelines', :js do
context 'when actor is a developer in parent project' do
let(:actor) { developer_in_parent }
it 'creates a pipeline in the parent project' do
it 'creates a pipeline in the parent project when user proceeds with the warning' do
visit project_merge_request_path(parent_project, merge_request)
create_merge_request_pipeline
act_on_security_warning(action: 'Run Pipeline')
check_pipeline(expected_project: parent_project)
check_head_pipeline(expected_project: parent_project)
end
it 'does not create a pipeline in the parent project when user cancels the action' do
visit project_merge_request_path(parent_project, merge_request)
create_merge_request_pipeline
act_on_security_warning(action: 'Cancel')
check_no_pipelines
end
end
context 'when actor is a developer in fork project' do
......@@ -187,6 +197,19 @@ RSpec.describe 'Merge request > User sees pipelines', :js do
expect(page.find('.pipeline-id')[:href]).to include(expected_project.full_path)
end
end
def act_on_security_warning(action:)
page.within('#create-pipeline-for-fork-merge-request-modal') do
expect(page).to have_content('Are you sure you want to run this pipeline?')
click_button(action)
end
end
def check_no_pipelines
page.within('.ci-table') do
expect(page).to have_selector('.commit', count: 1)
end
end
end
describe 'race condition' do
......
......@@ -121,14 +121,14 @@ describe('Pipelines table in Commits and Merge requests', () => {
pipelineCopy = { ...pipeline };
});
describe('when latest pipeline has detached flag and canRunPipeline is true', () => {
describe('when latest pipeline has detached flag', () => {
it('renders the run pipeline button', done => {
pipelineCopy.flags.detached_merge_request_pipeline = true;
pipelineCopy.flags.merge_request_pipeline = true;
mock.onGet('endpoint.json').reply(200, [pipelineCopy]);
vm = mountComponent(PipelinesTable, { ...props, canRunPipeline: true });
vm = mountComponent(PipelinesTable, { ...props });
setImmediate(() => {
expect(vm.$el.querySelector('.js-run-mr-pipeline')).not.toBeNull();
......@@ -137,30 +137,14 @@ describe('Pipelines table in Commits and Merge requests', () => {
});
});
describe('when latest pipeline has detached flag and canRunPipeline is false', () => {
it('does not render the run pipeline button', done => {
pipelineCopy.flags.detached_merge_request_pipeline = true;
pipelineCopy.flags.merge_request_pipeline = true;
mock.onGet('endpoint.json').reply(200, [pipelineCopy]);
vm = mountComponent(PipelinesTable, { ...props, canRunPipeline: false });
setImmediate(() => {
expect(vm.$el.querySelector('.js-run-mr-pipeline')).toBeNull();
done();
});
});
});
describe('when latest pipeline does not have detached flag and canRunPipeline is true', () => {
describe('when latest pipeline does not have detached flag', () => {
it('does not render the run pipeline button', done => {
pipelineCopy.flags.detached_merge_request_pipeline = false;
pipelineCopy.flags.merge_request_pipeline = false;
mock.onGet('endpoint.json').reply(200, [pipelineCopy]);
vm = mountComponent(PipelinesTable, { ...props, canRunPipeline: true });
vm = mountComponent(PipelinesTable, { ...props });
setImmediate(() => {
expect(vm.$el.querySelector('.js-run-mr-pipeline')).toBeNull();
......@@ -176,7 +160,7 @@ describe('Pipelines table in Commits and Merge requests', () => {
mock.onGet('endpoint.json').reply(200, [pipelineCopy]);
vm = mountComponent(PipelinesTable, { ...props, canRunPipeline: false });
vm = mountComponent(PipelinesTable, { ...props });
setImmediate(() => {
expect(vm.$el.querySelector('.js-run-mr-pipeline')).toBeNull();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment