Merge branch '241815-remove-security-on-demand-scans-site-validation-ff' into 'master'

Remove security_on_demand_scans_site_validation FF [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!50635

config/feature_flags/development/security_on_demand_scans_site_validation.yml

----
-name: security_on_demand_scans_site_validation
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40685
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/241815
-milestone: '13.4'
-type: development
-group: group::dynamic analysis
-default_enabled: true

doc/api/graphql/reference/index.md

@@ -3227,7 +3227,7 @@ Represents vulnerability finding of a security report on the pipeline.
 | `dastScannerProfiles` | DastScannerProfileConnection | The DAST scanner profiles associated with the project. |
 | `dastSiteProfile` | DastSiteProfile | DAST Site Profile associated with the project. |
 | `dastSiteProfiles` | DastSiteProfileConnection | DAST Site Profiles associated with the project. |
-| `dastSiteValidations` | DastSiteValidationConnection | DAST Site Validations associated with the project. Always returns no nodes if `security_on_demand_scans_site_validation` is disabled. |
+| `dastSiteValidations` | DastSiteValidationConnection | DAST Site Validations associated with the project. |
 | `description` | String | Short description of the project. |
 | `descriptionHtml` | String | The GitLab Flavored Markdown rendering of `description` |
 | `environment` | Environment | A single environment of the project. |

ee/app/assets/javascripts/on_demand_scans/components/on_demand_scans_form.vue

@@ -209,12 +209,7 @@ export default {
       return this.selectedSiteProfile?.validationStatus === DAST_SITE_VALIDATION_STATUS.PASSED;
     },
     hasProfilesConflict() {
-      return (
-        this.glFeatures.securityOnDemandScansSiteValidation &&
-        !this.someFieldEmpty &&
-        this.isActiveScannerProfile &&
-        !this.isValidatedSiteProfile
-      );
+      return !this.someFieldEmpty && this.isActiveScannerProfile && !this.isValidatedSiteProfile;
     },
     isFormInvalid() {
       return this.someFieldEmpty || this.hasProfilesConflict;

ee/app/assets/javascripts/on_demand_scans/components/profile_selector/scanner_profile_selector.vue

@@ -27,10 +27,7 @@ export default {
   computed: {
     formattedProfiles() {
       return this.profiles.map((profile) => {
-        const addSuffix = (str) =>
-          this.glFeatures.securityOnDemandScansSiteValidation
-            ? `${str} (${SCAN_TYPE_LABEL[profile.scanType]})`
-            : str;
+        const addSuffix = (str) => `${str} (${SCAN_TYPE_LABEL[profile.scanType]})`;
         return {
           ...profile,
           dropdownLabel: addSuffix(profile.profileName),

ee/app/assets/javascripts/on_demand_scans/components/profile_selector/site_profile_selector.vue

@@ -32,8 +32,7 @@ export default {
         const suffix = isValidated
           ? s__('DastProfiles|Validated')
           : s__('DastProfiles|Not Validated');
-        const addSuffix = (str) =>
-          this.glFeatures.securityOnDemandScansSiteValidation ? `${str} (${suffix})` : str;
+        const addSuffix = (str) => `${str} (${suffix})`;
         return {
           ...profile,
           dropdownLabel: addSuffix(`${profile.profileName}: ${profile.targetUrl}`),

ee/app/assets/javascripts/security_configuration/dast_profiles/components/dast_site_profiles_list.vue

@@ -12,7 +12,6 @@ import {
 import dastSiteValidationsQuery from 'ee/security_configuration/dast_site_validation/graphql/dast_site_validations.query.graphql';
 import { fetchPolicies } from '~/lib/graphql';
 import { s__ } from '~/locale';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { updateSiteProfilesStatuses } from '../graphql/cache_utils';
 import ProfilesList from './dast_profiles_list.vue';
 
@@ -40,9 +39,7 @@ export default {
       },
       pollInterval: DAST_SITE_VALIDATION_POLLING_INTERVAL,
       skip() {
-        return (
-          !this.glFeatures.securityOnDemandScansSiteValidation || !this.urlsPendingValidation.length
-        );
+        return !this.urlsPendingValidation.length;
       },
       result({
         data: {
@@ -60,7 +57,6 @@ export default {
   directives: {
     GlTooltip: GlTooltipDirective,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
     fullPath: {
       type: String,
@@ -112,7 +108,7 @@ export default {
         : s__('DastSiteValidation|Validate');
     },
     shouldShowValidationStatus(status) {
-      return this.glFeatures.securityOnDemandScansSiteValidation && status !== NONE;
+      return status !== NONE;
     },
     hasValidationPassed(status) {
       return status === PASSED;
@@ -169,10 +165,7 @@ export default {
 
     <template #actions="{ profile }">
       <gl-button
-        v-if="
-          glFeatures.securityOnDemandScansSiteValidation &&
-          !hasValidationPassed(profile.validationStatus)
-        "
+        v-if="!hasValidationPassed(profile.validationStatus)"
         :disabled="!canValidateProfile(profile.validationStatus)"
         variant="info"
         category="tertiary"
@@ -181,10 +174,7 @@ export default {
         >{{ validateBtnLabel(profile.validationStatus) }}</gl-button
       >
       <gl-button
-        v-else-if="
-          glFeatures.securityOnDemandScansSiteValidation &&
-          hasValidationPassed(profile.validationStatus)
-        "
+        v-else
         variant="info"
         category="tertiary"
         size="small"

ee/app/controllers/projects/on_demand_scans_controller.rb

@@ -5,7 +5,6 @@ module Projects
     include SecurityAndCompliancePermissions
 
     before_action do
-      push_frontend_feature_flag(:security_on_demand_scans_site_validation, @project, default_enabled: :yaml)
       push_frontend_feature_flag(:security_dast_site_profiles_additional_fields, @project, default_enabled: :yaml)
       push_frontend_feature_flag(:dast_saved_scans, @project, default_enabled: :yaml)
     end

ee/app/controllers/projects/security/dast_profiles_controller.rb

@@ -7,7 +7,6 @@ module Projects
 
       before_action do
         authorize_read_on_demand_scans!
-        push_frontend_feature_flag(:security_on_demand_scans_site_validation, @project, default_enabled: :yaml)
         push_frontend_feature_flag(:dast_saved_scans, @project, default_enabled: :yaml)
       end
 

ee/app/graphql/ee/types/project_type.rb

@@ -83,8 +83,7 @@ module EE
               ::Types::DastSiteValidationType.connection_type,
               null: true,
               resolver: ::Resolvers::DastSiteValidationResolver,
-              description: 'DAST Site Validations associated with the project. Always returns no nodes ' \
-                           'if `security_on_demand_scans_site_validation` is disabled.'
+              description: 'DAST Site Validations associated with the project.'
 
         field :cluster_agent,
               ::Types::Clusters::AgentType,

ee/app/graphql/mutations/dast_site_tokens/create.rb

@@ -31,7 +31,6 @@ module Mutations
 
       def resolve(full_path:, target_url:)
         project = authorized_find!(full_path)
-        raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless allowed?(project)
 
         response = ::DastSiteTokens::CreateService.new(
           container: project,
@@ -45,10 +44,6 @@ module Mutations
 
       private
 
-      def allowed?(project)
-        Feature.enabled?(:security_on_demand_scans_site_validation, project, default_enabled: :yaml)
-      end
-
       def error_response(errors)
         { errors: errors }
       end

ee/app/graphql/mutations/dast_site_validations/create.rb

@@ -35,7 +35,6 @@ module Mutations
 
       def resolve(full_path:, dast_site_token_id:, validation_path:, strategy: :text_file)
         project = authorized_find!(full_path)
-        raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless allowed?(project)
 
         dast_site_token = dast_site_token_id.find
 
@@ -55,10 +54,6 @@ module Mutations
 
       private
 
-      def allowed?(project)
-        Feature.enabled?(:security_on_demand_scans_site_validation, project, default_enabled: :yaml)
-      end
-
       def error_response(errors)
         { errors: errors }
       end

ee/app/graphql/mutations/dast_site_validations/revoke.rb

@@ -3,8 +3,6 @@
 module Mutations
   module DastSiteValidations
     class Revoke < BaseMutation
-      FEATURE_FLAG = :security_on_demand_scans_site_validation
-
       include FindsProject
 
       graphql_name 'DastSiteValidationRevoke'
@@ -21,7 +19,6 @@ module Mutations
 
       def resolve(full_path:, normalized_target_url:)
         project = authorized_find!(full_path)
-        raise Gitlab::Graphql::Errors::ResourceNotAvailable, "Feature disabled: #{FEATURE_FLAG}" unless allowed?(project)
 
         response = ::DastSiteValidations::RevokeService.new(
           container: project,
@@ -35,10 +32,6 @@ module Mutations
 
       private
 
-      def allowed?(project)
-        Feature.enabled?(FEATURE_FLAG, project, default_enabled: :yaml)
-      end
-
       def error_response(errors)
         { errors: errors }
       end

ee/app/graphql/resolvers/dast_site_validation_resolver.rb

@@ -10,15 +10,7 @@ module Resolvers
              description: 'Normalized URL of the target to be scanned.'
 
     def resolve(**args)
-      return DastSiteValidation.none unless allowed?
-
       DastSiteValidationsFinder.new(project_id: project.id, url_base: args[:normalized_target_urls], most_recent: true).execute
     end
-
-    private
-
-    def allowed?
-      ::Feature.enabled?(:security_on_demand_scans_site_validation, project, default_enabled: :yaml)
-    end
   end
 end

ee/app/services/dast_site_tokens/create_service.rb

@@ -29,8 +29,7 @@ module DastSiteTokens
     private
 
     def allowed?
-      container.feature_available?(:security_on_demand_scans) &&
-        Feature.enabled?(:security_on_demand_scans_site_validation, container, default_enabled: :yaml)
+      container.feature_available?(:security_on_demand_scans)
     end
 
     def normalize_target_url(target_url)

ee/app/services/dast_site_validations/create_service.rb

@@ -23,7 +23,6 @@ module DastSiteValidations
 
     def allowed?
       container.feature_available?(:security_on_demand_scans) &&
-        Feature.enabled?(:security_on_demand_scans_site_validation, container, default_enabled: :yaml) &&
         dast_site_token.project == container
     end
 

ee/app/services/dast_site_validations/revoke_service.rb

@@ -23,8 +23,7 @@ module DastSiteValidations
     private
 
     def allowed?
-      container.feature_available?(:security_on_demand_scans) &&
-        Feature.enabled?(:security_on_demand_scans_site_validation, container, default_enabled: :yaml)
+      container.feature_available?(:security_on_demand_scans)
     end
 
     def url_base

ee/app/services/dast_site_validations/validate_service.rb

@@ -24,8 +24,7 @@ module DastSiteValidations
     private
 
     def allowed?
-      container.feature_available?(:security_on_demand_scans) &&
-        Feature.enabled?(:security_on_demand_scans_site_validation, container, default_enabled: :yaml)
+      container.feature_available?(:security_on_demand_scans)
     end
 
     def dast_site_validation

ee/spec/frontend/on_demand_scans/components/on_demand_scans_form_spec.js

@@ -145,7 +145,6 @@ describe('OnDemandScansForm', () => {
             newScannerProfilePath,
             newSiteProfilePath,
             glFeatures: {
-              securityOnDemandScansSiteValidation: true,
               dastSavedScans: true,
             },
           },
@@ -502,24 +501,6 @@ describe('OnDemandScansForm', () => {
           expect(findSubmitButton().props('disabled')).toBe(hasConflict);
         },
       );
-
-      describe('securityOnDemandScansSiteValidation feature flag disabled', () => {
-        beforeEach(() => {
-          mountShallowSubject({
-            provide: {
-              glFeatures: {
-                securityOnDemandScansSiteValidation: false,
-              },
-            },
-          });
-          return setFormData();
-        });
-
-        it(`does not report any conflict when user selects ${description}`, () => {
-          expect(findProfilesConflictAlert().exists()).toBe(false);
-          expect(findSubmitButton().props('disabled')).toBe(false);
-        });
-      });
     },
   );
 

ee/spec/frontend/on_demand_scans/components/profile_selector/scanner_profile_selector_spec.js

@@ -29,7 +29,6 @@ describe('OnDemandScansScannerProfileSelector', () => {
           provide: {
             scannerProfilesLibraryPath: TEST_LIBRARY_PATH,
             newScannerProfilePath: TEST_NEW_PATH,
-            glFeatures: { securityOnDemandScansSiteValidation: true },
           },
           slots: {
             summary: `<div>${profiles[0].profileName}'s summary</div>`,
@@ -89,28 +88,5 @@ describe('OnDemandScansScannerProfileSelector', () => {
       });
       expect(sel.attributes()).toMatchObject(TEST_ATTRS);
     });
-
-    describe('feature flag disabled', () => {
-      beforeEach(() => {
-        createComponent({
-          propsData: { profiles },
-          provide: {
-            glFeatures: { securityOnDemandScansSiteValidation: false },
-          },
-        });
-      });
-
-      it('renders profile selector', () => {
-        const sel = findProfileSelector();
-
-        expect(sel.props()).toEqual({
-          libraryPath: TEST_LIBRARY_PATH,
-          newProfilePath: TEST_NEW_PATH,
-          profiles: scannerProfiles.map((x) => ({ ...x, dropdownLabel: `${x.profileName}` })),
-          value: null,
-        });
-        expect(sel.attributes()).toMatchObject(TEST_ATTRS);
-      });
-    });
   });
 });

ee/spec/frontend/on_demand_scans/components/profile_selector/site_profile_selector_spec.js

@@ -33,7 +33,6 @@ describe('OnDemandScansSiteProfileSelector', () => {
             siteProfilesLibraryPath: TEST_LIBRARY_PATH,
             newSiteProfilePath: TEST_NEW_PATH,
             glFeatures: {
-              securityOnDemandScansSiteValidation: true,
               securityDastSiteProfilesAdditionalFields: true,
             },
           },
@@ -95,34 +94,5 @@ describe('OnDemandScansSiteProfileSelector', () => {
       });
       expect(sel.attributes()).toMatchObject(TEST_ATTRS);
     });
-
-    describe('feature flags disabled', () => {
-      beforeEach(() => {
-        createComponent({
-          propsData: { profiles },
-          provide: {
-            glFeatures: {
-              securityOnDemandScansSiteValidation: false,
-              securityDastSiteProfilesAdditionalFields: false,
-            },
-          },
-        });
-      });
-
-      it('renders profile selector', () => {
-        const sel = findProfileSelector();
-
-        expect(sel.props()).toEqual({
-          libraryPath: TEST_LIBRARY_PATH,
-          newProfilePath: TEST_NEW_PATH,
-          profiles: siteProfiles.map((x) => ({
-            ...x,
-            dropdownLabel: `${x.profileName}: ${x.targetUrl}`,
-          })),
-          value: null,
-        });
-        expect(sel.attributes()).toMatchObject(TEST_ATTRS);
-      });
-    });
   });
 });

ee/spec/frontend/security_configuration/dast_profiles/components/dast_site_profiles_list_spec.js

@@ -52,9 +52,6 @@ describe('EE - DastSiteProfileList', () => {
       merge(
         {
           propsData: defaultProps,
-          provide: {
-            glFeatures: { securityOnDemandScansSiteValidation: true },
-          },
         },
         { ...options, localVue, apolloProvider },
       ),
@@ -107,7 +104,7 @@ describe('EE - DastSiteProfileList', () => {
     expect(inputHandler).toHaveBeenCalled();
   });
 
-  describe('with site validation enabled', () => {
+  describe('site validation', () => {
     const [pendingValidation, inProgressValidation] = siteProfiles;
     const urlsPendingValidation = [
       pendingValidation.normalizedTargetUrl,
@@ -229,22 +226,4 @@ describe('EE - DastSiteProfileList', () => {
       },
     );
   });
-
-  describe('without site validation enabled', () => {
-    beforeEach(() => {
-      createFullComponent({
-        provide: {
-          glFeatures: { securityOnDemandScansSiteValidation: false },
-        },
-        propsData: { siteProfiles },
-      });
-    });
-
-    it.each(siteProfiles)('profile %# should not have validate button and status', (profile) => {
-      const [, , validationStatusCell, actionsCell] = getTableRowForProfile(profile).cells;
-
-      expect(within(actionsCell).queryByRole('button', { name: /validate/i })).toBe(null);
-      expect(validationStatusCell.innerText).toBe('');
-    });
-  });
 });

ee/spec/graphql/mutations/dast_site_tokens/create_spec.rb

@@ -66,14 +66,6 @@ RSpec.describe Mutations::DastSiteTokens::Create do
             expect(result[:status]).to eq('failed')
           end
         end
-
-        context 'when on demand scan site validations feature is not enabled' do
-          it 'raises an exception' do
-            stub_feature_flags(security_on_demand_scans_site_validation: false)
-
-            expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
-          end
-        end
       end
     end
   end

ee/spec/graphql/mutations/dast_site_validations/create_spec.rb

@@ -51,14 +51,6 @@ RSpec.describe Mutations::DastSiteValidations::Create do
         it 'returns the dast_site_validation status' do
           expect(subject[:status]).to eq(dast_site_validation.state)
         end
-
-        context 'when on demand scan site validations feature is not enabled' do
-          it 'raises an exception' do
-            stub_feature_flags(security_on_demand_scans_site_validation: false)
-
-            expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
-          end
-        end
       end
     end
   end

ee/spec/graphql/mutations/dast_site_validations/revoke_spec.rb

@@ -55,16 +55,6 @@ RSpec.describe Mutations::DastSiteValidations::Revoke do
 
           subject
         end
-
-        context 'when on demand scan site validations feature is not enabled' do
-          it 'raises an exception' do
-            stub_feature_flags(security_on_demand_scans_site_validation: false)
-
-            expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) do |err|
-              expect(err.message).to eq('Feature disabled: security_on_demand_scans_site_validation')
-            end
-          end
-        end
       end
     end
   end

ee/spec/services/dast/profiles/update_service_spec.rb

@@ -45,7 +45,7 @@ RSpec.describe Dast::Profiles::UpdateService do
     context 'when on demand scan licensed feature is not available' do
       it 'communicates failure' do
         stub_licensed_features(security_on_demand_scans: false)
-        stub_feature_flags(security_on_demand_scans_site_validation: true)
+        stub_feature_flags(dast_saved_scans: true)
 
         aggregate_failures do
           expect(subject.status).to eq(:error)

ee/spec/services/dast_site_tokens/create_service_spec.rb

@@ -14,22 +14,9 @@ RSpec.describe DastSiteTokens::CreateService do
   end
 
   describe 'execute' do
-    context 'when on demand scan feature is disabled' do
-      it 'communicates failure' do
-        stub_licensed_features(security_on_demand_scans: true)
-        stub_feature_flags(security_on_demand_scans_site_validation: false)
-
-        aggregate_failures do
-          expect(subject.status).to eq(:error)
-          expect(subject.message).to eq('Insufficient permissions')
-        end
-      end
-    end
-
     context 'when on demand scan licensed feature is not available' do
       it 'communicates failure' do
         stub_licensed_features(security_on_demand_scans: false)
-        stub_feature_flags(security_on_demand_scans_site_validation: true)
 
         aggregate_failures do
           expect(subject.status).to eq(:error)
@@ -38,10 +25,9 @@ RSpec.describe DastSiteTokens::CreateService do
       end
     end
 
-    context 'when the feature is enabled' do
+    context 'when the feature is available' do
       before do
         stub_licensed_features(security_on_demand_scans: true)
-        stub_feature_flags(security_on_demand_scans_site_validation: true)
       end
 
       it 'communicates success' do

ee/spec/services/dast_site_validations/create_service_spec.rb

@@ -14,8 +14,8 @@ RSpec.describe DastSiteValidations::CreateService do
   describe 'execute', :clean_gitlab_redis_shared_state do
     context 'when on demand scan feature is disabled' do
       it 'communicates failure' do
-        stub_licensed_features(security_on_demand_scans: true)
-        stub_feature_flags(security_on_demand_scans_site_validation: false)
+        stub_licensed_features(security_on_demand_scans: false)
+        stub_feature_flags(dast_saved_scans: false)
 
         aggregate_failures do
           expect(subject.status).to eq(:error)
@@ -27,7 +27,6 @@ RSpec.describe DastSiteValidations::CreateService do
     context 'when on demand scan licensed feature is not available' do
       it 'communicates failure' do
         stub_licensed_features(security_on_demand_scans: false)
-        stub_feature_flags(security_on_demand_scans_site_validation: true)
 
         aggregate_failures do
           expect(subject.status).to eq(:error)
@@ -36,10 +35,9 @@ RSpec.describe DastSiteValidations::CreateService do
       end
     end
 
-    context 'when the feature is enabled' do
+    context 'when the feature is available' do
       before do
         stub_licensed_features(security_on_demand_scans: true)
-        stub_feature_flags(security_on_demand_scans_site_validation: true)
       end
 
       it 'communicates success' do

ee/spec/services/dast_site_validations/revoke_service_spec.rb

@@ -18,8 +18,8 @@ RSpec.describe DastSiteValidations::RevokeService do
   describe 'execute', :clean_gitlab_redis_shared_state do
     context 'when on demand scan feature is disabled' do
       it 'communicates failure' do
-        stub_licensed_features(security_on_demand_scans: true)
-        stub_feature_flags(security_on_demand_scans_site_validation: false)
+        stub_licensed_features(security_on_demand_scans: false)
+        stub_feature_flags(dast_saved_scans: false)
 
         aggregate_failures do
           expect(subject.status).to eq(:error)
@@ -31,7 +31,6 @@ RSpec.describe DastSiteValidations::RevokeService do
     context 'when on demand scan licensed feature is not available' do
       it 'communicates failure' do
         stub_licensed_features(security_on_demand_scans: false)
-        stub_feature_flags(security_on_demand_scans_site_validation: true)
 
         aggregate_failures do
           expect(subject.status).to eq(:error)
@@ -43,7 +42,6 @@ RSpec.describe DastSiteValidations::RevokeService do
     context 'when the feature is enabled' do
       before do
         stub_licensed_features(security_on_demand_scans: true)
-        stub_feature_flags(security_on_demand_scans_site_validation: true)
       end
 
       it 'communicates success' do

ee/spec/services/dast_site_validations/validate_service_spec.rb

@@ -15,28 +15,17 @@ RSpec.describe DastSiteValidations::ValidateService do
   end
 
   describe 'execute!' do
-    context 'when on demand scan feature is disabled' do
-      it 'communicates failure' do
-        stub_licensed_features(security_on_demand_scans: true)
-        stub_feature_flags(security_on_demand_scans_site_validation: false)
-
-        expect { subject }.to raise_error(DastSiteValidations::ValidateService::PermissionsError)
-      end
-    end
-
     context 'when on demand scan licensed feature is not available' do
       it 'communicates failure' do
         stub_licensed_features(security_on_demand_scans: false)
-        stub_feature_flags(security_on_demand_scans_site_validation: true)
 
         expect { subject }.to raise_error(DastSiteValidations::ValidateService::PermissionsError)
       end
     end
 
-    context 'when the feature is enabled' do
+    context 'when the feature is available' do
       before do
         stub_licensed_features(security_on_demand_scans: true)
-        stub_feature_flags(security_on_demand_scans_site_validation: true)
         stub_request(:get, dast_site_validation.validation_url).to_return(body: token, headers: headers)
       end
 

ee/spec/workers/dast_site_validation_worker_spec.rb

@@ -20,13 +20,12 @@ RSpec.describe DastSiteValidationWorker do
       subject
     end
 
-    context 'when the feature is enabled' do
+    context 'when the feature is available' do
       let(:response_body) { dast_site_validation.dast_site_token.token }
       let(:headers) { { 'Content-Type' => 'text/plain; charset=utf-8' } }
 
       before do
         stub_licensed_features(security_on_demand_scans: true)
-        stub_feature_flags(security_on_demand_scans_site_validation: true)
         stub_request(:get, dast_site_validation.validation_url).to_return(body: response_body, headers: headers)
       end