Skip to content
Snippets Groups Projects
Commit a844a63a authored by Luke Duncalfe's avatar Luke Duncalfe
Browse files

Merge branch...

Merge branch 'philipcunningham-add-excluded-urls-and-request-headers-to-dast-profiles-277353' into 'master'

Extend DastSiteProfile with more config options

See merge request gitlab-org/gitlab!55579
parents babc2fe7 ddea1b91
No related branches found
No related tags found
No related merge requests found
Showing
with 394 additions and 31 deletions
---
title: Add additional fields to dast_site_profiles database table
merge_request: 55579
author:
type: added
# frozen_string_literal: true
class AddExcludedUrlsAndRequestHeadersToDastSiteProfiles < ActiveRecord::Migration[6.0]
DOWNTIME = false
# rubocop:disable Migration/AddLimitToTextColumns
# limit is added in 20210311022012_add_text_limits_to_dast_site_profiles
def change
add_column :dast_site_profiles, :excluded_urls, :text, array: true, default: [], null: false
add_column :dast_site_profiles, :auth_enabled, :boolean, default: false, null: false
add_column :dast_site_profiles, :auth_url, :text
add_column :dast_site_profiles, :auth_username_field, :text
add_column :dast_site_profiles, :auth_password_field, :text
add_column :dast_site_profiles, :auth_username, :text
end
# rubocop:enable Migration/AddLimitToTextColumns
end
# frozen_string_literal: true
class AddTextLimitsToDastSiteProfiles < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
def up
add_text_limit :dast_site_profiles, :auth_url, 1024
add_text_limit :dast_site_profiles, :auth_username_field, 255
add_text_limit :dast_site_profiles, :auth_password_field, 255
add_text_limit :dast_site_profiles, :auth_username, 255
end
def down
remove_text_limit :dast_site_profiles, :auth_username
remove_text_limit :dast_site_profiles, :auth_password_field
remove_text_limit :dast_site_profiles, :auth_username_field
remove_text_limit :dast_site_profiles, :auth_url
end
end
bf47b1c4840c97459f99308d9de04644d18c301659ef5f021088911155d2c624
\ No newline at end of file
77d023cc7b635f5b3fc4d8c963183ca15e90f6bb747c145bd8efd1a4e47f65a0
\ No newline at end of file
Loading
Loading
@@ -11840,7 +11840,17 @@ CREATE TABLE dast_site_profiles (
created_at timestamp with time zone NOT NULL,
updated_at timestamp with time zone NOT NULL,
name text NOT NULL,
CONSTRAINT check_6cfab17b48 CHECK ((char_length(name) <= 255))
excluded_urls text[] DEFAULT '{}'::text[] NOT NULL,
auth_enabled boolean DEFAULT false NOT NULL,
auth_url text,
auth_username_field text,
auth_password_field text,
auth_username text,
CONSTRAINT check_5203110fee CHECK ((char_length(auth_username_field) <= 255)),
CONSTRAINT check_6cfab17b48 CHECK ((char_length(name) <= 255)),
CONSTRAINT check_c329dffdba CHECK ((char_length(auth_password_field) <= 255)),
CONSTRAINT check_d446f7047b CHECK ((char_length(auth_url) <= 1024)),
CONSTRAINT check_f22f18002a CHECK ((char_length(auth_username) <= 255))
);
 
CREATE SEQUENCE dast_site_profiles_id_seq
Loading
Loading
@@ -1989,15 +1989,31 @@ Represents a DAST Site Profile.
 
| Field | Type | Description |
| ----- | ---- | ----------- |
| `auth` | [`DastSiteProfileAuth`](#dastsiteprofileauth) | Target authentication details. Will always return `null` if `security_dast_site_profiles_additional_fields` feature flag is disabled. |
| `editPath` | [`String`](#string) | Relative web path to the edit page of a site profile. |
| `excludedUrls` | [`[String!]`](#string) | The URLs to skip during an authenticated scan. Will always return `null` if `security_dast_site_profiles_additional_fields` feature flag is disabled. |
| `id` | [`DastSiteProfileID!`](#dastsiteprofileid) | ID of the site profile. |
| `normalizedTargetUrl` | [`String`](#string) | Normalized URL of the target to be scanned. |
| `profileName` | [`String`](#string) | The name of the site profile. |
| `referencedInSecurityPolicies` | [`[String!]`](#string) | List of security policy names that are referencing given project. |
| `requestHeaders` | [`String`](#string) | Comma-separated list of request header names and values to be added to every request made by DAST. Will always return `null` if `security_dast_site_profiles_additional_fields` feature flag is disabled. |
| `targetUrl` | [`String`](#string) | The URL of the target to be scanned. |
| `userPermissions` | [`DastSiteProfilePermissions!`](#dastsiteprofilepermissions) | Permissions for the current user on the resource |
| `validationStatus` | [`DastSiteProfileValidationStatusEnum`](#dastsiteprofilevalidationstatusenum) | The current validation status of the site profile. |
 
### `DastSiteProfileAuth`
Input type for DastSiteProfile authentication.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `enabled` | [`Boolean`](#boolean) | Indicates whether authentication is enabled. |
| `password` | [`String`](#string) | Redacted password to authenticate with on the target website. |
| `passwordField` | [`String`](#string) | The name of password field at the sign-in HTML form. |
| `url` | [`String`](#string) | The URL of the page containing the sign-in HTML form on the target website. |
| `username` | [`String`](#string) | The username to authenticate with on the target website. |
| `usernameField` | [`String`](#string) | The name of username field at the sign-in HTML form. |
### `DastSiteProfileConnection`
 
The connection type for DastSiteProfile.
Loading
Loading
Loading
Loading
@@ -23,13 +23,34 @@ class Create < BaseMutation
required: false,
description: 'The URL of the target to be scanned.'
 
argument :excluded_urls, [GraphQL::STRING_TYPE],
required: false,
default_value: [],
description: 'The URLs to skip during an authenticated scan. Defaults to `[]`. Will be ignored ' \
'if `security_dast_site_profiles_additional_fields` feature flag is disabled.'
argument :request_headers, GraphQL::STRING_TYPE,
required: false,
description: 'Comma-separated list of request header names and values to be ' \
'added to every request made by DAST. Will be ignored ' \
'if `security_dast_site_profiles_additional_fields` feature flag is disabled.'
argument :auth, ::Types::Dast::SiteProfileAuthInputType,
required: false,
description: 'Parameters for authentication. Will be ignored ' \
'if `security_dast_site_profiles_additional_fields` feature flag is disabled.'
authorize :create_on_demand_dast_scan
 
def resolve(full_path:, profile_name:, target_url: nil)
def resolve(full_path:, profile_name:, target_url: nil, excluded_urls: [], request_headers: nil, auth: nil)
project = authorized_find!(full_path)
 
service = ::DastSiteProfiles::CreateService.new(project, current_user)
result = service.execute(name: profile_name, target_url: target_url)
result = service.execute(
name: profile_name,
target_url: target_url,
excluded_urls: feature_flagged_excluded_urls(project, excluded_urls)
)
 
if result.success?
{ id: result.payload.to_global_id, errors: [] }
Loading
Loading
@@ -37,6 +58,14 @@ def resolve(full_path:, profile_name:, target_url: nil)
{ errors: result.errors }
end
end
private
def feature_flagged_excluded_urls(project, excluded_urls)
return [] unless Feature.enabled?(:security_dast_site_profiles_additional_fields, project, default_enabled: :yaml)
excluded_urls
end
end
end
end
Loading
Loading
@@ -7,7 +7,9 @@ class Update < BaseMutation
 
graphql_name 'DastSiteProfileUpdate'
 
field :id, ::Types::GlobalIDType[::DastSiteProfile],
SiteProfileID = ::Types::GlobalIDType[::DastSiteProfile]
field :id, SiteProfileID,
null: true,
description: 'ID of the site profile.'
 
Loading
Loading
@@ -15,7 +17,7 @@ class Update < BaseMutation
required: true,
description: 'The project the site profile belongs to.'
 
argument :id, ::Types::GlobalIDType[::DastSiteProfile],
argument :id, SiteProfileID,
required: true,
description: 'ID of the site profile to be updated.'
 
Loading
Loading
@@ -27,16 +29,37 @@ class Update < BaseMutation
required: false,
description: 'The URL of the target to be scanned.'
 
argument :excluded_urls, [GraphQL::STRING_TYPE],
required: false,
description: 'The URLs to skip during an authenticated scan. Will be ignored ' \
'if `security_dast_site_profiles_additional_fields` feature flag is disabled.'
argument :request_headers, GraphQL::STRING_TYPE,
required: false,
description: 'Comma-separated list of request header names and values to be ' \
'added to every request made by DAST. Will be ignored ' \
'if `security_dast_site_profiles_additional_fields` feature flag is disabled.'
argument :auth, ::Types::Dast::SiteProfileAuthInputType,
required: false,
description: 'Parameters for authentication. Will be ignored ' \
'if `security_dast_site_profiles_additional_fields` feature flag is disabled.'
authorize :create_on_demand_dast_scan
 
def resolve(full_path:, id:, **service_args)
def resolve(full_path:, id:, profile_name:, target_url: nil, excluded_urls: nil, request_headers: nil, auth: nil)
project = authorized_find!(full_path)
# TODO: remove explicit coercion once compatibility layer has been removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
service_args[:id] = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(id).model_id
project = authorized_find!(full_path)
params = {
id: SiteProfileID.coerce_isolated_input(id).model_id,
name: profile_name,
target_url: target_url,
excluded_urls: feature_flagged_excluded_urls(project, excluded_urls)
}.compact
 
service = ::DastSiteProfiles::UpdateService.new(project, current_user)
result = service.execute(**service_args)
result = ::DastSiteProfiles::UpdateService.new(project, current_user).execute(**params)
 
if result.success?
{ id: result.payload.to_global_id, errors: [] }
Loading
Loading
@@ -44,6 +67,14 @@ def resolve(full_path:, id:, **service_args)
{ errors: result.errors }
end
end
private
def feature_flagged_excluded_urls(project, excluded_urls)
return unless Feature.enabled?(:security_dast_site_profiles_additional_fields, project, default_enabled: :yaml)
excluded_urls
end
end
end
end
# frozen_string_literal: true
module Types
module Dast
class SiteProfileAuthInputType < BaseInputObject
graphql_name 'DastSiteProfileAuthInput'
description 'Input type for DastSiteProfile authentication'
argument :enabled, GraphQL::BOOLEAN_TYPE,
required: false,
description: 'Indicates whether authentication is enabled.'
argument :url, GraphQL::STRING_TYPE,
required: false,
description: 'The URL of the page containing the sign-in HTML ' \
'form on the target website.'
argument :username_field, GraphQL::STRING_TYPE,
required: false,
description: 'The name of username field at the sign-in HTML form.'
argument :password_field, GraphQL::STRING_TYPE,
required: false,
description: 'The name of password field at the sign-in HTML form.'
argument :username, GraphQL::STRING_TYPE,
required: false,
description: 'The username to authenticate with on the target website.'
argument :password, GraphQL::STRING_TYPE,
required: false,
description: 'The password to authenticate with on the target website.'
end
end
end
# frozen_string_literal: true
module Types
module Dast
class SiteProfileAuthType < BaseObject
graphql_name 'DastSiteProfileAuth'
description 'Input type for DastSiteProfile authentication'
authorize :read_on_demand_scans
field :enabled, GraphQL::BOOLEAN_TYPE,
null: true,
method: :auth_enabled,
description: 'Indicates whether authentication is enabled.'
field :url, GraphQL::STRING_TYPE,
null: true,
method: :auth_url,
description: 'The URL of the page containing the sign-in HTML ' \
'form on the target website.'
field :username_field, GraphQL::STRING_TYPE,
null: true,
method: :auth_username_field,
description: 'The name of username field at the sign-in HTML form.'
field :password_field, GraphQL::STRING_TYPE,
null: true,
method: :auth_password_field,
description: 'The name of password field at the sign-in HTML form.'
field :username, GraphQL::STRING_TYPE,
null: true,
method: :auth_username,
description: 'The username to authenticate with on the target website.'
field :password, GraphQL::STRING_TYPE,
null: true,
description: 'Redacted password to authenticate with on the target website.'
def password
nil
end
end
end
end
Loading
Loading
@@ -22,6 +22,19 @@ class DastSiteProfileType < BaseObject
field :edit_path, GraphQL::STRING_TYPE, null: true,
description: 'Relative web path to the edit page of a site profile.'
 
field :auth, Types::Dast::SiteProfileAuthType, null: true,
description: 'Target authentication details. Will always return `null` ' \
'if `security_dast_site_profiles_additional_fields` feature flag is disabled.'
field :excluded_urls, [GraphQL::STRING_TYPE], null: true,
description: 'The URLs to skip during an authenticated scan. Will always return `null` ' \
'if `security_dast_site_profiles_additional_fields` feature flag is disabled.'
field :request_headers, GraphQL::STRING_TYPE, null: true,
description: 'Comma-separated list of request header names and values to be ' \
'added to every request made by DAST. Will always return `null` ' \
'if `security_dast_site_profiles_additional_fields` feature flag is disabled.'
field :validation_status, Types::DastSiteProfileValidationStatusEnum, null: true,
description: 'The current validation status of the site profile.',
method: :status
Loading
Loading
@@ -42,6 +55,22 @@ def edit_path
Rails.application.routes.url_helpers.edit_project_security_configuration_dast_profiles_dast_site_profile_path(object.project, object)
end
 
def auth
return unless Feature.enabled?(:security_dast_site_profiles_additional_fields, object.project, default_enabled: :yaml)
object
end
def excluded_urls
return unless Feature.enabled?(:security_dast_site_profiles_additional_fields, object.project, default_enabled: :yaml)
object.excluded_urls
end
def request_headers
nil
end
def normalized_target_url
DastSiteValidation.get_normalized_url_base(object.dast_site.url)
end
Loading
Loading
Loading
Loading
@@ -6,9 +6,15 @@ class DastSiteProfile < ApplicationRecord
 
has_many :secret_variables, class_name: 'Dast::SiteProfileSecretVariable'
 
validates :excluded_urls, length: { maximum: 25 }
validates :auth_url, addressable_url: true, length: { maximum: 1024 }, allow_nil: true
validates :auth_username_field, :auth_password_field, :auth_username, length: { maximum: 255 }
validates :name, length: { maximum: 255 }, uniqueness: { scope: :project_id }, presence: true
validates :project_id, :dast_site_id, presence: true
validate :dast_site_project_id_fk
validate :excluded_urls_contains_valid_urls
validate :excluded_urls_contains_valid_strings
 
scope :with_dast_site_and_validation, -> { includes(dast_site: :dast_site_validation) }
scope :with_name, -> (name) { where(name: name) }
Loading
Loading
@@ -40,4 +46,26 @@ def dast_site_project_id_fk
errors.add(:project_id, 'does not match dast_site.project')
end
end
def excluded_urls_contains_valid_urls
validate_excluded_urls_with("contains invalid URLs (%{urls})") do |excluded_url|
!Gitlab::UrlSanitizer.valid?(excluded_url)
end
end
def excluded_urls_contains_valid_strings
validate_excluded_urls_with("contains URLs that exceed the 1024 character limit (%{urls})") do |excluded_url|
excluded_url.length > 1024
end
end
def validate_excluded_urls_with(message, &block)
return if excluded_urls.blank?
invalid = excluded_urls.select(&block)
return if invalid.empty?
errors.add(:excluded_urls, message % { urls: invalid.join(', ') })
end
end
Loading
Loading
@@ -2,14 +2,20 @@
 
module DastSiteProfiles
class CreateService < BaseService
def execute(name:, target_url:)
def execute(name:, target_url:, excluded_urls: [])
return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
 
ActiveRecord::Base.transaction do
service = DastSites::FindOrCreateService.new(project, current_user)
dast_site = service.execute!(url: target_url)
 
dast_site_profile = DastSiteProfile.create!(project: project, dast_site: dast_site, name: name)
dast_site_profile = DastSiteProfile.create!(
project: project,
dast_site: dast_site,
name: name,
excluded_urls: excluded_urls || []
)
ServiceResponse.success(payload: dast_site_profile)
end
 
Loading
Loading
Loading
Loading
@@ -2,7 +2,7 @@
 
module DastSiteProfiles
class UpdateService < BaseService
def execute(id:, profile_name:, target_url:)
def execute(id:, **params)
return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
 
dast_site_profile = find_dast_site_profile!(id)
Loading
Loading
@@ -10,10 +10,11 @@ def execute(id:, profile_name:, target_url:)
return ServiceResponse.error(message: "Cannot modify #{dast_site_profile.name} referenced in security policy") if referenced_in_security_policy?(dast_site_profile)
 
ActiveRecord::Base.transaction do
service = DastSites::FindOrCreateService.new(project, current_user)
dast_site = service.execute!(url: target_url)
if target_url = params.delete(:target_url)
params[:dast_site] = DastSites::FindOrCreateService.new(project, current_user).execute!(url: target_url)
end
 
dast_site_profile.update!(name: profile_name, dast_site: dast_site)
dast_site_profile.update!(params)
 
ServiceResponse.success(payload: dast_site_profile)
end
Loading
Loading
Loading
Loading
@@ -10,6 +10,14 @@
"#{FFaker::Product.product_name.truncate(200)} - #{i}"
end
 
auth_enabled { false }
auth_url { "#{dast_site.url}/sign-in" }
auth_username_field { 'session[username]' }
auth_password_field { 'session[password]' }
auth_username { generate(:email) }
excluded_urls { ["#{dast_site.url}/sign-out", "#{dast_site.url}/hidden"] }
trait :with_dast_site_validation do
dast_site { association :dast_site, :with_dast_site_validation, project: project }
end
Loading
Loading
Loading
Loading
@@ -3,12 +3,15 @@
require 'spec_helper'
 
RSpec.describe Mutations::DastSiteProfiles::Create do
let(:group) { create(:group) }
let(:project) { create(:project, group: group) }
let(:user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, group: group) }
let_it_be(:user) { create(:user) }
let(:full_path) { project.full_path }
let(:profile_name) { SecureRandom.hex }
let(:target_url) { generate(:url) }
let(:excluded_urls) { ["#{target_url}/signout"] }
let(:dast_site_profile) { DastSiteProfile.find_by(project: project, name: profile_name) }
 
subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
Loading
Loading
@@ -24,7 +27,17 @@
mutation.resolve(
full_path: full_path,
profile_name: profile_name,
target_url: target_url
target_url: target_url,
excluded_urls: excluded_urls,
request_headers: 'Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0',
auth: {
enabled: true,
url: "#{target_url}/login",
username_field: 'session[username]',
password_field: 'session[password]',
username: generate(:email),
password: SecureRandom.hex
}
)
end
 
Loading
Loading
@@ -50,8 +63,10 @@
service = double(described_class)
result = double('result', success?: false, errors: [])
 
service_params = { name: profile_name, target_url: target_url, excluded_urls: excluded_urls }
expect(DastSiteProfiles::CreateService).to receive(:new).and_return(service)
expect(service).to receive(:execute).with(name: profile_name, target_url: target_url).and_return(result)
expect(service).to receive(:execute).with(service_params).and_return(result)
 
subject
end
Loading
Loading
@@ -69,6 +84,26 @@
expect(response[:errors]).to include('Name has already been taken')
end
end
context 'when excluded_urls is supplied as a param' do
context 'when the feature flag security_dast_site_profiles_additional_fields is disabled' do
it 'does not set the excluded_urls' do
stub_feature_flags(security_dast_site_profiles_additional_fields: false)
subject
expect(dast_site_profile.excluded_urls).to be_empty
end
end
context 'when the feature flag security_dast_site_profiles_additional_fields is enabled' do
it 'sets the excluded_urls' do
subject
expect(dast_site_profile.excluded_urls).to eq(excluded_urls)
end
end
end
end
end
end
Loading
Loading
Loading
Loading
@@ -3,14 +3,15 @@
require 'spec_helper'
 
RSpec.describe Mutations::DastSiteProfiles::Update do
let(:group) { create(:group) }
let(:project) { create(:project, group: group) }
let(:user) { create(:user) }
let(:full_path) { project.full_path }
let!(:dast_site_profile) { create(:dast_site_profile, project: project) }
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, group: group) }
let_it_be(:user) { create(:user) }
let_it_be(:dast_site_profile) { create(:dast_site_profile, project: project) }
 
let(:full_path) { project.full_path }
let(:new_profile_name) { SecureRandom.hex }
let(:new_target_url) { generate(:url) }
let(:new_excluded_urls) { ["#{new_target_url}/signout"] }
 
subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
 
Loading
Loading
@@ -26,7 +27,17 @@
full_path: full_path,
id: dast_site_profile.to_global_id,
profile_name: new_profile_name,
target_url: new_target_url
target_url: new_target_url,
excluded_urls: new_excluded_urls,
request_headers: 'Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0',
auth: {
enabled: true,
url: "#{new_target_url}/login",
username_field: 'session[username]',
password_field: 'session[password]',
username: generate(:email),
password: SecureRandom.hex
}
)
end
 
Loading
Loading
@@ -44,12 +55,21 @@
project.add_developer(user)
end
 
it 'updates the dast_site_profile' do
it 'updates the dast_site_profile', :aggregate_failures do
dast_site_profile = subject[:id].find
 
aggregate_failures do
expect(dast_site_profile.name).to eq(new_profile_name)
expect(dast_site_profile.dast_site.url).to eq(new_target_url)
expect(dast_site_profile.name).to eq(new_profile_name)
expect(dast_site_profile.dast_site.url).to eq(new_target_url)
expect(dast_site_profile.reload.excluded_urls).to eq(new_excluded_urls)
end
context 'when the feature flag security_dast_site_profiles_additional_fields is disabled' do
it 'does not set the branch_name' do
stub_feature_flags(security_dast_site_profiles_additional_fields: false)
dast_site_profile = subject[:id].find
expect(dast_site_profile.reload.excluded_urls).not_to eq(new_excluded_urls)
end
end
end
Loading
Loading
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Types::Dast::SiteProfileAuthInputType do
specify { expect(described_class.graphql_name).to eq('DastSiteProfileAuthInput') }
it 'has the correct arguments' do
expect(described_class.arguments.keys).to match_array(%w[enabled url usernameField passwordField username password])
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Types::Dast::SiteProfileAuthType do
specify { expect(described_class.graphql_name).to eq('DastSiteProfileAuth') }
specify { expect(described_class).to require_graphql_authorizations(:read_on_demand_scans) }
it { expect(described_class).to have_graphql_fields(%w[enabled url usernameField passwordField username password]) }
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment