Commit c3e9783d authored by Stan Hu's avatar Stan Hu

Merge branch '9412-split_vulnerability_feedback_permissions' into 'master'

Update vulnerability feedback permissions

Closes #9286 and #9412

See merge request gitlab-org/gitlab-ee!9700
parents 6af9b5eb 635d5edc
......@@ -68,9 +68,9 @@ Example response:
}
],
"project_fingerprint": "fa6f5b6c5d240b834ac5e901dc69f9484cef89ec",
"vulnerability_feedback_issue_path": "/tests/yarn-remediation-test/vulnerability_feedback",
"vulnerability_feedback_merge_request_path": "/tests/yarn-remediation-test/vulnerability_feedback",
"vulnerability_feedback_dismissal_path": "/tests/yarn-remediation-test/vulnerability_feedback",
"create_vulnerability_feedback_issue_path": "/tests/yarn-remediation-test/vulnerability_feedback",
"create_vulnerability_feedback_merge_request_path": "/tests/yarn-remediation-test/vulnerability_feedback",
"create_vulnerability_feedback_dismissal_path": "/tests/yarn-remediation-test/vulnerability_feedback",
"project": {
"id": 31,
"name": "yarn-remediation-test",
......
......@@ -2,7 +2,6 @@ import Vue from 'vue';
import Translate from '~/vue_shared/translate';
import SecurityReportApp from 'ee/vue_shared/security_reports/split_security_reports_app.vue';
import createStore from 'ee/vue_shared/security_reports/store';
import { parseBoolean } from '~/lib/utils/common_utils';
import { updateBadgeCount } from './utils';
Vue.use(Translate);
......@@ -22,12 +21,13 @@ export default () => {
dependencyScanningHelpPath,
vulnerabilityFeedbackPath,
vulnerabilityFeedbackHelpPath,
createVulnerabilityFeedbackIssuePath,
createVulnerabilityFeedbackMergeRequestPath,
createVulnerabilityFeedbackDismissalPath,
dastHeadPath,
sastContainerHeadPath,
dastHelpPath,
sastContainerHelpPath,
canCreateIssue,
canCreateFeedback,
} = datasetOptions;
const pipelineId = parseInt(datasetOptions.pipelineId, 10);
......@@ -52,13 +52,17 @@ export default () => {
dependencyScanningHelpPath,
vulnerabilityFeedbackPath,
vulnerabilityFeedbackHelpPath,
createVulnerabilityFeedbackIssuePath,
createVulnerabilityFeedbackMergeRequestPath,
createVulnerabilityFeedbackDismissalPath,
pipelineId,
dastHeadPath,
sastContainerHeadPath,
dastHelpPath,
sastContainerHelpPath,
canCreateFeedback: parseBoolean(canCreateFeedback),
canCreateIssue: parseBoolean(canCreateIssue),
canCreateIssue: Boolean(createVulnerabilityFeedbackIssuePath),
canCreateMergeRequest: Boolean(createVulnerabilityFeedbackMergeRequestPath),
canDismissVulnerability: Boolean(createVulnerabilityFeedbackDismissalPath),
},
on: {
updateBadgeCount: count => {
......
......@@ -18,8 +18,9 @@ document.addEventListener('DOMContentLoaded', () => {
refId,
refPath,
pipelineId,
canCreateFeedback,
canCreateIssue,
createVulnerabilityFeedbackIssuePath,
createVulnerabilityFeedbackMergeRequestPath,
createVulnerabilityFeedbackDismissalPath,
...rest
} = securityTab.dataset;
......@@ -39,8 +40,9 @@ document.addEventListener('DOMContentLoaded', () => {
props: {
pipelineId: parsedPipelineId,
hasPipelineData: parseBoolean(hasPipelineData),
canCreateIssue: parseBoolean(canCreateIssue),
canCreateFeedback: parseBoolean(canCreateFeedback),
canCreateIssue: Boolean(createVulnerabilityFeedbackIssuePath),
canCreateMergeRequest: Boolean(createVulnerabilityFeedbackMergeRequestPath),
canDismissVulnerability: Boolean(createVulnerabilityFeedbackDismissalPath),
triggeredBy: {
avatarPath: userAvatarPath,
name: userName,
......
<script>
import _ from 'underscore';
import { mapActions, mapState, mapGetters } from 'vuex';
import IssueModal from 'ee/vue_shared/security_reports/components/modal.vue';
import Filters from './filters.vue';
......@@ -50,13 +49,17 @@ export default {
...mapState('vulnerabilities', ['modal', 'pageInfo']),
...mapState('projects', ['projects']),
...mapGetters('filters', ['activeFilters']),
canCreateIssuePermission() {
canCreateIssue() {
const path = this.vulnerability.vulnerability_feedback_issue_path;
return _.isString(path) && !_.isEmpty(path);
return Boolean(path);
},
canCreateFeedbackPermission() {
const path = this.vulnerability.vulnerability_feedback_dismissal_path;
return _.isString(path) && !_.isEmpty(path);
canCreateMergeRequest() {
const path = this.vulnerability.create_vulnerability_feedback_merge_request_path;
return Boolean(path);
},
canDismissVulnerability() {
const path = this.vulnerability.create_vulnerability_feedback_dismissal_path;
return Boolean(path);
},
vulnerability() {
return this.modal.vulnerability;
......@@ -104,8 +107,9 @@ export default {
<issue-modal
:modal="modal"
:vulnerability-feedback-help-path="vulnerabilityFeedbackHelpPath"
:can-create-issue-permission="canCreateIssuePermission"
:can-create-feedback-permission="canCreateFeedbackPermission"
:can-create-issue="canCreateIssue"
:can-create-merge-request="canCreateMergeRequest"
:can-dismiss-vulnerability="canDismissVulnerability"
@createNewIssue="createIssue({ vulnerability })"
@dismissIssue="dismissVulnerability({ vulnerability })"
@revertDismissIssue="undoDismiss({ vulnerability })"
......
<script>
import _ from 'underscore';
import { mapActions } from 'vuex';
import { GlButton, GlSkeletonLoading } from '@gitlab/ui';
import SeverityBadge from 'ee/vue_shared/security_reports/components/severity_badge.vue';
......@@ -47,12 +46,12 @@ export default {
);
},
canDismissVulnerability() {
const path = this.vulnerability.vulnerability_feedback_dismissal_path;
return _.isString(path) && !_.isEmpty(path);
const path = this.vulnerability.create_vulnerability_feedback_dismissal_path;
return Boolean(path);
},
canCreateIssue() {
const path = this.vulnerability.vulnerability_feedback_issue_path;
return _.isString(path) && !_.isEmpty(path) && !this.hasIssue;
const path = this.vulnerability.create_vulnerability_feedback_issue_path;
return Boolean(path) && !this.hasIssue;
},
},
methods: {
......
......@@ -94,7 +94,7 @@ export const openModal = ({ commit }, payload = {}) => {
export const createIssue = ({ dispatch }, { vulnerability, flashError }) => {
dispatch('requestCreateIssue');
axios
.post(vulnerability.vulnerability_feedback_issue_path, {
.post(vulnerability.create_vulnerability_feedback_issue_path, {
vulnerability_feedback: {
feedback_type: 'issue',
category: vulnerability.report_type,
......@@ -137,7 +137,7 @@ export const dismissVulnerability = ({ dispatch }, { vulnerability, flashError }
dispatch('requestDismissVulnerability');
axios
.post(vulnerability.vulnerability_feedback_dismissal_path, {
.post(vulnerability.create_vulnerability_feedback_dismissal_path, {
vulnerability_feedback: {
feedback_type: 'dismissal',
category: vulnerability.report_type,
......@@ -177,14 +177,12 @@ export const receiveDismissVulnerabilityError = ({ commit }, { flashError }) =>
};
export const undoDismiss = ({ dispatch }, { vulnerability, flashError }) => {
const { vulnerability_feedback_dismissal_path, dismissal_feedback } = vulnerability;
// eslint-disable-next-line camelcase
const url = `${vulnerability_feedback_dismissal_path}/${dismissal_feedback.id}`;
const { destroy_vulnerability_feedback_dismissal_path } = vulnerability.dismissal_feedback;
dispatch('requestUndoDismiss');
axios
.delete(url)
.delete(destroy_vulnerability_feedback_dismissal_path)
.then(() => {
const { id } = vulnerability;
dispatch('receiveUndoDismissSuccess', { id });
......
......@@ -265,10 +265,16 @@ export default {
:dependency-scanning-help-path="mr.dependencyScanningHelp"
:vulnerability-feedback-path="mr.vulnerabilityFeedbackPath"
:vulnerability-feedback-help-path="mr.vulnerabilityFeedbackHelpPath"
:create-vulnerability-feedback-issue-path="mr.createVulnerabilityFeedbackIssuePath"
:create-vulnerability-feedback-merge-request-path="
mr.createVulnerabilityFeedbackMergeRequestPath
"
:create-vulnerability-feedback-dismissal-path="mr.createVulnerabilityFeedbackDismissalPath"
:pipeline-path="mr.pipeline.path"
:pipeline-id="mr.securityReportsPipelineId"
:can-create-issue="mr.canCreateIssue"
:can-create-feedback="mr.canCreateFeedback"
:can-create-merge-request="mr.canCreateMergeRequest"
:can-dismiss-vulnerability="mr.canDismissVulnerability"
/>
<mr-widget-licenses
v-if="shouldRenderLicenseReport"
......
......@@ -21,7 +21,14 @@ export default class MergeRequestStore extends CEMergeRequestStore {
this.vulnerabilityFeedbackHelpPath = data.vulnerability_feedback_help_path;
this.approvalsHelpPath = data.approvals_help_path;
this.securityReportsPipelineId = data.pipeline_id;
this.canCreateFeedback = data.can_create_feedback || false;
this.createVulnerabilityFeedbackIssuePath = data.create_vulnerability_feedback_issue_path;
this.createVulnerabilityFeedbackMergeRequestPath =
data.create_vulnerability_feedback_merge_request_path;
this.createVulnerabilityFeedbackDismissalPath =
data.create_vulnerability_feedback_dismissal_path;
this.canCreateIssue = Boolean(this.createVulnerabilityFeedbackIssuePath);
this.canCreateMergeRequest = Boolean(this.createVulnerabilityFeedbackMergeRequestPath);
this.canDismissVulnerability = Boolean(this.createVulnerabilityFeedbackDismissalPath);
this.initCodeclimate(data);
this.initPerformanceReport(data);
......
......@@ -115,11 +115,15 @@ export default {
required: false,
default: () => ({}),
},
canCreateFeedback: {
canCreateIssue: {
type: Boolean,
required: true,
},
canCreateIssue: {
canCreateMergeRequest: {
type: Boolean,
required: true,
},
canDismissVulnerability: {
type: Boolean,
required: true,
},
......@@ -173,8 +177,9 @@ export default {
:dependency-scanning-help-path="dependencyScanningHelpPath"
:vulnerability-feedback-path="vulnerabilityFeedbackPath"
:vulnerability-feedback-help-path="vulnerabilityFeedbackHelpPath"
:can-create-feedback="canCreateFeedback"
:can-create-issue="canCreateIssue"
:can-create-merge-request="canCreateMergeRequest"
:can-dismiss-vulnerability="canDismissVulnerability"
always-open
/>
</div>
......
......@@ -35,12 +35,17 @@ export default {
required: false,
default: '',
},
canCreateIssuePermission: {
canCreateIssue: {
type: Boolean,
required: false,
default: false,
},
canCreateFeedbackPermission: {
canCreateMergeRequest: {
type: Boolean,
required: false,
default: false,
},
canDismissVulnerability: {
type: Boolean,
required: false,
default: false,
......@@ -62,11 +67,11 @@ export default {
action: 'createMergeRequest',
};
if (!this.vulnerability.hasMergeRequest && this.remediation) {
if (!this.vulnerability.hasMergeRequest && this.canCreateMergeRequest && this.remediation) {
buttons.push(MRButton);
}
if (!this.vulnerability.hasIssue && this.canCreateIssuePermission) {
if (!this.vulnerability.hasIssue && this.canCreateIssue) {
buttons.push(issueButton);
}
......@@ -103,10 +108,7 @@ export default {
* The slot for the footer should be rendered if any of the conditions is true.
*/
shouldRenderFooterSection() {
return (
!this.modal.isResolved &&
(this.canCreateFeedbackPermission || this.canCreateIssuePermission)
);
return !this.modal.isResolved && (this.canCreateIssue || this.canDismissVulnerability);
},
vulnerability() {
return this.modal.vulnerability;
......@@ -194,7 +196,7 @@ export default {
</button>
<loading-button
v-if="canCreateFeedbackPermission"
v-if="canDismissVulnerability"
:loading="modal.isDismissingIssue"
:disabled="modal.isDismissingIssue"
:label="revertTitle"
......
......@@ -104,6 +104,21 @@ export default {
required: false,
default: '',
},
createVulnerabilityFeedbackIssuePath: {
type: String,
required: false,
default: '',
},
createVulnerabilityFeedbackMergeRequestPath: {
type: String,
required: false,
default: '',
},
createVulnerabilityFeedbackDismissalPath: {
type: String,
required: false,
default: '',
},
pipelineId: {
type: Number,
required: false,
......@@ -114,7 +129,11 @@ export default {
required: false,
default: undefined,
},
canCreateFeedback: {
canDismissVulnerability: {
type: Boolean,
required: true,
},
canCreateMergeRequest: {
type: Boolean,
required: true,
},
......@@ -159,6 +178,11 @@ export default {
this.setVulnerabilityFeedbackPath(this.vulnerabilityFeedbackPath);
this.setVulnerabilityFeedbackHelpPath(this.vulnerabilityFeedbackHelpPath);
this.setCreateVulnerabilityFeedbackIssuePath(this.createVulnerabilityFeedbackIssuePath);
this.setCreateVulnerabilityFeedbackMergeRequestPath(
this.createVulnerabilityFeedbackMergeRequestPath,
);
this.setCreateVulnerabilityFeedbackDismissalPath(this.createVulnerabilityFeedbackDismissalPath);
this.setPipelineId(this.pipelineId);
this.setCanCreateIssuePermission(this.canCreateIssue);
......@@ -220,6 +244,9 @@ export default {
'fetchDependencyScanningReports',
'setVulnerabilityFeedbackPath',
'setVulnerabilityFeedbackHelpPath',
'setCreateVulnerabilityFeedbackIssuePath',
'setCreateVulnerabilityFeedbackMergeRequestPath',
'setCreateVulnerabilityFeedbackDismissalPath',
'setPipelineId',
'setCanCreateIssuePermission',
'setCanCreateFeedbackPermission',
......@@ -323,11 +350,12 @@ export default {
<issue-modal
:modal="modal"
:vulnerability-feedback-help-path="vulnerabilityFeedbackHelpPath"
:can-create-issue-permission="canCreateIssuePermission"
:can-create-feedback-permission="canCreateFeedbackPermission"
:can-create-issue="canCreateIssue"
:can-create-merge-request="canCreateMergeRequest"
:can-dismiss-vulnerability="canDismissVulnerability"
@createNewIssue="createNewIssue"
@dismissIssue="dismissIssue"
@createMergeRequest="createMergeRequest"
@dismissIssue="dismissIssue"
@revertDismissIssue="revertDismissIssue"
/>
</div>
......
......@@ -81,16 +81,35 @@ export default {
required: false,
default: '',
},
createVulnerabilityFeedbackIssuePath: {
type: String,
required: false,
default: '',
},
createVulnerabilityFeedbackMergeRequestPath: {
type: String,
required: false,
default: '',
},
createVulnerabilityFeedbackDismissalPath: {
type: String,
required: false,
default: '',
},
pipelineId: {
type: Number,
required: false,
default: null,
},
canCreateFeedback: {
canCreateIssue: {
type: Boolean,
required: true,
},
canCreateMergeRequest: {
type: Boolean,
required: true,
},
canCreateIssue: {
canDismissVulnerability: {
type: Boolean,
required: true,
},
......@@ -149,6 +168,11 @@ export default {
this.setSourceBranch(this.sourceBranch);
this.setVulnerabilityFeedbackPath(this.vulnerabilityFeedbackPath);
this.setVulnerabilityFeedbackHelpPath(this.vulnerabilityFeedbackHelpPath);
this.setCreateVulnerabilityFeedbackIssuePath(this.createVulnerabilityFeedbackIssuePath);
this.setCreateVulnerabilityFeedbackMergeRequestPath(
this.createVulnerabilityFeedbackMergeRequestPath,
);
this.setCreateVulnerabilityFeedbackDismissalPath(this.createVulnerabilityFeedbackDismissalPath);
this.setPipelineId(this.pipelineId);
this.setCanCreateIssuePermission(this.canCreateIssue);
this.setCanCreateFeedbackPermission(this.canCreateFeedback);
......@@ -200,6 +224,9 @@ export default {
'fetchDastReports',
'setVulnerabilityFeedbackPath',
'setVulnerabilityFeedbackHelpPath',
'setCreateVulnerabilityFeedbackIssuePath',
'setCreateVulnerabilityFeedbackMergeRequestPath',
'setCreateVulnerabilityFeedbackDismissalPath',
'setPipelineId',
'setCanCreateIssuePermission',
'setCanCreateFeedbackPermission',
......@@ -287,8 +314,9 @@ export default {
<issue-modal
:modal="modal"
:vulnerability-feedback-help-path="vulnerabilityFeedbackHelpPath"
:can-create-issue-permission="canCreateIssuePermission"
:can-create-feedback-permission="canCreateFeedbackPermission"
:can-create-issue="canCreateIssue"
:can-create-merge-request="canCreateMergeRequest"
:can-dismiss-vulnerability="canDismissVulnerability"
@createNewIssue="createNewIssue"
@createMergeRequest="createMergeRequest"
@dismissIssue="dismissIssue"
......
......@@ -16,10 +16,20 @@ export const setVulnerabilityFeedbackPath = ({ commit }, path) =>
export const setVulnerabilityFeedbackHelpPath = ({ commit }, path) =>
commit(types.SET_VULNERABILITY_FEEDBACK_HELP_PATH, path);
export const setCreateVulnerabilityFeedbackIssuePath = ({ commit }, path) =>
commit(types.SET_CREATE_VULNERABILITY_FEEDBACK_ISSUE_PATH, path);
export const setCreateVulnerabilityFeedbackMergeRequestPath = ({ commit }, path) =>
commit(types.SET_CREATE_VULNERABILITY_FEEDBACK_MERGE_REQUEST_PATH, path);
export const setCreateVulnerabilityFeedbackDismissalPath = ({ commit }, path) =>
commit(types.SET_CREATE_VULNERABILITY_FEEDBACK_DISMISSAL_PATH, path);
export const setPipelineId = ({ commit }, id) => commit(types.SET_PIPELINE_ID, id);
export const setCanCreateIssuePermission = ({ commit }, permission) =>
commit(types.SET_CAN_CREATE_ISSUE_PERMISSION, permission);
export const setCanCreateFeedbackPermission = ({ commit }, permission) =>
commit(types.SET_CAN_CREATE_FEEDBACK_PERMISSION, permission);
......@@ -217,8 +227,8 @@ export const receiveDismissIssueError = ({ commit }, error) =>
export const dismissIssue = ({ state, dispatch }) => {
dispatch('requestDismissIssue');
return axios
.post(state.vulnerabilityFeedbackPath, {
axios
.post(state.createVulnerabilityFeedbackDismissalPath, {
vulnerability_feedback: {
feedback_type: 'dismissal',
category: state.modal.vulnerability.category,
......@@ -265,8 +275,10 @@ export const dismissIssue = ({ state, dispatch }) => {
export const revertDismissIssue = ({ state, dispatch }) => {
dispatch('requestDismissIssue');
return axios
.delete(`${state.vulnerabilityFeedbackPath}/${state.modal.vulnerability.dismissalFeedback.id}`)
axios
.delete(
state.modal.vulnerability.dismissalFeedback.destroy_vulnerability_feedback_dismissal_path,
)
.then(() => {
dispatch('receiveDismissIssue');
......@@ -310,8 +322,8 @@ export const receiveCreateIssueError = ({ commit }, error) =>
export const createNewIssue = ({ state, dispatch }) => {
dispatch('requestCreateIssue');
return axios
.post(state.vulnerabilityFeedbackPath, {
axios
.post(state.createVulnerabilityFeedbackIssuePath, {
vulnerability_feedback: {
feedback_type: 'issue',
category: state.modal.vulnerability.category,
......@@ -342,7 +354,7 @@ export const createMergeRequest = ({ state, dispatch }) => {
dispatch('requestCreateMergeRequest');
axios
.post(state.vulnerabilityFeedbackPath, {
.post(state.createVulnerabilityFeedbackMergeRequestPath, {
vulnerability_feedback: {
feedback_type: 'merge_request',
category,
......
......@@ -3,6 +3,12 @@ export const SET_BASE_BLOB_PATH = 'SET_BASE_BLOB_PATH';
export const SET_SOURCE_BRANCH = 'SET_SOURCE_BRANCH';
export const SET_VULNERABILITY_FEEDBACK_PATH = 'SET_VULNERABILITY_FEEDBACK_PATH';
export const SET_VULNERABILITY_FEEDBACK_HELP_PATH = 'SET_VULNERABILITY_FEEDBACK_HELP_PATH';
export const SET_CREATE_VULNERABILITY_FEEDBACK_ISSUE_PATH =
'SET_CREATE_VULNERABILITY_FEEDBACK_ISSUE_PATH';
export const SET_CREATE_VULNERABILITY_FEEDBACK_MERGE_REQUEST_PATH =
'SET_CREATE_VULNERABILITY_FEEDBACK_MERGE_REQUEST_PATH';
export const SET_CREATE_VULNERABILITY_FEEDBACK_DISMISSAL_PATH =
'SET_CREATE_VULNERABILITY_FEEDBACK_DISMISSAL_PATH';
export const SET_PIPELINE_ID = 'SET_PIPELINE_ID';
export const SET_CAN_CREATE_ISSUE_PERMISSION = 'SET_CAN_CREATE_ISSUE_PERMISSION';
export const SET_CAN_CREATE_FEEDBACK_PERMISSION = 'SET_CAN_CREATE_FEEDBACK_PERMISSION';
......
......@@ -32,6 +32,18 @@ export default {
state.vulnerabilityFeedbackHelpPath = path;
},
[types.SET_CREATE_VULNERABILITY_FEEDBACK_ISSUE_PATH](state, path) {
state.createVulnerabilityFeedbackIssuePath = path;
},
[types.SET_CREATE_VULNERABILITY_FEEDBACK_MERGE_REQUEST_PATH](state, path) {
state.createVulnerabilityFeedbackMergeRequestPath = path;
},
[types.SET_CREATE_VULNERABILITY_FEEDBACK_DISMISSAL_PATH](state, path) {
state.createVulnerabilityFeedbackDismissalPath = path;
},
[types.SET_PIPELINE_ID](state, id) {
state.pipelineId = id;
},
......
......@@ -9,6 +9,9 @@ export default () => ({
sourceBranch: null,
vulnerabilityFeedbackPath: null,
vulnerabilityFeedbackHelpPath: null,
createVulnerabilityFeedbackIssuePath: null,
createVulnerabilityFeedbackMergeRequestPath: null,
createVulnerabilityFeedbackDismissalPath: null,
pipelineId: null,
canCreateIssuePermission: false,
canCreateFeedbackPermission: false,
......
......@@ -3,7 +3,8 @@
class Projects::VulnerabilityFeedbackController < Projects::ApplicationController
before_action :vulnerability_feedback, only: [:destroy]
before_action :authorize_read_vulnerability_feedback!, only: [:index]
before_action :authorize_admin_vulnerability_feedback!, only: [:create, :destroy]
before_action :authorize_create_vulnerability_feedback!, only: [:create]
before_action :authorize_destroy_vulnerability_feedback!, only: [:destroy]
skip_before_action :authenticate_user!, only: [:index], raise: false
respond_to :json
......@@ -39,7 +40,7 @@ class Projects::VulnerabilityFeedbackController < Projects::ApplicationControlle
end
def destroy
service = VulnerabilityFeedbackModule::DestroyService.new(@vulnerability_feedback)
service = VulnerabilityFeedbackModule::DestroyService.new(project, current_user, vulnerability_feedback)
service.execute
head :no_content
......@@ -47,8 +48,17 @@ class Projects::VulnerabilityFeedbackController < Projects::ApplicationControlle
private
def authorize_admin_vulnerability_feedback!
render_403 unless can?(current_user, :admin_vulnerability_feedback, project)
def authorize_create_vulnerability_feedback!
type = vulnerability_feedback_params[:feedback_type]
return render_403 unless Vulnerabilities::Feedback.feedback_types.key?(type)
feedback = Vulnerabilities::Feedback.new(project: project, feedback_type: type)
return render_403 unless can?(current_user, :create_vulnerability_feedback, feedback)
end
def authorize_destroy_vulnerability_feedback!
render_403 unless can?(current_user, :destroy_vulnerability_feedback, vulnerability_feedback)
end
def serializer
......
......@@ -178,9 +178,7 @@ module EE
{
empty_state_illustration_path: image_path('illustrations/security-dashboard_empty.svg'),
security_dashboard_help_path: help_page_path("user/project/security_dashboard"),
has_pipeline_data: "false",
can_create_feedback: "false",
can_create_issue: "false"
has_pipeline_data: "false"
}
else
{
......@@ -206,12 +204,36 @@ module EE
pipeline_path: pipeline_url(pipeline),
pipeline_created: pipeline.created_at.to_s,
has_pipeline_data: "true",
can_create_feedback: can?(current_user, :admin_vulnerability_feedback, project).to_s,
can_create_issue: can?(current_user, :create_issue, project).to_s
create_vulnerability_feedback_issue_path: create_vulnerability_feedback_issue_path(project),
create_vulnerability_feedback_merge_request_path: create_vulnerability_feedback_merge_request_path(project),
create_vulnerability_feedback_dismissal_path: create_vulnerability_feedback_dismissal_path(project)
}
end
end
def can_create_feedback?(project, feedback_type)
feedback = Vulnerabilities::Feedback.new(project: project, feedback_type: feedback_type)
can?(current_user, :create_vulnerability_feedback, feedback)
end
def create_vulnerability_feedback_issue_path(project)
if can_create_feedback?(project, :issue)
project_vulnerability_feedback_index_path(project)
end
end
def create_vulnerability_feedback_merge_request_path(project)
if can_create_feedback?(project, :merge_request)
project_vulnerability_feedback_index_path(project)
end
end
def create_vulnerability_feedback_dismissal_path(project)
if can_create_feedback?(project, :dismissal)
project_vulnerability_feedback_index_path(project)
end
end
def settings_operations_available?
return true if super
......
......@@ -109,7 +109,8 @@ module EE
rule { can?(:developer_access) }.policy do
enable :admin_board
enable :admin_vulnerability_feedback
enable :create_vulnerability_feedback
enable :destroy_vulnerability_feedback
enable :create_package
enable :read_feature_flag
enable :create_feature_flag
......
# frozen_string_literal: true
module Vulnerabilities
class FeedbackPolicy < BasePolicy
delegate { @subject.project }
condition(:issue) { @subject.issue? }
condition(:merge_request) { @subject.merge_request? }
condition(:dismissal) { @subject.dismissal? }
rule { issue & ~can?(:create_issue) }.prevent :create_vulnerability_feedback
rule do
merge_request &
(~can?(:create_merge_request_in) | ~can?(:create_merge_request_from))
end.prevent :create_vulnerability_feedback
rule { ~dismissal }.prevent :destroy_vulnerability_feedback
end
end
......@@ -53,10 +53,37 @@ module EE
::ApproverGroup.filtered_approver_groups(merge_request.approver_groups, current_user)
end
def vulnerability_feedback_path
project_vulnerability_feedback_index_path(merge_request.project)
end
def create_vulnerability_feedback_issue_path
if expose_create_feedback_path?(:issue)
vulnerability_feedback_path
end
end
def create_vulnerability_feedback_merge_request_path
if expose_create_feedback_path?(:merge_request)
vulnerability_feedback_path
end
end
def create_vulnerability_feedback_dismissal_path
if expose_create_feedback_path?(:dismissal)
vulnerability_feedback_path
end
end
private
def expose_mr_approval_path?
approval_feature_available? && merge_request.iid
end
def expose_create_feedback_path?(feedback_type)
feedback = Vulnerabilities::Feedback.new(project: merge_request.project, feedback_type: feedback_type)
can?(current_user, :create_vulnerability_feedback, feedback)
end
end
end
......@@ -113,11 +113,19 @@ module EE
end
expose :vulnerability_feedback_path do |merge_request|
project_vulnerability_feedback_index_path(merge_request.project)
presenter(merge_request).vulnerability_feedback_path
end
expose :can_create_feedback do |merge_request|
can?(current_user, :admin_vulnerability_feedback, merge_request)
expose :create_vulnerability_feedback_issue_path do |merge_request|
presenter(merge_request).create_vulnerability_feedback_issue_path
end
expose :create_vulnerability_feedback_merge_request_path do |merge_request|
presenter(merge_request).create_vulnerability_feedback_merge_request_path
end
expose :create_vulnerability_feedback_dismissal_path do |merge_request|
presenter(merge_request).create_vulnerability_feedback_dismissal_path
end
expose :rebase_commit_sha
......
# frozen_string_literal: true
class Vulnerabilities::FeedbackEntity < Grape::Entity
include Gitlab::Routing
include GitlabRoutingHelper
include RequestAwareEntity
expose :id
expose :created_at
......@@ -40,10 +39,24 @@ class Vulnerabilities::FeedbackEntity < Grape::Entity
project_merge_request_path(feedback.project, feedback.merge_request)
end
expose :destroy_vulnerability_feedback_dismissal_path, if: ->(_, _) { can_destroy_feedback? }
expose :category
expose :feedback_type
expose :branch do |feedback|
feedback&.pipeline&.ref
end
expose :project_fingerprint
alias_method :feedback, :object
private
def destroy_vulnerability_feedback_dismissal_path
project_vulnerability_feedback_path(feedback.project, feedback)
end
def can_destroy_feedback?
can?(request.current_user, :destroy_vulnerability_feedback, feedback)
end
end
......@@ -7,9 +7,9 @@ class Vulnerabilities::OccurrenceEntity < Grape::Entity
expose :scanner, using: Vulnerabilities::ScannerEntity
expose :identifiers, using: Vulnerabilities::IdentifierEntity
expose :project_fingerprint
expose :vulnerability_feedback_path, as: :vulnerability_feedback_issue_path, if: ->(_, _) { can_admin_vulnerability_feedback? && can_create_issue? }
expose :vulnerability_feedback_path, as: :vulnerability_feedback_merge_request_path, if: ->(_, _) { can_admin_vulnerability_feedback? && can_create_merge_request? }
expose :vulnerability_feedback_path, as: :vulnerability_feedback_dismissal_path, if: ->(_, _) { can_admin_vulnerability_feedback? }
expose :vulnerability_feedback_path, as: :create_vulnerability_feedback_issue_path, if: ->(_, _) { can_create_feedback?(:issue) }
expose :vulnerability_feedback_path, as: :create_vulnerability_feedback_merge_request_path, if: ->(_, _) { can_create_feedback?(:merge_request) }
expose :vulnerability_feedback_path, as: :create_vulnerability_feedback_dismissal_path, if: ->(_, _) { can_create_feedback?(:dismissal) }
expose :project, using: ::ProjectEntity
expose :dismissal_feedback, using: Vulnerabilities::FeedbackEntity
expose :issue_feedback, using: Vulnerabilities::FeedbackEntity
......@@ -35,15 +35,8 @@ class Vulnerabilities::OccurrenceEntity < Grape::Entity
project_vulnerability_feedback_index_path(occurrence.project)
end
def can_admin_vulnerability_feedback?
can?(request.current_user, :admin_vulnerability_feedback, occurrence.project)
end
def can_create_issue?
can?(request.current_user, :create_issue, occurrence.project)
end
def can_create_merge_request?
can?(request.current_user, :create_merge_request_in, occurrence.project)
def can_create_feedback?(feedback_type)
feedback = Vulnerabilities::Feedback.new(project: occurrence.project, feedback_type: feedback_type)
can?(request.current_user, :create_vulnerability_feedback, feedback)
end
end
......@@ -5,6 +5,8 @@ module VulnerabilityFeedbackModule
def execute
vulnerability_feedback = @project.vulnerability_feedback.find_or_init_for(create_params)
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :create_vulnerability_feedback, vulnerability_feedback)
if vulnerability_feedback.issue? &&
!vulnerability_feedback.vulnerability_data.blank?
......
......@@ -2,12 +2,13 @@
module VulnerabilityFeedbackModule
class DestroyService < ::BaseService
def initialize(vulnerability_feedback)
@vulnerability_feedback = vulnerability_feedback
def initialize(project, user, vulnerability_feedback)
@project, @current_user, @vulnerability_feedback = project, user, vulnerability_feedback
end
def execute
# TODO: Add system note when destroying a dismissal feedback
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :destroy_vulnerability_feedback, @vulnerability_feedback)
@vulnerability_feedback.destroy
end
......
......@@ -23,8 +23,9 @@
dependency_scanning_help_path: help_page_path('user/project/merge_requests/dependency_scanning'),
dast_help_path: help_page_path('user/project/merge_requests/dast'),
sast_container_help_path: help_page_path('user/project/merge_requests/container_scanning'),
can_create_feedback: can?(current_user, :admin_vulnerability_feedback, project).to_s,
can_create_issue: show_new_issue_link?(project).to_s } }
create_vulnerability_feedback_issue_path: create_vulnerability_feedback_issue_path(project),
create_vulnerability_feedback_merge_request_path: create_vulnerability_feedback_merge_request_path(project),
create_vulnerability_feedback_dismissal_path: create_vulnerability_feedback_dismissal_path(project) } }
- if pipeline.expose_license_management_data?
#js-tab-licenses.tab-pane
......
---
title: Uses the more explicit vulnerability feedback endpoints on the front end
merge_request: 10461
author:
type: other
......@@ -126,26 +126,48 @@ describe Projects::VulnerabilityFeedbackController do
end
context 'with invalid params' do
it 'returns an unprocessable entity 422 response when feedbback_type is nil' do
it 'returns a forbidden 403 response when feedbback_type is nil' do
create_feedback user: user, project: project, params: create_params.except(:feedback_type)
expect(response).to have_gitlab_http_status(422)
expect(response).to have_gitlab_http_status(403)
end
it 'returns an unprocessable entity 422 response when feedbback_type is invalid' do
it 'returns a forbidden 403 response when feedbback_type is invalid' do
create_feedback user: user, project: project, params: create_params.merge(feedback_type: 'foo')
expect(response).to have_gitlab_http_status(422)
expect(response).to have_gitlab_http_status(403)
end
end
context 'with unauthorized user for feedback creation' do
it 'returns a forbidden 403 response' do
group.add_guest(guest)
context 'for issue feedback' do
it 'returns a forbidden 403 response' do
group.add_guest(guest)
create_feedback user: guest, project: project, params: create_params
create_feedback user: guest, project: project, params: create_params.merge(feedback_type: 'issue')
expect(response).to have_gitlab_http_status(403)
expect(response).to have_gitlab_http_status(403)
end
end
context 'for merge_request feedback' do
it 'returns a forbidden 403 response' do
group.add_guest(guest)
create_feedback user: guest, project: project, params: create_params.merge(feedback_type: 'merge_request')
expect(response).to have_gitlab_http_status(403)
end
end
context 'for dismissal feedback' do
it 'returns a forbidden 403 response' do
group.add_guest(guest)
create_feedback user: guest, project: project, params: create_params.merge(feedback_type: 'dismissal')
expect(response).to have_gitlab_http_status(403)
end
end
end
......@@ -214,6 +236,26 @@ describe Projects::VulnerabilityFeedbackController do
end
end
context 'for issue feedback' do
let!(:vuln_feedback) { create(:vulnerability_feedback, :issue, :sast, project: project, author: user, pipeline: pipeline) }
it 'returns a forbidden 403 response' do
destroy_feedback user: user, project: project, id: vuln_feedback.id
expect(response).to have_gitlab_http_status(403)
end
end
context 'for merge_request feedback' do
let!(:vuln_feedback) { create(:vulnerability_feedback, :merge_request, :sast, project: project, author: user, pipeline: pipeline) }
it 'returns a forbidden 403 response' do
destroy_feedback user: user, project: project, id: vuln_feedback.id
expect(response).to have_gitlab_http_status(403)
end
end
def destroy_feedback(user:, project:, id:)
sign_in(user)
......
......@@ -21,7 +21,9 @@
"base_path": { "type": "string" }
},
"vulnerability_feedback_path": { "type": "string" },
"can_create_feedback": { "type": "boolean" }
"create_vulnerability_feedback_issue_path": { "type": ["string", "null"] },
"create_vulnerability_feedback_merge_request_path": { "type": ["string", "null"] },
"create_vulnerability_feedback_dismissal_path": { "type": ["string", "null"] }
}
}
]
......
......@@ -12,9 +12,9 @@
"properties" : {
"name" : { "type": "string" },
"project_fingerprint": { "type": "string" },
"vulnerability_feedback_issue_path": { "type": "string" },
"vulnerability_feedback_merge_request_path": { "type": "string" },
"vulnerability_feedback_dismissal_path": { "type": "string" },
"create_vulnerability_feedback_issue_path": { "type": "string" },
"create_vulnerability_feedback_merge_request_path": { "type": "string" },
"create_vulnerability_feedback_dismissal_path": { "type": "string" },
"confidence" : {
"type": "string",
"enum": ["undefined", "ignore", "unknown", "experimental", "low", "medium", "high", "confirmed"]
......
......@@ -35,7 +35,8 @@
"enum": ["sast", "dependency_scanning", "container_scanning", "dast"]
},
"project_fingerprint": { "type": "string" },
"branch": { "type": "string" }
"branch": { "type": "string" },
"destroy_vulnerability_feedback_dismissal_path": { "type": "string" }
},
"additionalProperties": false
}
......@@ -16,7 +16,7 @@ describe('Security Reports modal', () => {
beforeEach(() => {
const props = {
modal: createState().modal,
canCreateFeedbackPermission: true,
canDismissVulnerability: true,
};
props.modal.vulnerability.isDismissed = true;
props.modal.vulnerability.dismissalFeedback = {
......@@ -50,7 +50,7 @@ describe('Security Reports modal', () => {
beforeEach(() => {
const props = {
modal: createState().modal,
canCreateFeedbackPermission: true,
canDismissVulnerability: true,
};
vm = mountComponent(Component, props);
});
......@@ -87,7 +87,7 @@ describe('Security Reports modal', () => {
beforeEach(() => {
const props = {
modal: createState().modal,
canCreateIssuePermission: true,
canCreateIssue: true,
};
vm = mountComponent(Component, props);
});
......
......@@ -385,7 +385,9 @@ describe('issue creation', () => {
describe('on success', () => {
beforeEach(() => {
mock.onPost(vulnerability.vulnerability_feedback_issue_path).replyOnce(200, { data });
mock
.onPost(vulnerability.create_vulnerability_feedback_issue_path)
.replyOnce(200, { data });
});
it('should dispatch the request and success actions', done => {
......@@ -408,7 +410,7 @@ describe('issue creation', () => {
describe('on error', () => {
beforeEach(() => {
mock.onPost(vulnerability.vulnerability_feedback_issue_path).replyOnce(404, {});
mock.onPost(vulnerability.create_vulnerability_feedback_issue_path).replyOnce(404, {});
});
it('should dispatch the request and error actions', done => {
......@@ -611,7 +613,9 @@ describe('vulnerability dismissal', () => {
describe('on success', () => {
beforeEach(() => {
mock.onPost(vulnerability.vulnerability_feedback_dismissal_path).replyOnce(200, data);
mock
.onPost(vulnerability.create_vulnerability_feedback_dismissal_path)
.replyOnce(200, data);
});
it('should dispatch the request and success actions', done => {
......@@ -634,7 +638,7 @@ describe('vulnerability dismissal', () => {
describe('on error', () => {
beforeEach(() => {
mock.onPost(vulnerability.vulnerability_feedback_dismissal_path).replyOnce(404, {});
mock.onPost(vulnerability.create_vulnerability_feedback_dismissal_path).replyOnce(404, {});
});
it('should dispatch the request and error actions', done => {
......@@ -710,9 +714,7 @@ describe('vulnerability dismissal', () => {
describe('revert vulnerability dismissal', () => {
describe('undoDismiss', () => {
const vulnerability = mockDataVulnerabilities[2];
const url = `${vulnerability.vulnerability_feedback_dismissal_path}/${
vulnerability.dismissal_feedback.id
}`;
const url = vulnerability.dismissal_feedback.destroy_vulnerability_feedback_dismissal_path;
let mock;
beforeEach(() => {
......
......@@ -32,8 +32,8 @@
},
"dismissal_feedback": null,
"issue_feedback": null,
"vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"description": "The cipher does not provide data integrity update 1",
"solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
"location": {
......@@ -95,8 +95,8 @@
},
"dismissal_feedback": null,
"issue_feedback": null,
"vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"description": "The cipher does not provide data integrity update 1",
"solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
"location": {
......@@ -165,11 +165,12 @@
"category": "sast",
"feedback_type": "dismissal",
"branch": "master",
"project_fingerprint": "4e5b6966dd100170b4b1ad599c7058cce91b57b4"
"project_fingerprint": "4e5b6966dd100170b4b1ad599c7058cce91b57b4",
"destroy_vulnerability_feedback_dismissal_path": "https://example.com/feedback_dismissal_path"
},
"issue_feedback": null,
"vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"description": "The cipher does not provide data integrity update 1",
"solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
"location": {
......@@ -242,8 +243,8 @@
"branch": "master",
"project_fingerprint": "4e5b6966dd100170b4b1ad599c7058cce91b57b4"
},
"vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"description": "The cipher does not provide data integrity update 1",
"solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
"location": {
......@@ -312,7 +313,8 @@
"category": "sast",
"feedback_type": "dismissal",
"branch": "master",
"project_fingerprint": "4e5b6966dd100170b4b1ad599c7058cce91b57b4"
"project_fingerprint": "4e5b6966dd100170b4b1ad599c7058cce91b57b4",
"destroy_vulnerability_feedback_dismissal_path": "https://example.com/feedback_dismissal_path"
},
"issue_feedback": {
"id": 2,
......@@ -338,8 +340,8 @@
"branch": "master",
"project_fingerprint": "4e5b6966dd100170b4b1ad599c7058cce91b57b4"
},
"vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"description": "The cipher does not provide data integrity update 1",
"solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
"location": {
......@@ -389,8 +391,8 @@
},
"dismissal_feedback": null,
"issue_feedback": null,
"vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
"create_vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
"description": "The cipher does not provide data integrity update 1",
"solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
"location": {
......
......@@ -60,8 +60,9 @@ describe('Card security reports app', () => {
dastHelpPath: 'path',
sastContainerHelpPath: 'path',
pipelineId: 123,
canCreateFeedback: true,
canCreateIssue: true,
canCreateMergeRequest: true,
canDismissVulnerability: true,
},
});
});
......
......@@ -60,7 +60,8 @@ describe('Grouped security reports app', () => {
vulnerabilityFeedbackHelpPath: 'path',
pipelineId: 123,
canCreateIssue: true,
canCreateFeedback: true,
canCreateMergeRequest: true,
canDismissVulnerability: true,
});
});
......@@ -116,7 +117,8 @@ describe('Grouped security reports app', () => {
vulnerabilityFeedbackHelpPath: 'path',
pipelineId: 123,
canCreateIssue: true,
canCreateFeedback: true,
canCreateMergeRequest: true,
canDismissVulnerability: true,
});
});
......@@ -170,7 +172,8 @@ describe('Grouped security reports app', () => {
vulnerabilityFeedbackHelpPath: 'path',
pipelineId: 123,
canCreateIssue: true,
canCreateFeedback: true,
canCreateMergeRequest: true,
canDismissVulnerability: true,
});
});
......@@ -242,8 +245,9 @@ describe('Grouped security reports app', () => {
beforeEach(() => {
vm = mountComponent(Component, {
headBlobPath: 'path',
canCreateFeedback: false,
canCreateIssue: false,
canCreateMergeRequest: false,
canDismissVulnerability: false,
pipelinePath,
});
});
......
......@@ -47,7 +47,8 @@ describe('Split security reports app', () => {
sastContainerHelpPath: 'path',
pipelineId: 123,
canCreateIssue: true,
canCreateFeedback: true,
canCreateMergeRequest: true,
canDismissVulnerability: true,
},
});
});
......@@ -90,7 +91,8 @@ describe('Split security reports app', () => {
sastContainerHelpPath: 'path',
pipelineId: 123,
canCreateIssue: true,
canCreateFeedback: true,
canCreateMergeRequest: true,
canDismissVulnerability: true,
},
});
});
......@@ -171,7 +173,8 @@ describe('Split security reports app', () => {
sastContainerHelpPath: 'path',
pipelineId: 123,
canCreateIssue: true,
canCreateFeedback: true,
canCreateMergeRequest: true,
canDismissVulnerability: true,
},
});
});
......
......@@ -1123,7 +1123,7 @@ describe('security reports actions', () => {
foo: 'bar',
};
mock.onPost('dismiss_issue_path').reply(200, dismissalFeedback);
mockedState.vulnerabilityFeedbackPath = 'dismiss_issue_path';
mockedState.createVulnerabilityFeedbackDismissalPath = 'dismiss_issue_path';
});
it('with success should dispatch `receiveDismissIssue`', done => {
......@@ -1263,7 +1263,7 @@ describe('security reports actions', () => {
it('with error should dispatch `receiveDismissIssueError`', done => {
mock.onPost('dismiss_issue_path').reply(500, {});
mockedState.vulnerabilityFeedbackPath = 'dismiss_issue_path';
mockedState.createVulnerabilityFeedbackDismissalPath = 'dismiss_issue_path';
testAction(
dismissIssue,
......@@ -1288,8 +1288,9 @@ describe('security reports actions', () => {
describe('with success', () => {
beforeEach(() => {
mock.onDelete('dismiss_issue_path/123').reply(200, {});
mockedState.modal.vulnerability.dismissalFeedback = { id: 123 };
mockedState.vulnerabilityFeedbackPath = 'dismiss_issue_path';
mockedState.modal.vulnerability.dismissalFeedback = {
destroy_vulnerability_feedback_dismissal_path: 'dismiss_issue_path/123',
};
});
it('should dispatch `receiveDismissIssue`', done => {
......@@ -1428,9 +1429,10 @@ describe('security reports actions', () => {
});
it('with error should dispatch `receiveDismissIssueError`', done => {
mockedState.modal.vulnerability.dismissalFeedback = {
destroy_vulnerability_feedback_dismissal_path: 'dismiss_issue_path',
};
mock.onDelete('dismiss_issue_path/123').reply(500, {});
mockedState.modal.vulnerability.dismissalFeedback = { id: 123 };
mockedState.vulnerabilityFeedbackPath = 'dismiss_issue_path';
testAction(
revertDismissIssue,
......@@ -1508,9 +1510,9 @@ describe('security reports actions', () => {
spyOnDependency(actions, 'visitUrl');
});
it('with success should dispatch `receiveDismissIssue`', done => {
it('with success should dispatch `receiveCreateIssue`', done => {
mock.onPost('create_issue_path').reply(200, { issue_path: 'new_issue' });
mockedState.vulnerabilityFeedbackPath = 'create_issue_path';
mockedState.createVulnerabilityFeedbackIssuePath = 'create_issue_path';
testAction(
createNewIssue,
......@@ -1613,8 +1615,8 @@ describe('security reports actions', () => {
it('with success should dispatch `receiveCreateMergeRequestSuccess`', done => {
const data = { merge_request_path: 'fakepath.html' };
mockedState.createVulnerabilityFeedbackMergeRequestPath = 'create_merge_request_path';
mock.onPost('create_merge_request_path').reply(200, data);
mockedState.vulnerabilityFeedbackPath = 'create_merge_request_path';
testAction(
createMergeRequest,
......
......@@ -259,6 +259,65 @@ describe ProjectPolicy do
end
end
describe 'vulnerability feedback permissions' do
subject { described_class.new(current_user, project) }
where(permission: %i[
create_vulnerability_feedback
destroy_vulnerability_feedback
])
with_them do
context 'with admin' do
let(:current_user) { admin }
it { is_expected.to be_allowed(permission) }
end
context 'with owner' do
let(:current_user) { owner }
it { is_expected.to be_allowed(permission) }
end
context 'with maintainer' do
let(:current_user) { maintainer }
it { is_expected.to be_allowed(permission) }
end
context 'with developer' do
let(:current_user) { developer }
it { is_expected.to be_allowed(permission) }
end
context 'with reporter' do
let(:current_user) { reporter }
it { is_expected.to be_disallowed(permission) }
end
context 'with guest' do
let(:current_user) { guest }
it { is_expected.to be_disallowed(permission) }
end
context 'with non member' do
let(:current_user) { create(:user) }
it { is_expected.to be_disallowed(permission) }
end
context 'with anonymous' do
let(:current_user) { nil }
it { is_expected.to be_disallowed(permission) }
end
end
end
describe 'read_project_security_dashboard' do
context 'with developer' do
let(:current_user) { developer }
......
# frozen_string_literal: true
require 'spec_helper'
describe Vulnerabilities::FeedbackPolicy do
include ExternalAuthorizationServiceHelpers
let(:current_user) { create(:user) }
let(:project) { create(:project, :public, namespace: current_user.namespace) }
subject { described_class.new(current_user, vulnerability_feedback) }
describe 'create_vulnerability_feedback' do
context 'when issue cannot be created' do
let(:vulnerability_feedback) { Vulnerabilities::Feedback.new(project: project, feedback_type: :issue) }
context 'when issues feature is disabled' do
before do
project.project_feature.update(issues_access_level: ProjectFeature::DISABLED)
end
it 'does not allow to create issue feedback' do
is_expected.to be_disallowed(:create_vulnerability_feedback)
end
end
context 'when user does not have permission to create issue in project' do
subject { described_class.new(nil, vulnerability_feedback) }
it 'does not allow to create issue feedback' do
is_expected.to be_disallowed(:create_issue)
is_expected.to be_disallowed(:create_vulnerability_feedback)
end
end
end
context 'when merge request cannot be created' do
let(:vulnerability_feedback) { Vulnerabilities::Feedback.new(project: project, feedback_type: :merge_request) }
context 'when merge request feature is disabled' do
before do
project.project_feature.update(merge_requests_access_level: ProjectFeature::DISABLED)
end
it 'does not allow to create merge request feedback' do
is_expected.to be_disallowed(:create_vulnerability_feedback)
end
end
context 'when user does not have permission to create merge_request in project' do
subject { described_class.new(nil, vulnerability_feedback) }
it 'does not allow to create merge request feedback' do
is_expected.to be_disallowed(:create_merge_request_in)
is_expected.to be_disallowed(:create_vulnerability_feedback)
end
end
context 'when user does not have permission to create merge_request from project' do
# guest can create merge request IN but not FROM
let(:guest) { create(:user) }
subject { described_class.new(guest, vulnerability_feedback) }
before do
project.add_guest(guest)
end
it 'does not allow to create merge request feedback' do
is_expected.to be_allowed(:create_merge_request_in)
is_expected.to be_disallowed(:create_merge_request_from)
is_expected.to be_disallowed(:create_vulnerability_feedback)
end
end
end
end
describe 'destroy_vulnerability_feedback' do
context 'when feedback type is issue' do
let(:vulnerability_feedback) { Vulnerabilities::Feedback.new(project: project, feedback_type: :issue) }
it 'does not allow to destroy issue feedback' do
is_expected.to be_disallowed(:destroy_vulnerability_feedback)
end
end
context 'when feedback type is merge_request' do
let(:vulnerability_feedback) { Vulnerabilities::Feedback.new(project: project, feedback_type: :merge_request) }
it 'does not allow to destroy merge request feedback' do
is_expected.to be_disallowed(:destroy_vulnerability_feedback)
end
end
context 'when feedback type is dismissal' do
let(:vulnerability_feedback) { Vulnerabilities::Feedback.new(project: project, feedback_type: :dismissal) }
it 'allows to destroy dismissal feedback' do
is_expected.to be_allowed(:destroy_vulnerability_feedback)
end
end
end
end
......@@ -192,4 +192,36 @@ describe MergeRequestPresenter do
is_expected.to match_array(approvers)
end
end
describe '#vulnerability_feedback_path' do
subject { described_class.new(merge_request, current_user: user).vulnerability_feedback_path }
it { is_expected.to eq("/#{merge_request.project.full_path}/vulnerability_feedback") }
end
describe 'create vulnerability feedback paths' do
where(:create_feedback_path) do
[
:create_vulnerability_feedback_issue_path,
:create_vulnerability_feedback_merge_request_path,
:create_vulnerability_feedback_dismissal_path
]
end
with_them do
subject { described_class.new(merge_request, current_user: user).public_send(create_feedback_path) }
it { is_expected.to eq("/#{merge_request.project.full_path}/vulnerability_feedback") }
context 'when not allowed to create vulnerability feedback' do
let(:unauthorized_user) { create(:user) }
subject { described_class.new(merge_request, current_user: unauthorized_user).public_send(create_feedback_path) }
it "does not contain #{params['create_feedback_path']}" do
expect(subject).to be_nil
end
end
end
end
end
......@@ -14,16 +14,8 @@ describe API::Vulnerabilities do
let(:ds_report) { pipeline.security_reports.reports["dependency_scanning"] }
let(:sast_report) { pipeline.security_reports.reports["sast"] }
before do
stub_licensed_features(security_dashboard: true, sast: true, dependency_scanning: true, container_scanning: true)
create(:ee_ci_job_artifact, :dependency_scanning, job: build_ds, project: project)
create(:ee_ci_job_artifact, :sast, job: build_sast, project: project)
create(
:vulnerability_feedback,
:dismissal,
:sast,
let(:dismissal) do
create(:vulnerability_feedback, :dismissal, :sast,
project: project,
pipeline: pipeline,
project_fingerprint: sast_report.occurrences.first.project_fingerprint,
......@@ -31,6 +23,14 @@ describe API::Vulnerabilities do
)
end
before do
stub_licensed_features(security_dashboard: true, sast: true, dependency_scanning: true, container_scanning: true)
create(:ee_ci_job_artifact, :dependency_scanning, job: build_ds, project: project)
create(:ee_ci_job_artifact, :sast, job: build_sast, project: project)
dismissal
end
describe "GET /projects/:id/vulnerabilities" do
context 'with an authorized user with proper permissions' do
before do
......@@ -89,7 +89,7 @@ describe API::Vulnerabilities do
expect(response.headers['X-Total']).to eq occurrence_count
expect(json_response.first['vulnerability_feedback_dismissal_path']).to be_present
expect(json_response.first.dig('dismissal_feedback', 'id')).to eq(dismissal.id)
end
it 'returns vulnerabilities with low severity' do
......
......@@ -181,8 +181,11 @@ describe MergeRequestWidgetEntity do
end
end
it 'has vulnerability feedbacks path' do
it 'has vulnerability feedback paths' do
expect(subject.as_json).to include(:vulnerability_feedback_path)
expect(subject.as_json).to include(:create_vulnerability_feedback_issue_path)
expect(subject.as_json).to include(:create_vulnerability_feedback_merge_request_path)
expect(subject.as_json).to include(:create_vulnerability_feedback_dismissal_path)
end
it 'has pipeline id' do
......
......@@ -3,27 +3,111 @@
require 'spec_helper'
describe Vulnerabilities::FeedbackEntity do
let(:feedback) { build(:vulnerability_feedback) }
set(:user) { create(:user) }
set(:project) { create(:project) }
let(:request) { double('request') }
let(:entity) { described_class.represent(feedback, request: request) }
let(:entity) { described_class.represent(feedback) }
subject { entity.as_json }
before do
allow(request).to receive(:current_user).and_return(user)
end
describe '#as_json' do
subject { entity.as_json }
let(:feedback) { build(:vulnerability_feedback, :issue, project: project) }
it { is_expected.to include(:created_at, :project_id, :author, :category, :feedback_type) }
context 'feedback type is issue' do
let(:feedback) { build(:vulnerability_feedback, :issue, project: project) }
it 'exposes issue information' do
is_expected.to include(:issue_iid)
is_expected.to include(:issue_url)
end
context 'when issue is not present' do
let(:feedback) { build(:vulnerability_feedback, feedback_type: :issue, project: project, issue: nil) }
it 'does not expose issue information' do
is_expected.not_to include(:issue_iid)
is_expected.not_to include(:issue_url)
end
end
context 'when allowed to destroy vulnerability feedback' do
before do
project.add_developer(user)
end
it 'does not contain destroy vulnerability feedback dismissal path' do
expect(subject).not_to include(:destroy_vulnerability_feedback_dismissal_path)
end
end
end
context 'feedback type is merge_request' do
let(:feedback) { build(:vulnerability_feedback, :merge_request, project: project) }
it 'exposes merge request information' do
is_expected.to include(:merge_request_iid)
is_expected.to include(:merge_request_path)
end
context 'when merge request is not present' do
let(:feedback) { build(:vulnerability_feedback, :merge_request, project: project, merge_request: nil) }
it 'does not expose merge request information' do
is_expected.not_to include(:merge_request_iid)
is_expected.not_to include(:merge_request_path)
end
end
context 'when allowed to destroy vulnerability feedback' do
before do
project.add_developer(user)
end
it 'does not contain destroy vulnerability feedback dismissal path' do
expect(subject).not_to include(:destroy_vulnerability_feedback_dismissal_path)
end
end
end
context 'feedback type is dismissal' do
let(:feedback) { create(:vulnerability_feedback, :dismissal, project: project) }
context 'when not allowed to destroy vulnerability feedback' do
before do
project.add_guest(user)
end
it 'does not contain destroy vulnerability feedback dismissal path' do
expect(subject).not_to include(:destroy_vulnerability_feedback_dismissal_path)
end
end
context 'when allowed to destroy vulnerability feedback' do
before do
project.add_developer(user)
end
it 'contains destroy vulnerability feedback dismissal path' do
expect(subject).to include(:destroy_vulnerability_feedback_dismissal_path)
end
end
end
end
context 'when comment is not present' do
subject { entity.as_json }
let(:feedback) { build(:vulnerability_feedback, :dismissal) }
it { is_expected.not_to include(:comment_details) }
end
context 'when comment is present' do
let(:feedback) { build(:vulnerability_feedback, :comment) }
let(:entity) { described_class.represent(feedback) }
subject { entity.as_json }
it 'exposes comment information' do
expect(subject).to include(:comment_details)
......@@ -35,9 +119,6 @@ describe Vulnerabilities::FeedbackEntity do
context 'when issue is present' do
let(:feedback) { build(:vulnerability_feedback, :issue ) }
let(:entity) { described_class.represent(feedback) }
subject { entity.as_json }
it 'exposes issue information' do
is_expected.to include(:issue_iid)
......@@ -47,9 +128,6 @@ describe Vulnerabilities::FeedbackEntity do
context 'when issue is not present' do
let(:feedback) { build(:vulnerability_feedback, feedback_type: 'issue', issue: nil) }
let(:entity) { described_class.represent(feedback) }
subject { entity.as_json }
it 'does not expose issue information' do
is_expected.not_to include(:issue_iid)
......@@ -59,9 +137,6 @@ describe Vulnerabilities::FeedbackEntity do
context 'when merge request is present' do
let(:feedback) { build(:vulnerability_feedback, :merge_request ) }
let(:entity) { described_class.represent(feedback) }
subject { entity.as_json }
it 'exposes merge request information' do
is_expected.to include(:merge_request_iid)
......@@ -71,9 +146,6 @@ describe Vulnerabilities::FeedbackEntity do
context 'when merge request is not present' do
let(:feedback) { build(:vulnerability_feedback, feedback_type: 'merge_request', merge_request: nil) }
let(:entity) { described_class.represent(feedback) }
subject { entity.as_json }
it 'does not expose merge request information' do
is_expected.not_to include(:merge_request_iid)
......
......@@ -62,9 +62,9 @@ describe Vulnerabilities::OccurrenceEntity do
end
it 'does not contain vulnerability feedback paths' do
expect(subject).not_to include(:vulnerability_feedback_issue_path)
expect(subject).not_to include(:vulnerability_feedback_merge_request_path)
expect(subject).not_to include(:vulnerability_feedback_dismissal_path)
expect(subject).not_to include(:create_vulnerability_feedback_issue_path)
expect(subject).not_to include(:create_vulnerability_feedback_merge_request_path)
expect(subject).not_to include(:create_vulnerability_feedback_dismissal_path)
end
end
......@@ -74,30 +74,30 @@ describe Vulnerabilities::OccurrenceEntity do
end
it 'contains vulnerability feedback dismissal path' do
expect(subject).to include(:vulnerability_feedback_dismissal_path)
expect(subject).to include(:create_vulnerability_feedback_dismissal_path)
end
it 'contains vulnerability feedback issue path' do
expect(subject).to include(:vulnerability_feedback_issue_path)
expect(subject).to include(:create_vulnerability_feedback_issue_path)
end
it 'contains vulnerability feedback merge_request path' do
expect(subject).to include(:vulnerability_feedback_merge_request_path)
expect(subject).to include(:create_vulnerability_feedback_merge_request_path)
end
context 'when disallowed to create issue' do
let(:project) { create(:project, issues_access_level: ProjectFeature::DISABLED) }
it 'does not contain vulnerability feedback issue path' do
expect(subject).not_to include(:vulnerability_feedback_issue_path)
expect(subject).not_to include(:create_vulnerability_feedback_issue_path)
end
it 'contains vulnerability feedback dismissal path' do
expect(subject).to include(:vulnerability_feedback_dismissal_path)
expect(subject).to include(:create_vulnerability_feedback_dismissal_path)
end
it 'contains vulnerability feedback merge_request path' do
expect(subject).to include(:vulnerability_feedback_merge_request_path)
expect(subject).to include(:create_vulnerability_feedback_merge_request_path)
end
end
......@@ -105,15 +105,15 @@ describe Vulnerabilities::OccurrenceEntity do
let(:project) { create(:project, merge_requests_access_level: ProjectFeature::DISABLED) }
it 'does not contain vulnerability feedback merge_request path' do
expect(subject).not_to include(:vulnerability_feedback_merge_request_path)
expect(subject).not_to include(:create_vulnerability_feedback_merge_request_path)
end
it 'contains vulnerability feedback issue path' do
expect(subject).to include(:vulnerability_feedback_issue_path)
expect(subject).to include(:create_vulnerability_feedback_issue_path)
end
it 'contains vulnerability feedback dismissal path' do
expect(subject).to include(:vulnerability_feedback_dismissal_path)
expect(subject).to include(:create_vulnerability_feedback_dismissal_path)
end
end
end
......
......@@ -28,6 +28,15 @@ describe VulnerabilityFeedbackModule::CreateService, '#execute' do
}
end
context 'when user is not authorized' do
let(:unauthorized_user) { create(:user) }
it 'raise error if permission is denied' do
expect { described_class.new(project, unauthorized_user, feedback_params).execute }
.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
context 'when feedback_type is dismissal' do
let(:result) { described_class.new(project, user, feedback_params).execute }
......
# frozen_string_literal: true
require 'spec_helper'
describe VulnerabilityFeedbackModule::DestroyService, '#execute' do
let(:group) { create(:group) }
let(:project) { create(:project, :public, :repository, namespace: group) }
let(:user) { create(:user) }
let(:vulnerability_feedback) { create(:vulnerability_feedback, feedback_type, project: project)}
before do
group.add_developer(user)
end
subject { described_class.new(project, user, vulnerability_feedback).execute }
context 'when feedback_type is dismissal' do
let(:feedback_type) { :dismissal }
it 'destroys the feedback' do
subject
expect { vulnerability_feedback.reload }.to raise_error ActiveRecord::RecordNotFound
end
context 'when user is not authorized' do
let(:unauthorized_user) { create(:user) }
it 'raise error if permission is denied' do
expect { described_class.new(project, unauthorized_user, vulnerability_feedback).execute }
.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
end
context 'when feedback_type is issue' do
let(:feedback_type) { :issue }
it 'raise error as this type of feedback can not be destroyed' do
expect { described_class.new(project, user, vulnerability_feedback).execute }
.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
context 'when feedback_type is merge_request' do
let(:feedback_type) { :merge_request }
it 'raise error as this type of feedback can not be destroyed' do
expect { described_class.new(project, user, vulnerability_feedback).execute }
.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
end
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