Disable loading vulnerability feedback when user is unauthorized

This change prevents the application from loading vulnerability feedback
on single Merge Request page when user is unauthorized to read that
information.

ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -340,6 +340,7 @@ export default {
         :container-scanning-help-path="mr.containerScanningHelp"
         :dependency-scanning-help-path="mr.dependencyScanningHelp"
         :secret-scanning-help-path="mr.secretScanningHelp"
+        :can-read-vulnerability-feedback="mr.canReadVulnerabilityFeedback"
         :vulnerability-feedback-path="mr.vulnerabilityFeedbackPath"
         :vulnerability-feedback-help-path="mr.vulnerabilityFeedbackHelpPath"
         :create-vulnerability-feedback-issue-path="mr.createVulnerabilityFeedbackIssuePath"

ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -16,6 +16,7 @@ export default class MergeRequestStore extends CEMergeRequestStore {
     this.secretScanningHelp = data.secret_scanning_help_path;
     this.dependencyScanningHelp = data.dependency_scanning_help_path;
     this.vulnerabilityFeedbackPath = data.vulnerability_feedback_path;
+    this.canReadVulnerabilityFeedback = data.can_read_vulnerability_feedback;
     this.vulnerabilityFeedbackHelpPath = data.vulnerability_feedback_help_path;
     this.approvalsHelpPath = data.approvals_help_path;
     this.codequalityHelpPath = data.codequality_help_path;

ee/app/assets/javascripts/vue_shared/security_reports/grouped_security_reports_app.vue

@@ -77,6 +77,11 @@ export default {
       required: false,
       default: '',
     },
+    canReadVulnerabilityFeedback: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
     vulnerabilityFeedbackPath: {
       type: String,
       required: false,
@@ -204,6 +209,7 @@ export default {
     this.setBaseBlobPath(this.baseBlobPath);
     this.setSourceBranch(this.sourceBranch);
 
+    this.setCanReadVulnerabilityFeedback(this.canReadVulnerabilityFeedback);
     this.setVulnerabilityFeedbackPath(this.vulnerabilityFeedbackPath);
     this.setVulnerabilityFeedbackHelpPath(this.vulnerabilityFeedbackHelpPath);
     this.setCreateVulnerabilityFeedbackIssuePath(this.createVulnerabilityFeedbackIssuePath);
@@ -253,6 +259,7 @@ export default {
       'setHeadBlobPath',
       'setBaseBlobPath',
       'setSourceBranch',
+      'setCanReadVulnerabilityFeedback',
       'setVulnerabilityFeedbackPath',
       'setVulnerabilityFeedbackHelpPath',
       'setCreateVulnerabilityFeedbackIssuePath',

ee/app/assets/javascripts/vue_shared/security_reports/store/actions.js

@@ -24,6 +24,9 @@ export const setBaseBlobPath = ({ commit }, blobPath) => commit(types.SET_BASE_B
 
 export const setSourceBranch = ({ commit }, branch) => commit(types.SET_SOURCE_BRANCH, branch);
 
+export const setCanReadVulnerabilityFeedback = ({ commit }, value) =>
+  commit(types.SET_CAN_READ_VULNERABILITY_FEEDBACK, value);
+
 export const setVulnerabilityFeedbackPath = ({ commit }, path) =>
   commit(types.SET_VULNERABILITY_FEEDBACK_PATH, path);
 
@@ -41,6 +44,19 @@ export const setCreateVulnerabilityFeedbackDismissalPath = ({ commit }, path) =>
 
 export const setPipelineId = ({ commit }, id) => commit(types.SET_PIPELINE_ID, id);
 
+export const fetchDiffData = (state, endpoint, category) => {
+  const requests = [pollUntilComplete(endpoint)];
+
+  if (state.canReadVulnerabilityFeedback) {
+    requests.push(axios.get(state.vulnerabilityFeedbackPath, { params: { category } }));
+  }
+
+  return Promise.all(requests).then(([diffResponse, enrichResponse]) => ({
+    diff: diffResponse.data,
+    enrichData: enrichResponse?.data ?? [],
+  }));
+};
+
 /**
  * CONTAINER SCANNING
  */
@@ -60,19 +76,9 @@ export const receiveContainerScanningDiffError = ({ commit }) =>
 export const fetchContainerScanningDiff = ({ state, dispatch }) => {
   dispatch('requestContainerScanningDiff');
 
-  return Promise.all([
-    pollUntilComplete(state.containerScanning.paths.diffEndpoint),
-    axios.get(state.vulnerabilityFeedbackPath, {
-      params: {
-        category: 'container_scanning',
-      },
-    }),
-  ])
-    .then(values => {
-      dispatch('receiveContainerScanningDiffSuccess', {
-        diff: values[0].data,
-        enrichData: values[1].data,
-      });
+  return fetchDiffData(state, state.containerScanning.paths.diffEndpoint, 'container_scanning')
+    .then(data => {
+      dispatch('receiveContainerScanningDiffSuccess', data);
     })
     .catch(() => {
       dispatch('receiveContainerScanningDiffError');
@@ -99,19 +105,9 @@ export const receiveDastDiffError = ({ commit }) => commit(types.RECEIVE_DAST_DI
 export const fetchDastDiff = ({ state, dispatch }) => {
   dispatch('requestDastDiff');
 
-  return Promise.all([
-    pollUntilComplete(state.dast.paths.diffEndpoint),
-    axios.get(state.vulnerabilityFeedbackPath, {
-      params: {
-        category: 'dast',
-      },
-    }),
-  ])
-    .then(values => {
-      dispatch('receiveDastDiffSuccess', {
-        diff: values[0].data,
-        enrichData: values[1].data,
-      });
+  return fetchDiffData(state, state.dast.paths.diffEndpoint, 'dast')
+    .then(data => {
+      dispatch('receiveDastDiffSuccess', data);
     })
     .catch(() => {
       dispatch('receiveDastDiffError');
@@ -137,19 +133,9 @@ export const receiveDependencyScanningDiffError = ({ commit }) =>
 export const fetchDependencyScanningDiff = ({ state, dispatch }) => {
   dispatch('requestDependencyScanningDiff');
 
-  return Promise.all([
-    pollUntilComplete(state.dependencyScanning.paths.diffEndpoint),
-    axios.get(state.vulnerabilityFeedbackPath, {
-      params: {
-        category: 'dependency_scanning',
-      },
-    }),
-  ])
-    .then(values => {
-      dispatch('receiveDependencyScanningDiffSuccess', {
-        diff: values[0].data,
-        enrichData: values[1].data,
-      });
+  return fetchDiffData(state, state.dependencyScanning.paths.diffEndpoint, 'dependency_scanning')
+    .then(data => {
+      dispatch('receiveDependencyScanningDiffSuccess', data);
     })
     .catch(() => {
       dispatch('receiveDependencyScanningDiffError');
@@ -177,19 +163,9 @@ export const receiveSecretScanningDiffError = ({ commit }) =>
 export const fetchSecretScanningDiff = ({ state, dispatch }) => {
   dispatch('requestSecretScanningDiff');
 
-  return Promise.all([
-    pollUntilComplete(state.secretScanning.paths.diffEndpoint),
-    axios.get(state.vulnerabilityFeedbackPath, {
-      params: {
-        category: 'secret_scanning',
-      },
-    }),
-  ])
-    .then(values => {
-      dispatch('receiveSecretScanningDiffSuccess', {
-        diff: values[0].data,
-        enrichData: values[1].data,
-      });
+  return fetchDiffData(state, state.secretScanning.paths.diffEndpoint, 'secret_scanning')
+    .then(data => {
+      dispatch('receiveSecretScanningDiffSuccess', data);
     })
     .catch(() => {
       dispatch('receiveSecretScanningDiffError');

ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/actions.js

-import axios from '~/lib/utils/axios_utils';
-import pollUntilComplete from '~/lib/utils/poll_until_complete';
 import * as types from './mutation_types';
+import { fetchDiffData } from '../../actions';
 
 export const setDiffEndpoint = ({ commit }, path) => commit(types.SET_DIFF_ENDPOINT, path);
 
@@ -18,19 +17,9 @@ export const receiveDiffError = ({ commit }, response) =>
 export const fetchDiff = ({ state, rootState, dispatch }) => {
   dispatch('requestDiff');
 
-  return Promise.all([
-    pollUntilComplete(state.paths.diffEndpoint),
-    axios.get(rootState.vulnerabilityFeedbackPath, {
-      params: {
-        category: 'sast',
-      },
-    }),
-  ])
-    .then(values => {
-      dispatch('receiveDiffSuccess', {
-        diff: values[0].data,
-        enrichData: values[1].data,
-      });
+  return fetchDiffData(rootState, state.paths.diffEndpoint, 'sast')
+    .then(data => {
+      dispatch('receiveDiffSuccess', data);
     })
     .catch(() => {
       dispatch('receiveDiffError');

ee/app/assets/javascripts/vue_shared/security_reports/store/mutation_types.js

 export const SET_HEAD_BLOB_PATH = 'SET_HEAD_BLOB_PATH';
 export const SET_BASE_BLOB_PATH = 'SET_BASE_BLOB_PATH';
 export const SET_SOURCE_BRANCH = 'SET_SOURCE_BRANCH';
+export const SET_CAN_READ_VULNERABILITY_FEEDBACK = 'SET_CAN_READ_VULNERABILITY_FEEDBACK';
 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 =

ee/app/assets/javascripts/vue_shared/security_reports/store/mutations.js

@@ -16,6 +16,10 @@ export default {
     state.sourceBranch = branch;
   },
 
+  [types.SET_CAN_READ_VULNERABILITY_FEEDBACK](state, value) {
+    state.canReadVulnerabilityFeedback = value;
+  },
+
   [types.SET_VULNERABILITY_FEEDBACK_PATH](state, path) {
     state.vulnerabilityFeedbackPath = path;
   },

ee/app/assets/javascripts/vue_shared/security_reports/store/state.js

@@ -5,6 +5,7 @@ export default () => ({
   },
 
   sourceBranch: null,
+  canReadVulnerabilityFeedback: false,
   vulnerabilityFeedbackPath: null,
   vulnerabilityFeedbackHelpPath: null,
   createVulnerabilityFeedbackIssuePath: null,

ee/app/serializers/ee/merge_request_widget_entity.rb

@@ -71,6 +71,10 @@ module EE
         merge_request.head_pipeline.id
       end
 
+      expose :can_read_vulnerability_feedback do |merge_request|
+        can?(current_user, :read_vulnerability_feedback, merge_request.project)
+      end
+
       expose :vulnerability_feedback_path do |merge_request|
         project_vulnerability_feedback_index_path(merge_request.project)
       end

ee/spec/fixtures/api/schemas/entities/merge_request_poll_widget.json

@@ -12,6 +12,7 @@
           }
         },
         "pipeline_id": { "type": "integer" },
+        "can_read_vulnerability_feedback": { "type": "boolean" },
         "vulnerability_feedback_path": { "type": "string" },
         "create_vulnerability_feedback_issue_path": { "type": "string" },
         "create_vulnerability_feedback_merge_request_path": { "type": "string" },

ee/spec/fixtures/api/schemas/entities/merge_request_widget.json

@@ -18,6 +18,7 @@
         "api_approve_path": { "type": ["string", "null"] },
         "api_unapprove_path": { "type": ["string", "null"] },
         "vulnerability_feedback_path": { "type": "string" },
+        "can_read_vulnerability_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"] }

ee/spec/frontend/vue_shared/security_reports/grouped_security_reports_app_spec.js

@@ -45,6 +45,7 @@ describe('Grouped security reports app', () => {
     dastHelpPath: 'path',
     dependencyScanningHelpPath: 'path',
     secretScanningHelpPath: 'path',
+    canReadVulnerabilityFeedbackPath: true,
     vulnerabilityFeedbackPath: 'vulnerability_feedback_path.json',
     vulnerabilityFeedbackHelpPath: 'path',
     pipelineId: 123,

ee/spec/frontend/vue_shared/security_reports/store/actions_spec.js

@@ -2,6 +2,7 @@ import MockAdapter from 'axios-mock-adapter';
 import {
   setHeadBlobPath,
   setBaseBlobPath,
+  setCanReadVulnerabilityFeedback,
   setVulnerabilityFeedbackPath,
   setVulnerabilityFeedbackHelpPath,
   setPipelineId,
@@ -148,6 +149,24 @@ describe('security reports actions', () => {
     });
   });
 
+  describe('setCanReadVulnerabilityFeedback', () => {
+    it('should commit set vulnerabulity feedback path', done => {
+      testAction(
+        setCanReadVulnerabilityFeedback,
+        true,
+        mockedState,
+        [
+          {
+            type: types.SET_CAN_READ_VULNERABILITY_FEEDBACK,
+            payload: true,
+          },
+        ],
+        [],
+        done,
+      );
+    });
+  });
+
   describe('setVulnerabilityFeedbackPath', () => {
     it('should commit set vulnerabulity feedback path', done => {
       testAction(
@@ -414,6 +433,7 @@ describe('security reports actions', () => {
     it('with error should dispatch `receiveDismissVulnerabilityError`', done => {
       mock.onPost('dismiss_vulnerability_path').reply(500, {});
       mockedState.vulnerabilityFeedbackPath = 'dismiss_vulnerability_path';
+      mockedState.canReadVulnerabilityFeedback = true;
 
       testAction(
         dismissVulnerability,
@@ -852,6 +872,7 @@ describe('security reports actions', () => {
     it('with error should dispatch `receiveCreateIssueError`', done => {
       mock.onPost('create_issue_path').reply(500, {});
       mockedState.vulnerabilityFeedbackPath = 'create_issue_path';
+      mockedState.canReadVulnerabilityFeedback = true;
 
       testAction(
         createNewIssue,
@@ -980,6 +1001,7 @@ describe('security reports actions', () => {
     it('with error should dispatch `receiveCreateMergeRequestError`', done => {
       mock.onPost('create_merge_request_path').reply(500, {});
       mockedState.vulnerabilityFeedbackPath = 'create_merge_request_path';
+      mockedState.canReadVulnerabilityFeedback = true;
 
       testAction(
         createMergeRequest,
@@ -1143,6 +1165,7 @@ describe('security reports actions', () => {
 
     beforeEach(() => {
       mockedState.vulnerabilityFeedbackPath = 'vulnerabilities_feedback';
+      mockedState.canReadVulnerabilityFeedback = true;
       mockedState.containerScanning.paths.diffEndpoint = endpoint;
     });
 
@@ -1179,6 +1202,35 @@ describe('security reports actions', () => {
       });
     });
 
+    describe('when diff endpoint responds successfully and fetching vulnerability feedback is not authorized', () => {
+      beforeEach(() => {
+        mockedState.canReadVulnerabilityFeedback = false;
+        mock.onGet(endpoint).reply(200, diff);
+      });
+
+      it('should dispatch `receiveContainerScanningDiffSuccess`', done => {
+        testAction(
+          fetchContainerScanningDiff,
+          null,
+          mockedState,
+          [],
+          [
+            {
+              type: 'requestContainerScanningDiff',
+            },
+            {
+              type: 'receiveContainerScanningDiffSuccess',
+              payload: {
+                diff,
+                enrichData: [],
+              },
+            },
+          ],
+          done,
+        );
+      });
+    });
+
     describe('when vulnerabilities path errors', () => {
       it('should dispatch `receiveContainerScanningError`', done => {
         mock.onGet(endpoint).reply(500);
@@ -1300,6 +1352,7 @@ describe('security reports actions', () => {
 
     beforeEach(() => {
       mockedState.vulnerabilityFeedbackPath = 'vulnerabilities_feedback';
+      mockedState.canReadVulnerabilityFeedback = true;
       mockedState.dependencyScanning.paths.diffEndpoint = 'dependency_scanning_diff.json';
     });
 
@@ -1336,6 +1389,35 @@ describe('security reports actions', () => {
       });
     });
 
+    describe('when diff endpoint responds successfully and fetching vulnerability feedback is not authorized', () => {
+      beforeEach(() => {
+        mockedState.canReadVulnerabilityFeedback = false;
+        mock.onGet('dependency_scanning_diff.json').reply(200, diff);
+      });
+
+      it('should dispatch `receiveDependencyScanningDiffSuccess`', done => {
+        testAction(
+          fetchDependencyScanningDiff,
+          null,
+          mockedState,
+          [],
+          [
+            {
+              type: 'requestDependencyScanningDiff',
+            },
+            {
+              type: 'receiveDependencyScanningDiffSuccess',
+              payload: {
+                diff,
+                enrichData: [],
+              },
+            },
+          ],
+          done,
+        );
+      });
+    });
+
     describe('when vulnerabilities path errors', () => {
       it('should dispatch `receiveDependencyScanningError`', done => {
         mock.onGet('dependency_scanning_diff.json').reply(500);
@@ -1457,6 +1539,7 @@ describe('security reports actions', () => {
 
     beforeEach(() => {
       mockedState.vulnerabilityFeedbackPath = 'vulnerabilities_feedback';
+      mockedState.canReadVulnerabilityFeedback = true;
       mockedState.dast.paths.diffEndpoint = 'dast_diff.json';
     });
 
@@ -1493,6 +1576,35 @@ describe('security reports actions', () => {
       });
     });
 
+    describe('when diff endpoint responds successfully and fetching vulnerability feedback is not authorized', () => {
+      beforeEach(() => {
+        mockedState.canReadVulnerabilityFeedback = false;
+        mock.onGet('dast_diff.json').reply(200, diff);
+      });
+
+      it('should dispatch `receiveDastDiffSuccess`', done => {
+        testAction(
+          fetchDastDiff,
+          null,
+          mockedState,
+          [],
+          [
+            {
+              type: 'requestDastDiff',
+            },
+            {
+              type: 'receiveDastDiffSuccess',
+              payload: {
+                diff,
+                enrichData: [],
+              },
+            },
+          ],
+          done,
+        );
+      });
+    });
+
     describe('when vulnerabilities path errors', () => {
       it('should dispatch `receiveDastError`', done => {
         mock.onGet('dast_diff.json').reply(500);
@@ -1615,6 +1727,7 @@ describe('security reports actions', () => {
 
     beforeEach(() => {
       mockedState.vulnerabilityFeedbackPath = 'vulnerabilities_feedback';
+      mockedState.canReadVulnerabilityFeedback = true;
       mockedState.secretScanning.paths.diffEndpoint = endpoint;
     });
 
@@ -1651,6 +1764,35 @@ describe('security reports actions', () => {
       });
     });
 
+    describe('when diff endpoint responds successfully and fetching vulnerability feedback is not authorized', () => {
+      beforeEach(() => {
+        mockedState.canReadVulnerabilityFeedback = false;
+        mock.onGet(endpoint).reply(200, diff);
+      });
+
+      it('should dispatch `secret_scanning`', done => {
+        testAction(
+          fetchSecretScanningDiff,
+          null,
+          mockedState,
+          [],
+          [
+            {
+              type: 'requestSecretScanningDiff',
+            },
+            {
+              type: 'receiveSecretScanningDiffSuccess',
+              payload: {
+                diff,
+                enrichData: [],
+              },
+            },
+          ],
+          done,
+        );
+      });
+    });
+
     describe('when vulnerabilities path errors', () => {
       it('should dispatch `receiveSecretScanningError`', done => {
         mock.onGet(endpoint).reply(500);

ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js

@@ -92,6 +92,7 @@ describe('sast report actions', () => {
     beforeEach(() => {
       mock = new MockAdapter(axios);
       state.paths.diffEndpoint = diffEndpoint;
+      rootState.canReadVulnerabilityFeedback = true;
     });
 
     afterEach(() => {
@@ -129,6 +130,35 @@ describe('sast report actions', () => {
       });
     });
 
+    describe('when diff endpoint responds successfully and fetching vulnerability feedback is not authorized', () => {
+      beforeEach(() => {
+        rootState.canReadVulnerabilityFeedback = false;
+        mock.onGet(diffEndpoint).replyOnce(200, reports.diff);
+      });
+
+      it('should dispatch the `receiveDiffSuccess` action with empty enrich data', done => {
+        const { diff } = reports;
+        const enrichData = [];
+        testAction(
+          actions.fetchDiff,
+          {},
+          { ...rootState, ...state },
+          [],
+          [
+            { type: 'requestDiff' },
+            {
+              type: 'receiveDiffSuccess',
+              payload: {
+                diff,
+                enrichData,
+              },
+            },
+          ],
+          done,
+        );
+      });
+    });
+
     describe('when the vulnerability feedback endpoint fails', () => {
       beforeEach(() => {
         mock

ee/spec/frontend/vue_shared/security_reports/store/mutations_spec.js

@@ -31,6 +31,14 @@ describe('security reports mutations', () => {
     });
   });
 
+  describe('SET_CAN_READ_VULNERABILITY_FEEDBACK', () => {
+    it('should set the vulnerabilities endpoint', () => {
+      mutations[types.SET_CAN_READ_VULNERABILITY_FEEDBACK](stateCopy, false);
+
+      expect(stateCopy.canReadVulnerabilityFeedback).toEqual(false);
+    });
+  });
+
   describe('SET_VULNERABILITY_FEEDBACK_PATH', () => {
     it('should set the vulnerabilities endpoint', () => {
       mutations[types.SET_VULNERABILITY_FEEDBACK_PATH](stateCopy, 'vulnerability_path');

ee/spec/serializers/merge_request_widget_entity_spec.rb

@@ -248,6 +248,32 @@ RSpec.describe MergeRequestWidgetEntity do
     expect(subject.as_json).to include(:create_vulnerability_feedback_dismissal_path)
   end
 
+  describe '#can_read_vulnerability_feedback' do
+    context 'when user has permissions to read vulnerability feedback' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'is set to true' do
+        expect(subject.as_json[:can_read_vulnerability_feedback]).to eq(true)
+      end
+    end
+
+    context 'when user has no permissions to read vulnerability feedback' do
+      before do
+        project.add_guest(user)
+      end
+
+      it 'is set to false' do
+        expect(subject.as_json[:can_read_vulnerability_feedback]).to eq(false)
+      end
+    end
+  end
+
+  it 'has can_read_vulnerability_feedback property' do
+    expect(subject.as_json).to include(:can_read_vulnerability_feedback)
+  end
+
   it 'has pipeline id' do
     allow(merge_request).to receive(:head_pipeline).and_return(pipeline)