Add Dast::SiteProfileSecretVariables and associations
What does this MR do?
this merge request adds the ability to store secrets for dast
on-demand scans via the introduction of a new table (dast_site_profile_secret_variables
).
this merge request started as !54397 (closed) but was separated out to make review easier.
Why?
as part of the work on dast
on-demand scans, we want to give users the ability to specify login credentials for their on-demand scans in order to perform authenticated scans.
during security review we identified that ci_variables
seemed like the correct place to store this data but that one of the risks associated was that sensitive ci_variables
could end up being sent to jobs that should not have access to them (e.g. if someone were set the environment scope incorrectly).
following discussion in review on this merge request, we opted to move away from using the ci_variables
and simplify the solution by introducing a new table and extending Ci::Contextable
.
Related Issue(s)
Related Merge Request(s)
- proof of concept: !47943 (closed)
- parent: !54397 (closed)
Database
Migrations
% rails db:migrate:up VERSION=20210305031822 && rails db:migrate:down VERSION=20210305031822
== 20210305031822 CreateDastSiteProfileVariables: migrating ===================
-- create_table(:dast_site_profile_secret_variables, {:comment=>"{\"owner\":\"group::dynamic analysis\",\"description\":\"Secret variables used in DAST on-demand scans\"}"})
-- quote_column_name(:key)
-> 0.0000s
-> 0.0143s
-- quote_table_name("check_8cbef204b2")
-> 0.0000s
-- quote_table_name("check_236213f179")
-> 0.0000s
-- quote_table_name("check_b49080abbf")
-> 0.0000s
-- quote_table_name(:dast_site_profile_secret_variables)
-> 0.0000s
-- execute("ALTER TABLE \"dast_site_profile_secret_variables\"\nADD CONSTRAINT \"check_8cbef204b2\" CHECK (char_length(\"key\") <= 255),\nADD CONSTRAINT \"check_236213f179\" CHECK (length(encrypted_value) <= 13352),\nADD CONSTRAINT \"check_b49080abbf\" CHECK (length(encrypted_value_iv) <= 17)\n")
-> 0.0012s
== 20210305031822 CreateDastSiteProfileVariables: migrated (0.0222s) ==========
== 20210305031822 CreateDastSiteProfileVariables: reverting ===================
-- drop_table(:dast_site_profile_secret_variables)
-> 0.0110s
== 20210305031822 CreateDastSiteProfileVariables: reverted (0.0110s) ==========
Does this MR meet the acceptance criteria?
Conformity
-
📋 Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because _____.
-
-
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team