diff --git a/ee/app/assets/javascripts/vulnerabilities/components/header.vue b/ee/app/assets/javascripts/vulnerabilities/components/header.vue index dbf9f8977b950984a50938f3b9c99347767d5696..36004eca5f2b4b36c0f0d766bfe3d3ca23a2f5ba 100644 --- a/ee/app/assets/javascripts/vulnerabilities/components/header.vue +++ b/ee/app/assets/javascripts/vulnerabilities/components/header.vue @@ -2,6 +2,7 @@ import { GlLoadingIcon, GlTooltipDirective as GlTooltip } from '@gitlab/ui'; import { v4 as uuidv4 } from 'uuid'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import glAbilitiesMixin from '~/vue_shared/mixins/gl_abilities_mixin'; import vulnerabilityStateMutations from 'ee/security_dashboard/graphql/mutate_vulnerability_state'; import StatusBadge from 'ee/vue_shared/security_reports/components/status_badge.vue'; import { createAlert } from '~/alert'; @@ -13,7 +14,6 @@ import download from '~/lib/utils/downloader'; import { visitUrl } from '~/lib/utils/url_utility'; import UsersCache from '~/lib/utils/users_cache'; import { __, s__ } from '~/locale'; -import { REPORT_TYPE_SAST } from '~/vue_shared/security_reports/constants'; import { helpCenterState } from '~/super_sidebar/constants'; import chatMutation from 'ee/ai/graphql/chat.mutation.graphql'; import aiResponseSubscription from 'ee/graphql_shared/subscriptions/ai_completion_response.subscription.graphql'; @@ -70,7 +70,7 @@ export default { directives: { GlTooltip, }, - mixins: [glFeatureFlagsMixin()], + mixins: [glFeatureFlagsMixin(), glAbilitiesMixin()], props: { vulnerability: { type: Object, @@ -102,13 +102,15 @@ export default { buttons.push(DOWNLOAD_PATCH_ACTION); } - if (this.vulnerability.reportType === REPORT_TYPE_SAST) { + if (this.glAbilities.resolveVulnerabilityWithAi) { if (glFeatures.resolveVulnerabilityAiGateway) { buttons.push(CREATE_MR_AI_ACTION); - } else if (glFeatures.resolveVulnerability) { + } else { buttons.push(CREATE_MR_AI_ACTION_DEPRECATED); } + } + if (this.glAbilities.explainVulnerabilityWithAi) { if (glFeatures.explainVulnerabilityTool) { buttons.push(EXPLAIN_VULNERABILITY_AI_ACTION); } diff --git a/ee/app/assets/javascripts/vulnerabilities/components/vulnerability_details.vue b/ee/app/assets/javascripts/vulnerabilities/components/vulnerability_details.vue index bf94443b0001772604662edcfe03402d42dc89ea..b14dc6cfd46fee187566da7b3bb568ef5e397c7c 100644 --- a/ee/app/assets/javascripts/vulnerabilities/components/vulnerability_details.vue +++ b/ee/app/assets/javascripts/vulnerabilities/components/vulnerability_details.vue @@ -11,8 +11,8 @@ import { } from 'ee/vulnerabilities/constants'; import { s__, __ } from '~/locale'; import CodeBlock from '~/vue_shared/components/code_block.vue'; -import { REPORT_TYPE_SAST } from '~/vue_shared/security_reports/constants'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import glAbilitiesMixin from '~/vue_shared/mixins/gl_abilities_mixin'; import DetailItem from './detail_item.vue'; import FalsePositiveAlert from './false_positive_alert.vue'; import VulnerabilityDetailSection from './vulnerability_detail_section.vue'; @@ -40,7 +40,7 @@ export default { directives: { SafeHtml, }, - mixins: [glFeatureFlagMixin()], + mixins: [glFeatureFlagMixin(), glAbilitiesMixin()], inject: { projectFullPath: { default: '', @@ -209,9 +209,7 @@ export default { return this.location.method || this.location.vulnerableMethod; }, shouldShowExplainVulnerability() { - return ( - this.glFeatures.explainVulnerability && this.vulnerability.reportType === REPORT_TYPE_SAST - ); + return this.glAbilities.explainVulnerabilityWithAi; }, shouldShowFileContents() { // Only show file contents if there's a file and a start line. The start line check prevents the entire file from diff --git a/ee/app/controllers/projects/security/vulnerabilities_controller.rb b/ee/app/controllers/projects/security/vulnerabilities_controller.rb index 7e0d2d1876cf40ec4dfa1b5ea28f954fa55025be..80199a72b633889b8bb7defc0c6d5208ffd4ee31 100644 --- a/ee/app/controllers/projects/security/vulnerabilities_controller.rb +++ b/ee/app/controllers/projects/security/vulnerabilities_controller.rb @@ -17,14 +17,8 @@ class VulnerabilitiesController < Projects::ApplicationController track_govern_activity 'security_vulnerabilities', :show def show - push_force_frontend_feature_flag( - :explain_vulnerability, - can?(current_user, :explain_vulnerability, vulnerability) - ) - push_force_frontend_feature_flag( - :resolve_vulnerability, - can?(current_user, :resolve_vulnerability, vulnerability) - ) + push_frontend_ability(ability: :explain_vulnerability_with_ai, resource: vulnerability, user: current_user) + push_frontend_ability(ability: :resolve_vulnerability_with_ai, resource: vulnerability, user: current_user) push_frontend_feature_flag(:explain_vulnerability_tool, vulnerability.project) push_frontend_feature_flag(:resolve_vulnerability_ai_gateway, vulnerability.project) diff --git a/ee/app/policies/vulnerability_policy.rb b/ee/app/policies/vulnerability_policy.rb index a5f38a80356807898764479c26cb74d33ac95200..adc79d6b8199227b81f6a1a4a9b238ca3bdbfe1b 100644 --- a/ee/app/policies/vulnerability_policy.rb +++ b/ee/app/policies/vulnerability_policy.rb @@ -26,8 +26,8 @@ class VulnerabilityPolicy < BasePolicy ).allowed? end - rule { explain_vulnerability_enabled & can?(:read_security_resource) }.enable(:explain_vulnerability) - rule { resolve_vulnerability_enabled & can?(:read_security_resource) }.enable(:resolve_vulnerability) + rule { explain_vulnerability_enabled & can?(:read_security_resource) }.enable(:explain_vulnerability_with_ai) + rule { resolve_vulnerability_enabled & can?(:read_security_resource) }.enable(:resolve_vulnerability_with_ai) rule { can?(:admin_vulnerability) }.enable :create_external_issue_link rule { project.security_dashboard_enabled & can?(:developer_access) }.enable :create_external_issue_link end diff --git a/ee/spec/frontend/vulnerabilities/header_spec.js b/ee/spec/frontend/vulnerabilities/header_spec.js index 0370bddc447182c452ee47607b05c00638048bcc..ab8ba04ba5aa0983951588e950f938cf0510940b 100644 --- a/ee/spec/frontend/vulnerabilities/header_spec.js +++ b/ee/spec/frontend/vulnerabilities/header_spec.js @@ -81,19 +81,12 @@ describe('Vulnerability Header', () => { const diff = 'some diff to download'; - const getVulnerability = ({ - canCreateMergeRequest, - canDownloadPatch, - canResolveWithAI, - canExplainWithAI, - canAdmin = true, - }) => ({ + const getVulnerability = ({ canCreateMergeRequest, canDownloadPatch, canAdmin = true } = {}) => ({ remediations: canCreateMergeRequest || canDownloadPatch ? [{ diff }] : null, state: canDownloadPatch ? 'detected' : 'resolved', mergeRequestLinks: canCreateMergeRequest || canDownloadPatch ? [] : [{}], mergeRequestFeedback: canCreateMergeRequest ? null : {}, canAdmin, - ...(canResolveWithAI || canExplainWithAI ? { reportType: 'sast' } : {}), ...(canDownloadPatch && canCreateMergeRequest === undefined ? { createMrUrl: '' } : {}), }); @@ -121,7 +114,7 @@ describe('Vulnerability Header', () => { dropdown.vm.$emit('change', { action }); }; - const createWrapper = ({ vulnerability = {}, apolloProvider, glFeatures }) => { + const createWrapper = ({ vulnerability = {}, apolloProvider, glFeatures, glAbilities }) => { wrapper = shallowMount(Header, { apolloProvider, directives: { @@ -140,6 +133,11 @@ describe('Vulnerability Header', () => { resolveVulnerabilityAiGateway: true, ...glFeatures, }, + glAbilities: { + explainVulnerabilityWithAi: true, + resolveVulnerabilityWithAi: true, + ...glAbilities, + }, }, }); }; @@ -349,8 +347,6 @@ describe('Vulnerability Header', () => { vulnerability: getVulnerability({ canCreateMergeRequest: true, canDownloadPatch: true, - canResolveWithAI: true, - canExplainWithAI: true, }), }); await waitForPromises(); @@ -369,8 +365,6 @@ describe('Vulnerability Header', () => { vulnerability: getVulnerability({ canCreateMergeRequest: true, canDownloadPatch: true, - canResolveWithAI: true, - canExplainWithAI: true, }), }); await waitForPromises(); @@ -385,9 +379,11 @@ describe('Vulnerability Header', () => { vulnerability: getVulnerability({ canCreateMergeRequest: false, canDownloadPatch: false, - canResolveWithAI: false, - canExplainWithAI: false, }), + glAbilities: { + explainVulnerabilityWithAi: false, + resolveVulnerabilityWithAi: false, + }, }); expect(findSplitButton().exists()).toBe(false); @@ -402,6 +398,10 @@ describe('Vulnerability Header', () => { vulnerability: getVulnerability({ [state]: true, }), + glAbilities: { + explainVulnerabilityWithAi: false, + resolveVulnerabilityWithAi: false, + }, }); const buttons = findSplitButton().props('buttons'); @@ -424,10 +424,14 @@ describe('Vulnerability Header', () => { ...defaultVulnerability, canCreateMergeRequest: true, canDownloadPatch: true, - canResolveWithAI: false, }; - createWrapper({ vulnerability: getVulnerability(vulnerability) }); + createWrapper({ + vulnerability: getVulnerability(vulnerability), + glAbilities: { + resolveVulnerabilityWithAi: false, + }, + }); await waitForPromises(); const mergeRequestPath = '/group/project/merge_request/123'; mockAxios.onPost(vulnerability.createMrUrl).reply(HTTP_STATUS_OK, { @@ -459,7 +463,6 @@ describe('Vulnerability Header', () => { vulnerability: getVulnerability({ canCreateMergeRequest: true, canDownloadPatch: true, - canResolveWithAI: true, }), }); await waitForPromises(); @@ -490,7 +493,6 @@ describe('Vulnerability Header', () => { vulnerability: getVulnerability({ canCreateMergeRequest: true, canDownloadPatch: true, - canResolveWithAI: true, }), }); await waitForPromises(); @@ -511,7 +513,6 @@ describe('Vulnerability Header', () => { const createWrapperWithAiApollo = ({ mutationResponse = MUTATION_AI_ACTION_DEFAULT_RESPONSE, - getVulnerabilityParam = { canResolveWithAI: true }, } = {}) => { mockSubscription = createMockSubscription(); subscriptionSpy = jest.fn().mockReturnValue(mockSubscription); @@ -520,7 +521,7 @@ describe('Vulnerability Header', () => { apolloProvider.defaultClient.setRequestHandler(aiResponseSubscription, subscriptionSpy); createWrapper({ - vulnerability: getVulnerability(getVulnerabilityParam), + vulnerability: getVulnerability(), apolloProvider, }); @@ -547,9 +548,7 @@ describe('Vulnerability Header', () => { it('passes the category and tanuki icon', () => { createWrapper({ - vulnerability: getVulnerability({ - canResolveWithAI: true, - }), + vulnerability: getVulnerability(), }); expect(findResolveWithAIButton()).toMatchObject({ @@ -561,9 +560,7 @@ describe('Vulnerability Header', () => { // Note: when the `resolveVulnerability` is removed, these tests can be deleted as well it('does not pass the experimental badge or tooltip', () => { createWrapper({ - vulnerability: getVulnerability({ - canResolveWithAI: true, - }), + vulnerability: getVulnerability(), }); expect(findResolveWithAIButton()).not.toHaveProperty('badge'); @@ -629,7 +626,6 @@ describe('Vulnerability Header', () => { await createWrapperWithAiApollo({ canCreateMergeRequest: true, canDownloadPatch: true, - canResolveWithAI: true, }); await clickButton('start-subscription'); expect(subscriptionSpy).toHaveBeenCalled(); @@ -651,9 +647,7 @@ describe('Vulnerability Header', () => { const apolloProvider = createApolloProvider([chatMutation, chatMutationHandlerMock]); createWrapper({ - vulnerability: getVulnerability({ - canExplainWithAI: true, - }), + vulnerability: getVulnerability(), apolloProvider, }); }); @@ -695,10 +689,12 @@ describe('Vulnerability Header', () => { resolveVulnerabilityAiGateway: false, resolveVulnerability: false, }, + glAbilities: { + resolveVulnerabilityWithAi: false, + }, vulnerability: getVulnerability({ canCreateMergeRequest: true, canDownloadPatch: true, - canResolveWithAI: true, }), }); await waitForPromises(); @@ -720,9 +716,7 @@ describe('Vulnerability Header', () => { resolveVulnerabilityAiGateway: false, resolveVulnerability: true, }, - vulnerability: getVulnerability({ - canResolveWithAI: true, - }), + vulnerability: getVulnerability(), }); const buttons = findSplitButton().props('buttons'); @@ -742,9 +736,7 @@ describe('Vulnerability Header', () => { glFeatures: { explainVulnerabilityTool: false, }, - vulnerability: getVulnerability({ - canExplainWithAI: true, - }), + vulnerability: getVulnerability(), }); await waitForPromises(); diff --git a/ee/spec/frontend/vulnerabilities/vulnerability_details_spec.js b/ee/spec/frontend/vulnerabilities/vulnerability_details_spec.js index 18215a0c2e3ead093564f03c6c72e3d910fe1098..2dfb4029524964724dafc802ffd5b5b1b2d4aaf9 100644 --- a/ee/spec/frontend/vulnerabilities/vulnerability_details_spec.js +++ b/ee/spec/frontend/vulnerabilities/vulnerability_details_spec.js @@ -43,7 +43,7 @@ describe('Vulnerability Details', () => { const createWrapper = ( vulnerabilityOverrides, { - explainVulnerability = false, + explainVulnerabilityWithAiAbility = false, mockVulnerabilityTrainingTemplate = false, duoChatIntegration = true, } = {}, @@ -56,7 +56,8 @@ describe('Vulnerability Details', () => { provide: { projectFullPath: TEST_PROJECT_FULL_PATH, canViewFalsePositive: true, - glFeatures: { explainVulnerability, explainVulnerabilityTool: duoChatIntegration }, + glFeatures: { explainVulnerabilityTool: duoChatIntegration }, + glAbilities: { explainVulnerabilityWithAi: explainVulnerabilityWithAiAbility }, }, stubs: { VulnerabilityFileContents: true, @@ -569,20 +570,20 @@ describe('Vulnerability Details', () => { describe('Explain vulnerability feature', () => { it.each` - reportType | isShown | explainVulnerability + reportType | isShown | explainVulnerabilityWithAiAbility ${REPORT_TYPE_SAST} | ${true} | ${true} ${REPORT_TYPE_SAST} | ${false} | ${false} - ${REPORT_TYPE_DAST} | ${false} | ${true} - ${REPORT_TYPE_SECRET_DETECTION} | ${false} | ${true} - ${REPORT_TYPE_DEPENDENCY_SCANNING} | ${false} | ${true} - ${REPORT_TYPE_CONTAINER_SCANNING} | ${false} | ${true} - ${REPORT_TYPE_CLUSTER_IMAGE_SCANNING} | ${false} | ${true} - ${REPORT_TYPE_COVERAGE_FUZZING} | ${false} | ${true} - ${REPORT_TYPE_API_FUZZING} | ${false} | ${true} + ${REPORT_TYPE_DAST} | ${false} | ${false} + ${REPORT_TYPE_SECRET_DETECTION} | ${false} | ${false} + ${REPORT_TYPE_DEPENDENCY_SCANNING} | ${false} | ${false} + ${REPORT_TYPE_CONTAINER_SCANNING} | ${false} | ${false} + ${REPORT_TYPE_CLUSTER_IMAGE_SCANNING} | ${false} | ${false} + ${REPORT_TYPE_COVERAGE_FUZZING} | ${false} | ${false} + ${REPORT_TYPE_API_FUZZING} | ${false} | ${false} `( 'shows the Explain Vulnerability component for report type $reportType? $isShown', - ({ reportType, isShown, explainVulnerability }) => { - createWrapper({ reportType }, { explainVulnerability }); + ({ reportType, isShown, explainVulnerabilityWithAiAbility }) => { + createWrapper({ reportType }, { explainVulnerabilityWithAiAbility }); expect(findExplainVulnerability().exists()).toBe(isShown); expect(wrapper.findComponent(ExplainVulnerabilityLegacy).exists()).toBe(false); @@ -592,7 +593,7 @@ describe('Vulnerability Details', () => { it('shows the legacy Explain Vulnerability component when the "explainVulnerabilityTool" flag is disabled', () => { createWrapper( { reportType: REPORT_TYPE_SAST }, - { explainVulnerability: true, duoChatIntegration: false }, + { explainVulnerabilityWithAiAbility: true, duoChatIntegration: false }, ); expect(wrapper.findComponent(ExplainVulnerabilityLegacy).exists()).toBe(true); diff --git a/ee/spec/policies/vulnerability_policy_spec.rb b/ee/spec/policies/vulnerability_policy_spec.rb index 47344b1bdc440ce9cb16c0b80a893f1f0f11e689..2eb5084ec6b0904feaaae8ee8f4d0896fceeda10 100644 --- a/ee/spec/policies/vulnerability_policy_spec.rb +++ b/ee/spec/policies/vulnerability_policy_spec.rb @@ -78,7 +78,7 @@ end end - describe 'explain_vulnerability' do + describe 'explain_vulnerability_with_ai' do let(:authorizer) { instance_double(::Gitlab::Llm::FeatureAuthorizer) } before do @@ -92,20 +92,20 @@ allow(authorizer).to receive(:allowed?).and_return(true) end - it { is_expected.to be_allowed(:explain_vulnerability) } + it { is_expected.to be_allowed(:explain_vulnerability_with_ai) } context 'when user cannot read_security_resource' do before do project.add_guest(user) end - it { is_expected.to be_disallowed(:explain_vulnerability) } + it { is_expected.to be_disallowed(:explain_vulnerability_with_ai) } end context 'without finding' do let(:vulnerability) { build(:vulnerability, project: project) } - it { is_expected.to be_disallowed(:explain_vulnerability) } + it { is_expected.to be_disallowed(:explain_vulnerability_with_ai) } end end @@ -114,11 +114,11 @@ allow(authorizer).to receive(:allowed?).and_return(false) end - it { is_expected.to be_disallowed(:explain_vulnerability) } + it { is_expected.to be_disallowed(:explain_vulnerability_with_ai) } end end - describe 'resolve_vulnerability' do + describe 'resolve_vulnerability_with_ai' do let(:authorizer) { instance_double(::Gitlab::Llm::FeatureAuthorizer) } before do @@ -133,7 +133,7 @@ end context 'when resolve_vulnerability_ai is enabled' do - it { is_expected.to be_allowed(:resolve_vulnerability) } + it { is_expected.to be_allowed(:resolve_vulnerability_with_ai) } end context 'when resolve_vulnerability_ai is disabled' do @@ -141,7 +141,7 @@ stub_feature_flags(resolve_vulnerability_ai: false) end - it { is_expected.to be_disallowed(:resolve_vulnerability) } + it { is_expected.to be_disallowed(:resolve_vulnerability_with_ai) } end context 'when user cannot read_security_resource' do @@ -149,13 +149,13 @@ project.add_guest(user) end - it { is_expected.to be_disallowed(:resolve_vulnerability) } + it { is_expected.to be_disallowed(:resolve_vulnerability_with_ai) } end context 'without finding' do let(:vulnerability) { build(:vulnerability, project: project) } - it { is_expected.to be_disallowed(:resolve_vulnerability) } + it { is_expected.to be_disallowed(:resolve_vulnerability_with_ai) } end end @@ -164,7 +164,7 @@ allow(authorizer).to receive(:allowed?).and_return(false) end - it { is_expected.to be_disallowed(:resolve_vulnerability) } + it { is_expected.to be_disallowed(:resolve_vulnerability_with_ai) } end end end