Commit d32f9530 authored by Philip Cunningham's avatar Philip Cunningham Committed by Toon Claes

Add Dast::SiteProfileVariables and associations

- Adds new models and specs to support DAST on-demand secrets
parent 1632d456
---
title: Add dast_profile_secret_variables table
merge_request: 56067
author:
type: added
# frozen_string_literal: true
class CreateDastSiteProfileVariables < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
table_comment = { owner: 'group::dynamic analysis', description: 'Secret variables used in DAST on-demand scans' }
encrypted_value_constraint_name = check_constraint_name(:dast_site_profile_secret_variables, 'encrypted_value', 'max_length')
encrypted_value_iv_constraint_name = check_constraint_name(:dast_site_profile_secret_variables, 'encrypted_value_iv', 'max_length')
create_table_with_constraints :dast_site_profile_secret_variables, comment: table_comment.to_json do |t|
t.references :dast_site_profile, null: false, foreign_key: { on_delete: :cascade }, index: false
t.timestamps_with_timezone
t.integer :variable_type, null: false, default: 1, limit: 2
t.text :key, null: false
t.binary :encrypted_value, null: false
t.binary :encrypted_value_iv, null: false, unique: true
t.index [:dast_site_profile_id, :key], unique: true, name: :index_site_profile_secret_variables_on_site_profile_id_and_key
t.text_limit :key, 255
# This does not currently have first-class support via create_table_with_constraints
t.check_constraint encrypted_value_constraint_name, 'length(encrypted_value) <= 13352'
t.check_constraint encrypted_value_iv_constraint_name, 'length(encrypted_value_iv) <= 17'
end
end
def down
drop_table :dast_site_profile_secret_variables
end
end
d91eb442db670adef6d610a2c79259377709e5c98615ba10b85eb998715b3130
\ No newline at end of file
......@@ -11808,6 +11808,31 @@ CREATE SEQUENCE dast_scanner_profiles_id_seq
ALTER SEQUENCE dast_scanner_profiles_id_seq OWNED BY dast_scanner_profiles.id;
CREATE TABLE dast_site_profile_secret_variables (
id bigint NOT NULL,
dast_site_profile_id bigint NOT NULL,
created_at timestamp with time zone NOT NULL,
updated_at timestamp with time zone NOT NULL,
variable_type smallint DEFAULT 1 NOT NULL,
key text NOT NULL,
encrypted_value bytea NOT NULL,
encrypted_value_iv bytea NOT NULL,
CONSTRAINT check_236213f179 CHECK ((length(encrypted_value) <= 13352)),
CONSTRAINT check_8cbef204b2 CHECK ((char_length(key) <= 255)),
CONSTRAINT check_b49080abbf CHECK ((length(encrypted_value_iv) <= 17))
);
COMMENT ON TABLE dast_site_profile_secret_variables IS '{"owner":"group::dynamic analysis","description":"Secret variables used in DAST on-demand scans"}';
CREATE SEQUENCE dast_site_profile_secret_variables_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER SEQUENCE dast_site_profile_secret_variables_id_seq OWNED BY dast_site_profile_secret_variables.id;
CREATE TABLE dast_site_profiles (
id bigint NOT NULL,
project_id bigint NOT NULL,
......@@ -19141,6 +19166,8 @@ ALTER TABLE ONLY dast_profiles ALTER COLUMN id SET DEFAULT nextval('dast_profile
ALTER TABLE ONLY dast_scanner_profiles ALTER COLUMN id SET DEFAULT nextval('dast_scanner_profiles_id_seq'::regclass);
ALTER TABLE ONLY dast_site_profile_secret_variables ALTER COLUMN id SET DEFAULT nextval('dast_site_profile_secret_variables_id_seq'::regclass);
ALTER TABLE ONLY dast_site_profiles ALTER COLUMN id SET DEFAULT nextval('dast_site_profiles_id_seq'::regclass);
ALTER TABLE ONLY dast_site_tokens ALTER COLUMN id SET DEFAULT nextval('dast_site_tokens_id_seq'::regclass);
......@@ -20341,6 +20368,9 @@ ALTER TABLE ONLY dast_profiles
ALTER TABLE ONLY dast_scanner_profiles
ADD CONSTRAINT dast_scanner_profiles_pkey PRIMARY KEY (id);
ALTER TABLE ONLY dast_site_profile_secret_variables
ADD CONSTRAINT dast_site_profile_secret_variables_pkey PRIMARY KEY (id);
ALTER TABLE ONLY dast_site_profiles
ADD CONSTRAINT dast_site_profiles_pkey PRIMARY KEY (id);
......@@ -23679,6 +23709,8 @@ CREATE UNIQUE INDEX index_services_on_unique_group_id_and_type ON services USING
CREATE UNIQUE INDEX index_shards_on_name ON shards USING btree (name);
CREATE UNIQUE INDEX index_site_profile_secret_variables_on_site_profile_id_and_key ON dast_site_profile_secret_variables USING btree (dast_site_profile_id, key);
CREATE INDEX index_slack_integrations_on_service_id ON slack_integrations USING btree (service_id);
CREATE UNIQUE INDEX index_slack_integrations_on_team_id_and_alias ON slack_integrations USING btree (team_id, alias);
......@@ -25560,6 +25592,9 @@ ALTER TABLE ONLY operations_strategies_user_lists
ALTER TABLE ONLY lfs_file_locks
ADD CONSTRAINT fk_rails_43df7a0412 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
ALTER TABLE ONLY dast_site_profile_secret_variables
ADD CONSTRAINT fk_rails_43e2897950 FOREIGN KEY (dast_site_profile_id) REFERENCES dast_site_profiles(id) ON DELETE CASCADE;
ALTER TABLE ONLY merge_request_assignees
ADD CONSTRAINT fk_rails_443443ce6f FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE;
......@@ -8,6 +8,8 @@ module Dast
belongs_to :dast_site_profile
belongs_to :dast_scanner_profile
has_many :secret_variables, through: :dast_site_profile, class_name: 'Dast::SiteProfileSecretVariable'
validates :description, length: { maximum: 255 }
validates :name, length: { maximum: 255 }, uniqueness: { scope: :project_id }, presence: true
validates :branch_name, length: { maximum: 255 }
......
# frozen_string_literal: true
module Dast
class SiteProfileSecretVariable < ApplicationRecord
include Ci::HasVariable
include Ci::Maskable
self.table_name = 'dast_site_profile_secret_variables'
belongs_to :dast_site_profile
delegate :project, to: :dast_site_profile, allow_nil: false
attribute :masked, default: true
attr_encrypted :value,
mode: :per_attribute_iv,
algorithm: 'aes-256-gcm',
key: Settings.attr_encrypted_db_key_base_32,
encode: false # No need to encode for binary column https://github.com/attr-encrypted/attr_encrypted#the-encode-encode_iv-encode_salt-and-default_encoding-options
# Secret variables must be masked to prevent them being readable in CI jobs
validates :masked, inclusion: { in: [true] }
validates :variable_type, inclusion: { in: ['env_var'] }
validates :key, uniqueness: { scope: :dast_site_profile_id, message: "(%{value}) has already been taken" }
# Since user input is base64 encoded before being encrypted, we must validate against the encoded length
MAX_VALUE_LENGTH = 10_000
MAX_ENCODED_VALUE_LENGTH = ((4 * MAX_VALUE_LENGTH / 3) + 3) & ~3
validates :value, length: {
maximum: MAX_ENCODED_VALUE_LENGTH, # encoded user input length
too_long: -> (object, data) { "exceeds the #{MAX_VALUE_LENGTH} character limit" } # user input length
}
# User input is base64 encoded before being encrypted in order to allow it to be masked by default
def raw_value=(new_value)
self.value = Base64.strict_encode64(new_value)
end
# Use #raw_value= to ensure value is maskable
private :value=
end
end
......@@ -4,6 +4,8 @@ class DastSiteProfile < ApplicationRecord
belongs_to :project
belongs_to :dast_site
has_many :secret_variables, class_name: 'Dast::SiteProfileSecretVariable'
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
......
# frozen_string_literal: true
FactoryBot.define do
factory :dast_site_profile_secret_variable, class: 'Dast::SiteProfileSecretVariable' do
dast_site_profile
sequence(:key) { |n| "VARIABLE_#{n}" }
raw_value { 'VARIABLE_VALUE' }
end
end
......@@ -9,6 +9,7 @@ RSpec.describe Dast::Profile, type: :model do
it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:dast_site_profile) }
it { is_expected.to belong_to(:dast_scanner_profile) }
it { is_expected.to have_many(:secret_variables).through(:dast_site_profile).class_name('Dast::SiteProfileSecretVariable') }
end
describe 'validations' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Dast::SiteProfileSecretVariable, type: :model do
let_it_be(:dast_site_profile) { create(:dast_site_profile) }
subject { create(:dast_site_profile_secret_variable, dast_site_profile: dast_site_profile) }
it_behaves_like 'CI variable'
describe 'constants' do
describe 'MAX_ENCODED_VALUE_LENGTH' do
it 'correctly expresses the relationship between input and encoded length' do
raw_value = SecureRandom.alphanumeric(described_class::MAX_VALUE_LENGTH)
expect(described_class::MAX_ENCODED_VALUE_LENGTH).to eq(Base64.strict_encode64(raw_value).length)
end
end
end
describe 'associations' do
it { is_expected.to belong_to(:dast_site_profile) }
end
describe 'validations' do
it { is_expected.to be_valid }
it { is_expected.to include_module(Ci::Maskable) }
it { is_expected.to include_module(Ci::HasVariable) }
it { is_expected.to validate_inclusion_of(:masked).in_array([true]) }
it { is_expected.to validate_uniqueness_of(:key).scoped_to(:dast_site_profile_id).with_message(/\(\w+\) has already been taken/) }
it 'only allows records where variable_type=env_var', :aggregate_failures do
subject = build(:dast_site_profile_secret_variable, variable_type: :file)
expect(subject).not_to be_valid
expect(subject.errors.full_messages).to include('Variable type is not included in the list')
end
describe '#value' do
subject { build(:dast_site_profile_secret_variable, dast_site_profile: dast_site_profile, raw_value: raw_value) }
context 'when the value is over the limit' do
let(:raw_value) { SecureRandom.alphanumeric(10_003) }
it 'is not valid', :aggregate_failures do
expect(subject).not_to be_valid
expect(subject.errors.full_messages).to include('Value exceeds the 10000 character limit')
end
it 'raises a database level error' do
allow(subject).to receive(:valid?).and_return(true)
expect { subject.save! }.to raise_error(ActiveRecord::StatementInvalid)
end
end
context 'when value is under the limit' do
let(:raw_value) { SecureRandom.alphanumeric(10_000) }
it 'is valid' do
expect(subject).to be_valid
end
it 'does not raise database level error' do
allow(subject).to receive(:valid?).and_return(true)
expect { subject.save! }.not_to raise_error
end
end
end
end
describe '#masked' do
it 'defaults to true', :aggregate_failures do
expect(subject.masked).to eq(true)
expect(described_class.new.masked).to eq(true)
end
end
describe '#project' do
it 'delegates to dast_site_profile' do
expect(subject.project).to eq(subject.dast_site_profile.project)
end
end
describe '#raw_value=' do
it 'pre-encodes the value' do
value = SecureRandom.alphanumeric
subject = create(:dast_site_profile_secret_variable, raw_value: value)
expect(Base64.strict_decode64(subject.value)).to eq(value)
end
end
describe '#value=' do
it 'raises an error because #raw_value= should be used instead' do
expect { subject.value = SecureRandom.alphanumeric }.to raise_error(NoMethodError, /private method `value=' called for/)
end
end
describe '#variable_type' do
it 'defaults to env_var', :aggregate_failures do
variable_type = 'env_var'
expect(subject.variable_type).to eq(variable_type)
expect(described_class.new.variable_type).to eq(variable_type)
end
end
end
......@@ -8,6 +8,7 @@ RSpec.describe DastSiteProfile, type: :model do
describe 'associations' do
it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:dast_site) }
it { is_expected.to have_many(:secret_variables).class_name('Dast::SiteProfileSecretVariable') }
end
describe 'validations' do
......
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