Commit ddea1b91 authored by Philip Cunningham's avatar Philip Cunningham Committed by Luke Duncalfe

Extend DastSiteProfile with more config options

- Adds new db columns
- Extends create/update with additional fields
- Stubs out fields not currently in use
- Amends services to correctly handle nil values
parent babc2fe7
---
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
...@@ -11840,7 +11840,17 @@ CREATE TABLE dast_site_profiles ( ...@@ -11840,7 +11840,17 @@ CREATE TABLE dast_site_profiles (
created_at timestamp with time zone NOT NULL, created_at timestamp with time zone NOT NULL,
updated_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL,
name text 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 CREATE SEQUENCE dast_site_profiles_id_seq
...@@ -1989,15 +1989,31 @@ Represents a DAST Site Profile. ...@@ -1989,15 +1989,31 @@ Represents a DAST Site Profile.
| Field | Type | Description | | 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. | | `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. | | `id` | [`DastSiteProfileID!`](#dastsiteprofileid) | ID of the site profile. |
| `normalizedTargetUrl` | [`String`](#string) | Normalized URL of the target to be scanned. | | `normalizedTargetUrl` | [`String`](#string) | Normalized URL of the target to be scanned. |
| `profileName` | [`String`](#string) | The name of the site profile. | | `profileName` | [`String`](#string) | The name of the site profile. |
| `referencedInSecurityPolicies` | [`[String!]`](#string) | List of security policy names that are referencing given project. | | `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. | | `targetUrl` | [`String`](#string) | The URL of the target to be scanned. |
| `userPermissions` | [`DastSiteProfilePermissions!`](#dastsiteprofilepermissions) | Permissions for the current user on the resource | | `userPermissions` | [`DastSiteProfilePermissions!`](#dastsiteprofilepermissions) | Permissions for the current user on the resource |
| `validationStatus` | [`DastSiteProfileValidationStatusEnum`](#dastsiteprofilevalidationstatusenum) | The current validation status of the site profile. | | `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` ### `DastSiteProfileConnection`
The connection type for DastSiteProfile. The connection type for DastSiteProfile.
......
...@@ -23,13 +23,34 @@ module Mutations ...@@ -23,13 +23,34 @@ module Mutations
required: false, required: false,
description: 'The URL of the target to be scanned.' 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 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) project = authorized_find!(full_path)
service = ::DastSiteProfiles::CreateService.new(project, current_user) 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? if result.success?
{ id: result.payload.to_global_id, errors: [] } { id: result.payload.to_global_id, errors: [] }
...@@ -37,6 +58,14 @@ module Mutations ...@@ -37,6 +58,14 @@ module Mutations
{ errors: result.errors } { errors: result.errors }
end end
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 end
end end
...@@ -7,7 +7,9 @@ module Mutations ...@@ -7,7 +7,9 @@ module Mutations
graphql_name 'DastSiteProfileUpdate' graphql_name 'DastSiteProfileUpdate'
field :id, ::Types::GlobalIDType[::DastSiteProfile], SiteProfileID = ::Types::GlobalIDType[::DastSiteProfile]
field :id, SiteProfileID,
null: true, null: true,
description: 'ID of the site profile.' description: 'ID of the site profile.'
...@@ -15,7 +17,7 @@ module Mutations ...@@ -15,7 +17,7 @@ module Mutations
required: true, required: true,
description: 'The project the site profile belongs to.' description: 'The project the site profile belongs to.'
argument :id, ::Types::GlobalIDType[::DastSiteProfile], argument :id, SiteProfileID,
required: true, required: true,
description: 'ID of the site profile to be updated.' description: 'ID of the site profile to be updated.'
...@@ -27,16 +29,37 @@ module Mutations ...@@ -27,16 +29,37 @@ module Mutations
required: false, required: false,
description: 'The URL of the target to be scanned.' 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 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 # TODO: remove explicit coercion once compatibility layer has been removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883 # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
service_args[:id] = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(id).model_id params = {
project = authorized_find!(full_path) 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 = ::DastSiteProfiles::UpdateService.new(project, current_user).execute(**params)
result = service.execute(**service_args)
if result.success? if result.success?
{ id: result.payload.to_global_id, errors: [] } { id: result.payload.to_global_id, errors: [] }
...@@ -44,6 +67,14 @@ module Mutations ...@@ -44,6 +67,14 @@ module Mutations
{ errors: result.errors } { errors: result.errors }
end end
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 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
...@@ -22,6 +22,19 @@ module Types ...@@ -22,6 +22,19 @@ module Types
field :edit_path, GraphQL::STRING_TYPE, null: true, field :edit_path, GraphQL::STRING_TYPE, null: true,
description: 'Relative web path to the edit page of a site profile.' 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, field :validation_status, Types::DastSiteProfileValidationStatusEnum, null: true,
description: 'The current validation status of the site profile.', description: 'The current validation status of the site profile.',
method: :status method: :status
...@@ -42,6 +55,22 @@ module Types ...@@ -42,6 +55,22 @@ module Types
Rails.application.routes.url_helpers.edit_project_security_configuration_dast_profiles_dast_site_profile_path(object.project, object) Rails.application.routes.url_helpers.edit_project_security_configuration_dast_profiles_dast_site_profile_path(object.project, object)
end 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 def normalized_target_url
DastSiteValidation.get_normalized_url_base(object.dast_site.url) DastSiteValidation.get_normalized_url_base(object.dast_site.url)
end end
......
...@@ -6,9 +6,15 @@ class DastSiteProfile < ApplicationRecord ...@@ -6,9 +6,15 @@ class DastSiteProfile < ApplicationRecord
has_many :secret_variables, class_name: 'Dast::SiteProfileSecretVariable' 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 :name, length: { maximum: 255 }, uniqueness: { scope: :project_id }, presence: true
validates :project_id, :dast_site_id, presence: true validates :project_id, :dast_site_id, presence: true
validate :dast_site_project_id_fk 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_dast_site_and_validation, -> { includes(dast_site: :dast_site_validation) }
scope :with_name, -> (name) { where(name: name) } scope :with_name, -> (name) { where(name: name) }
...@@ -40,4 +46,26 @@ class DastSiteProfile < ApplicationRecord ...@@ -40,4 +46,26 @@ class DastSiteProfile < ApplicationRecord
errors.add(:project_id, 'does not match dast_site.project') errors.add(:project_id, 'does not match dast_site.project')
end end
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 end
...@@ -2,14 +2,20 @@ ...@@ -2,14 +2,20 @@
module DastSiteProfiles module DastSiteProfiles
class CreateService < BaseService class CreateService < BaseService
def execute(name:, target_url:) def execute(name:, target_url:, excluded_urls: [])
return ServiceResponse.error(message: 'Insufficient permissions') unless allowed? return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
ActiveRecord::Base.transaction do ActiveRecord::Base.transaction do
service = DastSites::FindOrCreateService.new(project, current_user) service = DastSites::FindOrCreateService.new(project, current_user)
dast_site = service.execute!(url: target_url) 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) ServiceResponse.success(payload: dast_site_profile)
end end
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
module DastSiteProfiles module DastSiteProfiles
class UpdateService < BaseService class UpdateService < BaseService
def execute(id:, profile_name:, target_url:) def execute(id:, **params)
return ServiceResponse.error(message: 'Insufficient permissions') unless allowed? return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
dast_site_profile = find_dast_site_profile!(id) dast_site_profile = find_dast_site_profile!(id)
...@@ -10,10 +10,11 @@ module DastSiteProfiles ...@@ -10,10 +10,11 @@ module DastSiteProfiles
return ServiceResponse.error(message: "Cannot modify #{dast_site_profile.name} referenced in security policy") if referenced_in_security_policy?(dast_site_profile) 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 ActiveRecord::Base.transaction do
service = DastSites::FindOrCreateService.new(project, current_user) if target_url = params.delete(:target_url)
dast_site = service.execute!(url: 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) ServiceResponse.success(payload: dast_site_profile)
end end
......
...@@ -10,6 +10,14 @@ FactoryBot.define do ...@@ -10,6 +10,14 @@ FactoryBot.define do
"#{FFaker::Product.product_name.truncate(200)} - #{i}" "#{FFaker::Product.product_name.truncate(200)} - #{i}"
end 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 trait :with_dast_site_validation do
dast_site { association :dast_site, :with_dast_site_validation, project: project } dast_site { association :dast_site, :with_dast_site_validation, project: project }
end end
......
...@@ -3,12 +3,15 @@ ...@@ -3,12 +3,15 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Mutations::DastSiteProfiles::Create do RSpec.describe Mutations::DastSiteProfiles::Create do
let(:group) { create(:group) } let_it_be(:group) { create(:group) }
let(:project) { create(:project, group: group) } let_it_be(:project) { create(:project, group: group) }
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:full_path) { project.full_path } let(:full_path) { project.full_path }
let(:profile_name) { SecureRandom.hex } let(:profile_name) { SecureRandom.hex }
let(:target_url) { generate(:url) } let(:target_url) { generate(:url) }
let(:excluded_urls) { ["#{target_url}/signout"] }
let(:dast_site_profile) { DastSiteProfile.find_by(project: project, name: profile_name) } 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) } subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
...@@ -24,7 +27,17 @@ RSpec.describe Mutations::DastSiteProfiles::Create do ...@@ -24,7 +27,17 @@ RSpec.describe Mutations::DastSiteProfiles::Create do
mutation.resolve( mutation.resolve(
full_path: full_path, full_path: full_path,
profile_name: profile_name, 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 end
...@@ -50,8 +63,10 @@ RSpec.describe Mutations::DastSiteProfiles::Create do ...@@ -50,8 +63,10 @@ RSpec.describe Mutations::DastSiteProfiles::Create do
service = double(described_class) service = double(described_class)
result = double('result', success?: false, errors: []) 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(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 subject
end end
...@@ -69,6 +84,26 @@ RSpec.describe Mutations::DastSiteProfiles::Create do ...@@ -69,6 +84,26 @@ RSpec.describe Mutations::DastSiteProfiles::Create do
expect(response[:errors]).to include('Name has already been taken') expect(response[:errors]).to include('Name has already been taken')
end end
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 end
end end
......
...@@ -3,14 +3,15 @@ ...@@ -3,14 +3,15 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Mutations::DastSiteProfiles::Update do RSpec.describe Mutations::DastSiteProfiles::Update do
let(:group) { create(:group) } let_it_be(:group) { create(:group) }
let(:project) { create(:project, group: group) } let_it_be(:project) { create(:project, group: group) }
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:full_path) { project.full_path } let_it_be(:dast_site_profile) { create(:dast_site_profile, project: project) }
let!(:dast_site_profile) { create(:dast_site_profile, project: project) }
let(:full_path) { project.full_path }
let(:new_profile_name) { SecureRandom.hex } let(:new_profile_name) { SecureRandom.hex }
let(:new_target_url) { generate(:url) } 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) } subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
...@@ -26,7 +27,17 @@ RSpec.describe Mutations::DastSiteProfiles::Update do ...@@ -26,7 +27,17 @@ RSpec.describe Mutations::DastSiteProfiles::Update do
full_path: full_path, full_path: full_path,
id: dast_site_profile.to_global_id, id: dast_site_profile.to_global_id,
profile_name: new_profile_name, 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 end
...@@ -44,12 +55,21 @@ RSpec.describe Mutations::DastSiteProfiles::Update do ...@@ -44,12 +55,21 @@ RSpec.describe Mutations::DastSiteProfiles::Update do
project.add_developer(user) project.add_developer(user)
end end
it 'updates the dast_site_profile' do it 'updates the dast_site_profile', :aggregate_failures do
dast_site_profile = subject[:id].find dast_site_profile = subject[:id].find
aggregate_failures do
expect(dast_site_profile.name).to eq(new_profile_name) 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.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 end
end end
......
# 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
...@@ -3,10 +3,12 @@ ...@@ -3,10 +3,12 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe GitlabSchema.types['DastSiteProfile'] do RSpec.describe GitlabSchema.types['DastSiteProfile'] do
include GraphqlHelpers
let_it_be(:dast_site_profile) { create(:dast_site_profile) } let_it_be(:dast_site_profile) { create(:dast_site_profile) }
let_it_be(:project) { dast_site_profile.project } let_it_be(:project) { dast_site_profile.project }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:fields) { %i[id profileName targetUrl editPath validationStatus userPermissions normalizedTargetUrl referencedInSecurityPolicies] } let_it_be(:fields) { %i[id profileName targetUrl editPath excludedUrls requestHeaders validationStatus userPermissions normalizedTargetUrl auth referencedInSecurityPolicies] }
subject do subject do
GitlabSchema.execute( GitlabSchema.execute(
...@@ -40,15 +42,7 @@ RSpec.describe GitlabSchema.types['DastSiteProfile'] do ...@@ -40,15 +42,7 @@ RSpec.describe GitlabSchema.types['DastSiteProfile'] do
query project($fullPath: ID!) { query project($fullPath: ID!) {
project(fullPath: $fullPath) { project(fullPath: $fullPath) {
dastSiteProfiles(first: 1) { dastSiteProfiles(first: 1) {
nodes { nodes { #{all_graphql_fields_for('DastSiteProfile')} }
id
profileName
targetUrl
editPath
validationStatus
normalizedTargetUrl
referencedInSecurityPolicies
}
} }
} }
} }
...@@ -85,6 +79,47 @@ RSpec.describe GitlabSchema.types['DastSiteProfile'] do ...@@ -85,6 +79,47 @@ RSpec.describe GitlabSchema.types['DastSiteProfile'] do
end end
end end
describe 'excludedUrls field' do
context 'when the feature flag is disabled' do
it 'is nil' do
stub_feature_flags(security_dast_site_profiles_additional_fields: false)
expect(first_dast_site_profile['excludedUrls']).to eq(nil)
end
end
context 'when the feature flag is enabled' do
it 'is the excluded urls' do
expect(first_dast_site_profile['excludedUrls']).to eq(dast_site_profile.excluded_urls)
end
end
end
describe 'auth field' do
context 'when the feature flag is disabled' do
it 'is nil' do
stub_feature_flags(security_dast_site_profiles_additional_fields: false)
expect(first_dast_site_profile['auth']).to eq(nil)
end
end
context 'when the feature flag is enabled' do
it 'includes the correct values' do
auth = first_dast_site_profile['auth']
expect(auth).to include(
'enabled' => false,
'url' => dast_site_profile.auth_url,
'usernameField' => dast_site_profile.auth_username_field,
'passwordField' => dast_site_profile.auth_password_field,
'username' => dast_site_profile.auth_username,
'password' => nil
)
end
end
end
describe 'validation_status field' do describe 'validation_status field' do
it 'is the validation status' do it 'is the validation status' do
expect(first_dast_site_profile['validationStatus']).to eq('NONE') expect(first_dast_site_profile['validationStatus']).to eq('NONE')
......
...@@ -3,7 +3,9 @@ ...@@ -3,7 +3,9 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe DastSiteProfile, type: :model do RSpec.describe DastSiteProfile, type: :model do
subject { create(:dast_site_profile, :with_dast_site_validation) } let_it_be(:project) { create(:project) }
subject { create(:dast_site_profile, :with_dast_site_validation, project: project) }
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:project) }
...@@ -12,25 +14,86 @@ RSpec.describe DastSiteProfile, type: :model do ...@@ -12,25 +14,86 @@ RSpec.describe DastSiteProfile, type: :model do
end end
describe 'validations' do describe 'validations' do
let_it_be(:dast_site) { create(:dast_site, project: project) }
it { is_expected.to be_valid } it { is_expected.to be_valid }
it { is_expected.to validate_length_of(:auth_password_field).is_at_most(255) }
it { is_expected.to validate_length_of(:auth_url).is_at_most(1024).allow_nil }
it { is_expected.to validate_length_of(:auth_username).is_at_most(255) }
it { is_expected.to validate_length_of(:auth_username_field).is_at_most(255) }
it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_length_of(:name).is_at_most(255) }
it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) }
it { is_expected.to validate_presence_of(:project_id) }
it { is_expected.to validate_presence_of(:dast_site_id) } it { is_expected.to validate_presence_of(:dast_site_id) }
it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:name) }
it { is_expected.to validate_presence_of(:project_id) }
it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) }
context 'when the project_id and dast_site.project_id do not match' do describe '#auth_url' do
let(:project) { create(:project) } context 'when the auth_uri is nil' do
let(:dast_site) { create(:dast_site) } it 'is valid' do
expect(subject).to be_valid
end
end
subject { build(:dast_site_profile, project: project, dast_site: dast_site) } context 'when the auth_url is not a valid uri' do
subject { build(:dast_site_profile, project: project, dast_site: dast_site, auth_url: 'hello-world') }
it 'is not valid' do it 'is not valid' do
expect(subject.valid?).to be_falsey expect(subject).not_to be_valid
end
end
context 'when the auth_url is not public' do
subject { build(:dast_site_profile, project: project, dast_site: dast_site) }
it 'is valid' do
expect(subject).to be_valid
end
end
end
describe '#excluded_urls' do
let(:excluded_urls) { [] }
subject { build(:dast_site_profile, project: project, dast_site: dast_site, excluded_urls: excluded_urls) }
it { is_expected.to allow_value(Array.new(25, generate(:url))).for(:excluded_urls) }
it { is_expected.not_to allow_value(Array.new(26, generate(:url))).for(:excluded_urls) }
context 'when there are some urls that are invalid' do
let(:excluded_urls) do
[
generate(:url),
generate(:url) + '/' + SecureRandom.alphanumeric(1024),
'hello-world',
'hello-world' + '/' + SecureRandom.alphanumeric(1024)
]
end
it 'is not valid', :aggregate_failures do
expected_full_messages = [
"Excluded urls contains invalid URLs (#{excluded_urls[2]}, #{excluded_urls[3]})",
"Excluded urls contains URLs that exceed the 1024 character limit (#{excluded_urls[1]}, #{excluded_urls[3]})"
]
expect(subject).not_to be_valid
expect(subject.errors.full_messages).to eq(expected_full_messages)
end
end
end
describe '#project' do
context 'when the project_id and dast_site.project_id do not match' do
let_it_be(:dast_site) { create(:dast_site) }
subject { build(:dast_site_profile, dast_site: dast_site, project: project) }
it 'is not valid', :aggregate_failures do
expect(subject).not_to be_valid
expect(subject.errors.full_messages).to include('Project does not match dast_site.project') expect(subject.errors.full_messages).to include('Project does not match dast_site.project')
end end
end end
end end
end
describe 'scopes' do describe 'scopes' do
describe '.with_dast_site_and_validation' do describe '.with_dast_site_and_validation' do
...@@ -38,7 +101,7 @@ RSpec.describe DastSiteProfile, type: :model do ...@@ -38,7 +101,7 @@ RSpec.describe DastSiteProfile, type: :model do
subject.dast_site_validation.update!(state: :failed) subject.dast_site_validation.update!(state: :failed)
end end
it 'eager loads the association' do it 'eager loads the association', :aggregate_failures do
subject subject
recorder = ActiveRecord::QueryRecorder.new do recorder = ActiveRecord::QueryRecorder.new do
...@@ -46,12 +109,10 @@ RSpec.describe DastSiteProfile, type: :model do ...@@ -46,12 +109,10 @@ RSpec.describe DastSiteProfile, type: :model do
subject.dast_site_validation subject.dast_site_validation
end end
aggregate_failures do
expect(subject.status).to eq('failed') # ensures guard passed expect(subject.status).to eq('failed') # ensures guard passed
expect(recorder.count).to be_zero expect(recorder.count).to be_zero
end end
end end
end
describe '.with_name' do describe '.with_name' do
it 'returns the dast_site_profiles with given name' do it 'returns the dast_site_profiles with given name' do
...@@ -83,15 +144,13 @@ RSpec.describe DastSiteProfile, type: :model do ...@@ -83,15 +144,13 @@ RSpec.describe DastSiteProfile, type: :model do
describe '#status' do describe '#status' do
context 'when dast_site_validation association does not exist' do context 'when dast_site_validation association does not exist' do
it 'is none' do it 'is none', :aggregate_failures do
subject.dast_site.update!(dast_site_validation_id: nil) subject.dast_site.update!(dast_site_validation_id: nil)
aggregate_failures do
expect(subject.dast_site_validation).to be_nil expect(subject.dast_site_validation).to be_nil
expect(subject.status).to eq('none') expect(subject.status).to eq('none')
end end
end end
end
context 'when dast_site_validation association does exist' do context 'when dast_site_validation association does exist' do
it 'is dast_site_validation#state' do it 'is dast_site_validation#state' do
......
...@@ -15,7 +15,17 @@ RSpec.describe 'Creating a DAST Site Profile' do ...@@ -15,7 +15,17 @@ RSpec.describe 'Creating a DAST Site Profile' do
mutation_name, mutation_name,
full_path: full_path, full_path: full_path,
profile_name: profile_name, profile_name: profile_name,
target_url: target_url target_url: target_url,
excluded_urls: ["#{target_url}/logout"],
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 end
......
...@@ -17,7 +17,17 @@ RSpec.describe 'Creating a DAST Site Profile' do ...@@ -17,7 +17,17 @@ RSpec.describe 'Creating a DAST Site Profile' do
full_path: full_path, full_path: full_path,
id: dast_site_profile.to_global_id.to_s, id: dast_site_profile.to_global_id.to_s,
profile_name: new_profile_name, profile_name: new_profile_name,
target_url: new_target_url target_url: new_target_url,
excluded_urls: ["#{new_target_url}/signout"],
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 end
......
...@@ -13,13 +13,7 @@ RSpec.describe 'Query.project(fullPath).dastSiteProfile' do ...@@ -13,13 +13,7 @@ RSpec.describe 'Query.project(fullPath).dastSiteProfile' do
%( %(
query project($fullPath: ID!, $id: DastSiteProfileID!) { query project($fullPath: ID!, $id: DastSiteProfileID!) {
project(fullPath: $fullPath) { project(fullPath: $fullPath) {
dastSiteProfile(id: $id) { dastSiteProfile(id: $id) { #{all_graphql_fields_for('DastSiteProfile')} }
id
profileName
targetUrl
validationStatus
normalizedTargetUrl
}
} }
} }
) )
......
...@@ -17,14 +17,7 @@ RSpec.describe 'Query.project(fullPath).dastSiteProfiles' do ...@@ -17,14 +17,7 @@ RSpec.describe 'Query.project(fullPath).dastSiteProfiles' do
pageInfo { pageInfo {
hasNextPage hasNextPage
} }
nodes { nodes { #{all_graphql_fields_for('DastSiteProfile')} }
id
profileName
targetUrl
editPath
validationStatus
normalizedTargetUrl
}
} }
} }
} }
......
...@@ -7,13 +7,14 @@ RSpec.describe DastSiteProfiles::CreateService do ...@@ -7,13 +7,14 @@ RSpec.describe DastSiteProfiles::CreateService do
let(:project) { create(:project, :repository, creator: user) } let(:project) { create(:project, :repository, creator: user) }
let(:name) { FFaker::Company.catch_phrase } let(:name) { FFaker::Company.catch_phrase }
let(:target_url) { generate(:url) } let(:target_url) { generate(:url) }
let(:excluded_urls) { ["#{target_url}/signout"] }
before do before do
stub_licensed_features(security_on_demand_scans: true) stub_licensed_features(security_on_demand_scans: true)
end end
describe '#execute' do describe '#execute' do
subject { described_class.new(project, user).execute(name: name, target_url: target_url) } subject { described_class.new(project, user).execute(name: name, target_url: target_url, excluded_urls: excluded_urls) }
let(:status) { subject.status } let(:status) { subject.status }
let(:message) { subject.message } let(:message) { subject.message }
...@@ -77,6 +78,22 @@ RSpec.describe DastSiteProfiles::CreateService do ...@@ -77,6 +78,22 @@ RSpec.describe DastSiteProfiles::CreateService do
end end
end end
context 'when excluded_urls is nil' do
let(:excluded_urls) { nil }
it 'defaults to an empty array' do
expect(payload.excluded_urls).to be_empty
end
end
context 'when excluded_urls is not supplied' do
subject { described_class.new(project, user).execute(name: name, target_url: target_url) }
it 'defaults to an empty array' do
expect(payload.excluded_urls).to be_empty
end
end
context 'when on demand scan licensed feature is not available' do context 'when on demand scan licensed feature is not available' do
before do before do
stub_licensed_features(security_on_demand_scans: false) stub_licensed_features(security_on_demand_scans: false)
......
...@@ -9,6 +9,7 @@ RSpec.describe DastSiteProfiles::UpdateService do ...@@ -9,6 +9,7 @@ RSpec.describe DastSiteProfiles::UpdateService do
let(:new_profile_name) { SecureRandom.hex } let(:new_profile_name) { SecureRandom.hex }
let(:new_target_url) { generate(:url) } let(:new_target_url) { generate(:url) }
let(:new_excluded_urls) { ["#{new_target_url}/signout"] }
before do before do
stub_licensed_features(security_on_demand_scans: true) stub_licensed_features(security_on_demand_scans: true)
...@@ -18,8 +19,9 @@ RSpec.describe DastSiteProfiles::UpdateService do ...@@ -18,8 +19,9 @@ RSpec.describe DastSiteProfiles::UpdateService do
subject do subject do
described_class.new(project, user).execute( described_class.new(project, user).execute(
id: dast_profile.id, id: dast_profile.id,
profile_name: new_profile_name, name: new_profile_name,
target_url: new_target_url target_url: new_target_url,
excluded_urls: new_excluded_urls
) )
end end
...@@ -50,10 +52,11 @@ RSpec.describe DastSiteProfiles::UpdateService do ...@@ -50,10 +52,11 @@ RSpec.describe DastSiteProfiles::UpdateService do
it 'updates the dast_site_profile' do it 'updates the dast_site_profile' do
updated_dast_site_profile = payload.reload updated_dast_site_profile = payload.reload
aggregate_failures do expect(updated_dast_site_profile).to have_attributes(
expect(updated_dast_site_profile.name).to eq(new_profile_name) name: new_profile_name,
expect(updated_dast_site_profile.dast_site.url).to eq(new_target_url) excluded_urls: new_excluded_urls,
end dast_site: have_attributes(url: new_target_url)
)
end end
it 'returns a dast_site_profile payload' do it 'returns a dast_site_profile payload' do
...@@ -72,6 +75,27 @@ RSpec.describe DastSiteProfiles::UpdateService do ...@@ -72,6 +75,27 @@ RSpec.describe DastSiteProfiles::UpdateService do
end end
end end
context 'when the target url is nil' do
let(:new_target_url) { nil }
let(:new_excluded_urls) { [generate(:url)] }
it 'returns a success status' do
expect(status).to eq(:success)
end
it 'does not attempt to change the associated dast_site' do
finder = double(DastSiteProfilesFinder)
profile = double(DastSiteProfile, referenced_in_security_policies: [])
allow(DastSiteProfilesFinder).to receive(:new).and_return(finder)
allow(finder).to receive_message_chain(:execute, :first!).and_return(profile)
expect(profile).to receive(:update!).with(hash_excluding(dast_profile.dast_site))
subject
end
end
context 'when the dast_site_profile doesn\'t exist' do context 'when the dast_site_profile doesn\'t exist' do
before do before do
dast_profile.destroy! dast_profile.destroy!
......
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