Skip to content

G
gitlab-ce
Commit e1d3bc04 authored by Philip Cunningham's avatar Philip Cunningham Committed by Bob Van Landuyt
Browse files
Options

Remove DAST site validation feature flags

parent 31462b14
Hide whitespace changes
Inline Side-by-side
Showing with 40 additions and 530 deletions
config/feature_flags/development/dast_meta_tag_validation.yml deleted 100644 → 0
View file @ 31462b14
---
name: dast_meta_tag_validation
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67945
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/337711
milestone: '14.2'
type: development
group: group::dynamic analysis
default_enabled: true
config/feature_flags/development/dast_runner_site_validation.yml deleted 100644 → 0
View file @ 31462b14
---
name: dast_runner_site_validation
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61649
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/331082
milestone: '14.0'
type: development
group: group::dynamic analysis
default_enabled: true
doc/user/application_security/dast/index.md
View file @ e1d3bc04
......@@ -1049,11 +1049,7 @@ When an API site type is selected, a [host override](#host-override) is used to
#### Site profile validation
> - Site profile validation [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/233020) in GitLab 13.8.
> - Meta tag validation [enabled on GitLab.com](https://gitlab.com/groups/gitlab-org/-/epics/6460) in GitLab 14.2 and is ready for production use.
> - Meta tag validation [enabled with `dast_meta_tag_validation flag` flag](https://gitlab.com/gitlab-org/gitlab/-/issues/337711) for self-managed GitLab in GitLab 14.2 and is ready for production use.
FLAG:
On self-managed GitLab, by default this feature is available. To hide the feature, ask an administrator to [disable the `dast_meta_tag_validation` flag](../../../administration/feature_flags.md). On GitLab.com, this feature is available but can be configured by GitLab.com administrators only.
> - Meta tag validation [introduced](https://gitlab.com/groups/gitlab-org/-/epics/6460) in GitLab 14.2.
Site profile validation reduces the risk of running an active scan against the wrong website. A site
must be validated before an active scan can run against it. The site validation methods are as
......
ee/app/assets/javascripts/security_configuration/dast_site_validation/components/dast_site_validation_modal.vue
View file @ e1d3bc04
......@@ -18,7 +18,6 @@ import download from '~/lib/utils/downloader';
import { cleanLeadingSeparator, joinPaths, stripPathTail } from '~/lib/utils/url_utility';
import { __, s__ } from '~/locale';
import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue';
import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import {
DAST_SITE_VALIDATION_MODAL_ID,
DAST_SITE_VALIDATION_HTTP_HEADER_KEY,
......@@ -47,7 +46,6 @@ export default {
GlTruncate,
GlLink,
},
mixins: [glFeatureFlagMixin()],
props: {
fullPath: {
type: String,
......@@ -91,11 +89,7 @@ export default {
};
},
validationMethodOptions() {
const options = Object.values(DAST_SITE_VALIDATION_METHODS);
if (!this.glFeatures.dastMetaTagValidation) {
return options.filter(({ value }) => value !== DAST_SITE_VALIDATION_METHOD_META_TAG);
}
return options;
return Object.values(DAST_SITE_VALIDATION_METHODS);
},
urlObject() {
try {
......
ee/app/controllers/projects/security/dast_profiles_controller.rb
View file @ e1d3bc04
......@@ -8,7 +8,6 @@ module Projects
before_action do
authorize_read_on_demand_scans!
push_frontend_feature_flag(:dast_failed_site_validations, @project, default_enabled: :yaml)
push_frontend_feature_flag(:dast_meta_tag_validation, @project, default_enabled: :yaml)
end
feature_category :dynamic_application_security_testing
......
ee/app/models/dast_site.rb
View file @ e1d3bc04
......@@ -6,8 +6,7 @@ class DastSite < ApplicationRecord
has_many :dast_site_profiles
validates :url, length: { maximum: 255 }, uniqueness: { scope: :project_id }
validates :url, addressable_url: true, if: :runner_validation_enabled?
validates :url, public_url: true, unless: :runner_validation_enabled?
validates :url, addressable_url: true
validates :project_id, presence: true
validate :dast_site_validation_project_id_fk
......@@ -21,8 +20,4 @@ class DastSite < ApplicationRecord
errors.add(:project_id, 'does not match dast_site_validation.project')
end
end
def runner_validation_enabled?
::Feature.enabled?(:dast_runner_site_validation, project, default_enabled: :yaml)
end
end
ee/app/models/dast_site_validation.rb
View file @ e1d3bc04
......@@ -9,8 +9,6 @@ class DastSiteValidation < ApplicationRecord
validates :dast_site_token_id, presence: true
validates :validation_strategy, presence: true
validate :meta_tag_validation_must_happen_on_runner, if: :meta_tag?
scope :by_project_id, -> (project_id) do
joins(:dast_site_token).where(dast_site_tokens: { project_id: project_id })
end
......@@ -81,11 +79,4 @@ class DastSiteValidation < ApplicationRecord
def set_normalized_url_base
self.url_base = self.class.get_normalized_url_base(dast_site_token.url)
end
def meta_tag_validation_must_happen_on_runner
return if ::Feature.enabled?(:dast_runner_site_validation, project, default_enabled: :yaml) &&
::Feature.enabled?(:dast_meta_tag_validation, project, default_enabled: :yaml)
errors.add(:base, 'Meta tag validation is not enabled')
end
end
ee/app/services/app_sec/dast/site_validations/runner_service.rb
View file @ e1d3bc04
......@@ -22,8 +22,7 @@ module AppSec
private
def allowed?
can?(current_user, :create_on_demand_dast_scan, project) &&
::Feature.enabled?(:dast_runner_site_validation, project, default_enabled: :yaml)
can?(current_user, :create_on_demand_dast_scan, project)
end
def dast_site_validation
......
ee/app/services/dast_site_validations/create_service.rb
View file @ e1d3bc04
......@@ -12,7 +12,7 @@ module DastSiteValidations
associate_dast_site!(dast_site_validation)
perform_async_validation(dast_site_validation)
perform_runner_validation(dast_site_validation)
rescue ActiveRecord::RecordInvalid => err
ServiceResponse.error(message: err.record.errors.full_messages)
rescue KeyError => err
......@@ -66,29 +66,7 @@ module DastSiteValidations
)
end
def perform_async_validation(dast_site_validation)
if Feature.enabled?(:dast_runner_site_validation, dast_site_validation.project, default_enabled: :yaml)
runner_validation(dast_site_validation)
else
worker_validation(dast_site_validation)
end
end
def worker_validation(dast_site_validation)
jid = DastSiteValidationWorker.perform_async(dast_site_validation.id)
unless jid.present?
log_error(message: 'Unable to validate dast_site_validation', dast_site_validation_id: dast_site_validation.id)
dast_site_validation.fail_op
return ServiceResponse.error(message: 'Validation failed')
end
ServiceResponse.success(payload: dast_site_validation)
end
def runner_validation(dast_site_validation)
def perform_runner_validation(dast_site_validation)
AppSec::Dast::SiteValidations::RunnerService.new(
project: dast_site_validation.project,
current_user: current_user,
......
ee/app/services/dast_site_validations/validate_service.rb deleted 100644 → 0
View file @ 31462b14
# frozen_string_literal: true
module DastSiteValidations
class ValidateService < BaseContainerService
PermissionsError = Class.new(StandardError)
TokenNotFound = Class.new(StandardError)
def execute!
raise PermissionsError, 'Insufficient permissions' unless allowed?
return if dast_site_validation.passed?
if dast_site_validation.pending?
dast_site_validation.start
else
dast_site_validation.retry
end
response = make_http_request!
validate!(response)
end
private
def allowed?
container.feature_available?(:security_on_demand_scans)
end
def dast_site_validation
@dast_site_validation ||= params.fetch(:dast_site_validation)
end
def make_http_request!
Gitlab::HTTP.get(dast_site_validation.validation_url, use_read_total_timeout: true)
end
def token_found?(response)
token = dast_site_validation.dast_site_token.token
case dast_site_validation.validation_strategy
when 'text_file'
response.content_type == 'text/plain' && response.body.rstrip == token
when 'header'
response.headers[DastSiteValidation::HEADER] == token
else
false
end
end
def validate!(response)
raise TokenNotFound, 'Could not find token' unless token_found?(response)
dast_site_validation.pass
end
end
end
ee/app/workers/dast_site_validation_worker.rb
View file @ e1d3bc04
......@@ -14,18 +14,8 @@ class DastSiteValidationWorker
feature_category :dynamic_application_security_testing
tags :exclude_from_kubernetes
sidekiq_retries_exhausted do |job|
dast_site_validation = DastSiteValidation.find(job['args'][0])
dast_site_validation.fail_op
end
def perform(dast_site_validation_id)
dast_site_validation = DastSiteValidation.find(dast_site_validation_id)
project = dast_site_validation.project
DastSiteValidations::ValidateService.new(
container: project,
params: { dast_site_validation: dast_site_validation }
).execute!
def perform(_dast_site_validation_id)
# Scheduled for removal in %15.0
# Please see https://gitlab.com/gitlab-org/gitlab/-/issues/339088
end
end
ee/lib/api/internal/app_sec/dast/site_validations.rb
View file @ e1d3bc04
......@@ -15,9 +15,7 @@ module API
namespace :internal do
namespace :dast do
resource :site_validations do
desc 'Transitions a DAST site validation to a new state.' do
detail 'This feature is gated by the :dast_runner_site_validation feature flag.'
end
desc 'Transitions a DAST site validation to a new state.'
route_setting :authentication, job_token_allowed: true
params do
requires :event, type: Symbol, values: %i[start fail_op retry pass], desc: 'The transition event.'
......@@ -25,10 +23,6 @@ module API
post ':id/transition' do
validation = DastSiteValidation.find(params[:id])
unless Feature.enabled?(:dast_runner_site_validation, validation.project, default_enabled: :yaml)
render_api_error!('404 Feature flag disabled: :dast_runner_site_validation', 404)
end
authorize!(:create_on_demand_dast_scan, validation)
bad_request!('Project mismatch') unless current_authenticated_job.project == validation.project
......
ee/spec/frontend/security_configuration/dast_profiles/dast_site_validation/components/dast_site_validation_modal_spec.js
View file @ e1d3bc04
......@@ -57,9 +57,6 @@ describe('DastSiteValidationModal', () => {
static: true,
visible: true,
},
provide: {
glFeatures: { dastMetaTagValidation: true },
},
},
mountOptions,
{
......@@ -324,22 +321,6 @@ describe('DastSiteValidationModal', () => {
});
});
describe('with the dastMetaTagValidation feature flag disabled', () => {
beforeEach(() => {
createFullComponent({
provide: {
glFeatures: {
dastMetaTagValidation: false,
},
},
});
});
it('does not render the meta tag validation method', () => {
expect(findRadioInputForValidationMethod('meta tag')).toBe(null);
});
});
describe.each(validationMethods)('"%s" validation submission', (validationMethod) => {
beforeEach(async () => {
createFullComponent();
......
ee/spec/graphql/mutations/dast_site_validations/create_spec.rb
View file @ e1d3bc04
......@@ -29,42 +29,28 @@ RSpec.describe Mutations::DastSiteValidations::Create do
)
end
shared_examples 'a validation mutation' do
context 'when on demand scan feature is enabled' do
context 'when the project does not exist' do
let(:full_path) { SecureRandom.hex }
context 'when on demand scan feature is enabled' do
context 'when the project does not exist' do
let(:full_path) { SecureRandom.hex }
it 'raises an exception' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
it 'raises an exception' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
context 'when the user can run a dast scan' do
before do
project.add_developer(user)
end
it 'returns the dast_site_validation id' do
expect(subject[:id]).to eq(dast_site_validation.to_global_id)
end
context 'when the user can run a dast scan' do
before do
project.add_developer(user)
end
it 'returns the dast_site_validation status' do
expect(subject[:status]).to eq(dast_site_validation.state)
end
it 'returns the dast_site_validation id' do
expect(subject[:id]).to eq(dast_site_validation.to_global_id)
end
end
end
context 'worker validation' do
before do
stub_feature_flags(dast_runner_site_validation: false)
it 'returns the dast_site_validation status' do
expect(subject[:status]).to eq(dast_site_validation.state)
end
end
it_behaves_like 'a validation mutation'
end
context 'runner validation' do
it_behaves_like 'a validation mutation'
end
end
end
ee/spec/models/dast_site_spec.rb
View file @ e1d3bc04
......@@ -38,22 +38,9 @@ RSpec.describe DastSite, type: :model do
subject { build(:dast_site, project: project, url: 'http://127.0.0.1') }
context 'worker validation' do
before do
stub_feature_flags(dast_runner_site_validation: false)
end
it 'is not valid', :aggregate_failures do
expect(subject).not_to be_valid
expect(subject.errors.full_messages).to include(message)
end
end
context 'runner validation' do
it 'is is valid', :aggregate_failures do
expect(subject).to be_valid
expect(subject.errors.full_messages).not_to include(message)
end
it 'is is valid', :aggregate_failures do
expect(subject).to be_valid
expect(subject.errors.full_messages).not_to include(message)
end
end
end
......
ee/spec/models/dast_site_validation_spec.rb
View file @ e1d3bc04
......@@ -17,37 +17,6 @@ RSpec.describe DastSiteValidation, type: :model do
describe 'validations' do
it { is_expected.to be_valid }
it { is_expected.to validate_presence_of(:dast_site_token_id) }
context 'when strategy is meta_tag' do
subject { build(:dast_site_validation, dast_site_token: dast_site_token, validation_strategy: :meta_tag) }
shared_examples 'meta tag validation is disabled' do
it 'is not valid', :aggregate_failures do
expect(subject).not_to be_valid
expect(subject.errors.full_messages).to include('Meta tag validation is not enabled')
end
end
context 'when dast_meta_tag_validation and dast_runner_site_validation are enabled' do
it { is_expected.to be_valid }
end
context 'when dast_meta_tag_validation is disabled' do
before do
stub_feature_flags(dast_meta_tag_validation: false)
end
it_behaves_like 'meta tag validation is disabled'
end
context 'when dast_runner_site_validation is disabled' do
before do
stub_feature_flags(dast_runner_site_validation: false)
end
it_behaves_like 'meta tag validation is disabled'
end
end
end
describe 'before_create' do
......
ee/spec/requests/api/internal/app_sec/dast/site_validations_spec.rb
View file @ e1d3bc04
......@@ -101,19 +101,6 @@ RSpec.describe API::Internal::AppSec::Dast::SiteValidations do
end
end
context 'when the feature flag is disabled' do
before do
stub_feature_flags(dast_runner_site_validation: false)
end
it 'returns 404 and a contextual error message', :aggregate_failures do
subject
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response).to eq('message' => '404 Feature flag disabled: :dast_runner_site_validation')
end
end
context 'when user has access to the site validation' do
context 'when the state transition is unknown' do
let(:event_param) { :unknown_transition }
......
ee/spec/services/app_sec/dast/site_validations/runner_service_spec.rb
View file @ e1d3bc04
......@@ -75,14 +75,6 @@ RSpec.describe AppSec::Dast::SiteValidations::RunnerService do
expect { subject }.to change { dast_site_validation.state }.from('pending').to('failed')
end
end
context 'when feature flag is disabled' do
before do
stub_feature_flags(dast_runner_site_validation: false)
end
it_behaves_like 'a failure'
end
end
end
end
ee/spec/services/dast_site_validations/create_service_spec.rb
View file @ e1d3bc04
......@@ -87,80 +87,23 @@ RSpec.describe DastSiteValidations::CreateService do
end
describe 'execute', :clean_gitlab_redis_shared_state do
context 'worker validation' do
before do
stub_feature_flags(dast_runner_site_validation: false)
end
it_behaves_like 'the licensed feature is not available'
it_behaves_like 'the licensed feature is available' do
it 'attempts to validate' do
expect(DastSiteValidationWorker).to receive(:perform_async)
subject
end
context 'when worker does not return a job id' do
before do
allow(DastSiteValidationWorker).to receive(:perform_async).and_return(nil)
end
let(:dast_site_validation) do
DastSiteValidation.find_by!(dast_site_token: dast_site_token, url_path: params[:url_path])
end
it 'communicates failure' do
aggregate_failures do
expect(subject.status).to eq(:error)
expect(subject.message).to eq('Validation failed')
end
end
it 'sets dast_site_validation.state to failed' do
subject
expect(dast_site_validation.state).to eq('failed')
end
it_behaves_like 'the licensed feature is not available'
it 'logs an error' do
allow(Gitlab::AppLogger).to receive(:error)
it_behaves_like 'the licensed feature is available' do
it 'attempts to validate' do
expected_args = { project: project, current_user: developer, params: { dast_site_validation: instance_of(DastSiteValidation) } }
subject
expect(AppSec::Dast::SiteValidations::RunnerService).to receive(:new).with(expected_args).and_call_original
expect(Gitlab::AppLogger).to have_received(:error).with(message: 'Unable to validate dast_site_validation', dast_site_validation_id: dast_site_validation.id)
end
end
it_behaves_like 'a dast_site_validation already exists' do
it 'does not attempt to re-validate' do
expect(DastSiteValidationWorker).not_to receive(:perform_async)
subject
end
end
subject
end
end
context 'runner validation' do
it_behaves_like 'the licensed feature is not available'
it_behaves_like 'the licensed feature is available' do
it 'attempts to validate' do
expected_args = { project: project, current_user: developer, params: { dast_site_validation: instance_of(DastSiteValidation) } }
expect(AppSec::Dast::SiteValidations::RunnerService).to receive(:new).with(expected_args).and_call_original
it_behaves_like 'a dast_site_validation already exists' do
it 'does not attempt to re-validate' do
expect(AppSec::Dast::SiteValidations::RunnerService).not_to receive(:new)
subject
end
it_behaves_like 'a dast_site_validation already exists' do
it 'does not attempt to re-validate' do
expect(AppSec::Dast::SiteValidations::RunnerService).not_to receive(:new)
subject
end
end
end
end
end
......
ee/spec/services/dast_site_validations/validate_service_spec.rb deleted 100644 → 0
View file @ 31462b14
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe DastSiteValidations::ValidateService do
let(:dast_site_validation) { create(:dast_site_validation) }
let(:token) { dast_site_validation.dast_site_token.token }
let(:headers) { { 'Content-Type' => 'text/plain; charset=utf-8' } }
subject do
described_class.new(
container: dast_site_validation.project,
params: { dast_site_validation: dast_site_validation }
).execute!
end
describe 'execute!' do
context 'when on demand scan licensed feature is not available' do
it 'communicates failure' do
stub_licensed_features(security_on_demand_scans: false)
expect { subject }.to raise_error(DastSiteValidations::ValidateService::PermissionsError)
end
end
context 'when the feature is available' do
before do
stub_licensed_features(security_on_demand_scans: true)
stub_request(:get, dast_site_validation.validation_url).to_return(body: token, headers: headers)
end
it 'validates the url before making an http request' do
uri = URI(dast_site_validation.validation_url)
opt = hash_including(allow_local_network: false, allow_localhost: false, dns_rebind_protection: true)
aggregate_failures do
expect(Gitlab::UrlBlocker).to receive(:validate!).with(uri, opt).and_call_original
expect(Gitlab::HTTP).to receive(:get).with(dast_site_validation.validation_url, use_read_total_timeout: true).and_call_original
end
subject
end
context 'when validation has already been attempted' do
let_it_be(:dast_site_validation) { create(:dast_site_validation, state: :failed) }
it 'marks the validation as a retry' do
freeze_time do
subject
expect(dast_site_validation.reload.validation_last_retried_at).to eq(Time.now.utc)
end
end
end
shared_examples 'a validation' do
context 'when the token is found' do
it 'calls dast_site_validation#start' do
expect(dast_site_validation).to receive(:start)
subject
end
it 'calls dast_site_validation#pass' do
expect(dast_site_validation).to receive(:pass)
subject
end
it 'marks the validation successful' do
subject
expect(dast_site_validation.reload.state).to eq('passed')
end
context 'when validation has already started' do
before do
dast_site_validation.update_column(:state, :inprogress)
end
it 'does not call dast_site_validation#pass' do
expect(dast_site_validation).not_to receive(:start)
subject
end
end
context 'when validation is already complete' do
before do
dast_site_validation.update_column(:state, :passed)
end
it 'does not re-validate' do
expect(Gitlab::HTTP).not_to receive(:get)
subject
end
end
end
context 'when the token is not found' do
let(:token) do
'<div>' + dast_site_validation.dast_site_token.token + '</div>'
end
it 'raises an exception' do
expect { subject }.to raise_error(DastSiteValidations::ValidateService::TokenNotFound)
end
end
end
context 'when validation_strategy=text_file' do
let(:dast_site_validation) { create(:dast_site_validation, validation_strategy: :text_file) }
before do
stub_request(:get, dast_site_validation.validation_url).to_return(body: token, headers: headers)
end
it_behaves_like 'a validation'
context 'when the http response body has trailing newlines after the token' do
let(:token) { dast_site_validation.dast_site_token.token + "\n\n" }
it 'does not raise an exception' do
expect { subject }.not_to raise_error(DastSiteValidations::ValidateService::TokenNotFound)
end
end
context 'when content type is incorrect' do
let(:headers) { { 'Content-Type' => 'text/html; charset=UTF-8' } }
it 'raises an exception' do
expect { subject }.to raise_error(DastSiteValidations::ValidateService::TokenNotFound)
end
end
end
context 'when validation_strategy=header' do
let(:dast_site_validation) { create(:dast_site_validation, validation_strategy: :header) }
before do
headers = { DastSiteValidation::HEADER => token }
stub_request(:get, dast_site_validation.validation_url).to_return(headers: headers)
end
it_behaves_like 'a validation'
end
end
end
end
ee/spec/workers/dast_site_validation_worker_spec.rb
View file @ e1d3bc04
......@@ -10,56 +10,9 @@ RSpec.describe DastSiteValidationWorker do
end
describe '#perform' do
it 'calls DastSiteValidations::ValidateService' do
double = double(DastSiteValidations::ValidateService, "execute!" => true)
args = { container: dast_site_validation.project, params: { dast_site_validation: dast_site_validation } }
expect(double).to receive(:execute!)
expect(DastSiteValidations::ValidateService).to receive(:new).with(args).and_return(double)
subject
end
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_request(:get, dast_site_validation.validation_url).to_return(body: response_body, headers: headers)
end
context 'when the request body contains the token' do
include_examples 'an idempotent worker' do
subject do
perform_multiple([dast_site_validation.id], worker: described_class.new)
end
end
end
end
end
describe '.sidekiq_retries_exhausted' do
it 'calls find with the correct id' do
job = { 'args' => [dast_site_validation.id], 'jid' => '1' }
expect(dast_site_validation.class).to receive(:find).with(dast_site_validation.id).and_call_original
described_class.sidekiq_retries_exhausted_block.call(job)
end
it 'marks validation failed' do
job = { 'args' => [dast_site_validation.id], 'jid' => '1' }
freeze_time do
described_class.sidekiq_retries_exhausted_block.call(job)
aggregate_failures do
obj = dast_site_validation.reload
expect(obj.state).to eq('failed')
expect(obj.validation_failed_at).to eq(Time.now.utc)
end
include_examples 'an idempotent worker' do
subject do
perform_multiple([dast_site_validation.id], worker: described_class.new)
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7