Use allowlist/denylist in application settings backend (!46170) · Merge requests · GitLab.org / GitLab

Doug Stull requested to merge 243555-update-whitelist-blacklist-to-allowlist-denylist-in-signup-restrictions-window-2 into master Oct 26, 2020
What does this MR do?

Renames the following columns on application_settings
domain_blacklist_enabled -> domain_denylist_enabled
domain_blacklist -> domain_denylist
domain_whitelist -> domain_allowlist
~~outbound_local_requests_whitelist -> outbound_local_requests_allowlist~~ issues in app/validators/qualified_domain_array_validator.rb - aborting this rename for now
Database

Renamed columns as per this guide - https://docs.gitlab.com/ee/development/what_requires_downtime.html#renaming-columns
Migration output:
10:41 $ be rake db:migrate
== 20201029143650 RenameApplicationSettingsToAllowDenyNames: migrating ========
-- column_exists?(:application_settings, :id)
   -> 0.0353s
-- transaction_open?()
   -> 0.0000s
-- columns(:application_settings)
   -> 0.0339s
-- add_column(:application_settings, :domain_denylist_enabled, :boolean, {:limit=>nil, :precision=>nil, :scale=>nil})
   -> 0.0062s
-- change_column_default(:application_settings, :domain_denylist_enabled, "false")
   -> 0.0391s
-- transaction_open?()
   -> 0.0000s
-- exec_query("SELECT COUNT(*) AS count FROM \"application_settings\"")
   -> 0.0026s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" ORDER BY \"application_settings\".\"id\" ASC LIMIT 1")
   -> 0.0005s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" WHERE \"application_settings\".\"id\" >= 1 ORDER BY \"application_settings\".\"id\" ASC LIMIT 1 OFFSET 1")
   -> 0.0003s
-- execute("UPDATE \"application_settings\" SET \"domain_denylist_enabled\" = \"application_settings\".\"domain_blacklist_enabled\" WHERE \"application_settings\".\"id\" >= 1")
   -> 0.0026s
-- indexes(:application_settings)
   -> 0.0039s
-- foreign_keys(:application_settings)
   -> 0.0055s
-- transaction_open?()
   -> 0.0000s
-- column_exists?(:application_settings, :domain_blacklist_enabled)
   -> 0.0326s
-- column_exists?(:application_settings, :domain_denylist_enabled)
   -> 0.0330s
-- current_schema()
   -> 0.0002s
-- quote_table_name(:application_settings)
   -> 0.0000s
-- quote_column_name(:domain_blacklist_enabled)
   -> 0.0000s
-- quote_column_name(:domain_denylist_enabled)
   -> 0.0000s
-- execute("CREATE OR REPLACE FUNCTION trigger_1f0ae12498d3()\nRETURNS trigger AS\n$BODY$\nBEGIN\n  NEW.\"domain_denylist_enabled\" := NEW.\"domain_blacklist_enabled\";\n  RETURN NEW;\nEND;\n$BODY$\nLANGUAGE 'plpgsql'\nVOLATILE\n")
   -> 0.0122s
-- execute("DROP TRIGGER IF EXISTS trigger_1f0ae12498d3\nON \"application_settings\"\n")
   -> 0.0002s
-- execute("CREATE TRIGGER trigger_1f0ae12498d3\nBEFORE INSERT OR UPDATE\nON \"application_settings\"\nFOR EACH ROW\nEXECUTE FUNCTION trigger_1f0ae12498d3()\n")
   -> 0.0036s
-- column_exists?(:application_settings, :id)
   -> 0.0328s
-- transaction_open?()
   -> 0.0000s
-- columns(:application_settings)
   -> 0.0295s
-- add_column(:application_settings, :domain_denylist, :text, {:limit=>nil, :precision=>nil, :scale=>nil})
   -> 0.0009s
-- transaction_open?()
   -> 0.0000s
-- exec_query("SELECT COUNT(*) AS count FROM \"application_settings\"")
   -> 0.0005s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" ORDER BY \"application_settings\".\"id\" ASC LIMIT 1")
   -> 0.0002s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" WHERE \"application_settings\".\"id\" >= 1 ORDER BY \"application_settings\".\"id\" ASC LIMIT 1 OFFSET 1")
   -> 0.0002s
-- execute("UPDATE \"application_settings\" SET \"domain_denylist\" = \"application_settings\".\"domain_blacklist\" WHERE \"application_settings\".\"id\" >= 1")
   -> 0.0007s
-- indexes(:application_settings)
   -> 0.0033s
-- foreign_keys(:application_settings)
   -> 0.0015s
-- transaction_open?()
   -> 0.0000s
-- column_exists?(:application_settings, :domain_blacklist)
   -> 0.0299s
-- column_exists?(:application_settings, :domain_denylist)
   -> 0.0290s
-- current_schema()
   -> 0.0002s
-- quote_table_name(:application_settings)
   -> 0.0000s
-- quote_column_name(:domain_blacklist)
   -> 0.0000s
-- quote_column_name(:domain_denylist)
   -> 0.0000s
-- execute("CREATE OR REPLACE FUNCTION trigger_72b785aa2f14()\nRETURNS trigger AS\n$BODY$\nBEGIN\n  NEW.\"domain_denylist\" := NEW.\"domain_blacklist\";\n  RETURN NEW;\nEND;\n$BODY$\nLANGUAGE 'plpgsql'\nVOLATILE\n")
   -> 0.0006s
-- execute("DROP TRIGGER IF EXISTS trigger_72b785aa2f14\nON \"application_settings\"\n")
   -> 0.0002s
-- execute("CREATE TRIGGER trigger_72b785aa2f14\nBEFORE INSERT OR UPDATE\nON \"application_settings\"\nFOR EACH ROW\nEXECUTE FUNCTION trigger_72b785aa2f14()\n")
   -> 0.0004s
-- column_exists?(:application_settings, :id)
   -> 0.0293s
-- transaction_open?()
   -> 0.0000s
-- columns(:application_settings)
   -> 0.0287s
-- add_column(:application_settings, :domain_allowlist, :text, {:limit=>nil, :precision=>nil, :scale=>nil})
   -> 0.0010s
-- transaction_open?()
   -> 0.0000s
-- exec_query("SELECT COUNT(*) AS count FROM \"application_settings\"")
   -> 0.0005s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" ORDER BY \"application_settings\".\"id\" ASC LIMIT 1")
   -> 0.0002s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" WHERE \"application_settings\".\"id\" >= 1 ORDER BY \"application_settings\".\"id\" ASC LIMIT 1 OFFSET 1")
   -> 0.0003s
-- execute("UPDATE \"application_settings\" SET \"domain_allowlist\" = \"application_settings\".\"domain_whitelist\" WHERE \"application_settings\".\"id\" >= 1")
   -> 0.0007s
-- indexes(:application_settings)
   -> 0.0029s
-- foreign_keys(:application_settings)
   -> 0.0015s
-- transaction_open?()
   -> 0.0000s
-- column_exists?(:application_settings, :domain_whitelist)
   -> 0.0313s
-- column_exists?(:application_settings, :domain_allowlist)
   -> 0.0293s
-- current_schema()
   -> 0.0003s
-- quote_table_name(:application_settings)
   -> 0.0000s
-- quote_column_name(:domain_whitelist)
   -> 0.0000s
-- quote_column_name(:domain_allowlist)
   -> 0.0000s
-- execute("CREATE OR REPLACE FUNCTION trigger_3d531acd472b()\nRETURNS trigger AS\n$BODY$\nBEGIN\n  NEW.\"domain_allowlist\" := NEW.\"domain_whitelist\";\n  RETURN NEW;\nEND;\n$BODY$\nLANGUAGE 'plpgsql'\nVOLATILE\n")
   -> 0.0005s
-- execute("DROP TRIGGER IF EXISTS trigger_3d531acd472b\nON \"application_settings\"\n")
   -> 0.0003s
-- execute("CREATE TRIGGER trigger_3d531acd472b\nBEFORE INSERT OR UPDATE\nON \"application_settings\"\nFOR EACH ROW\nEXECUTE FUNCTION trigger_3d531acd472b()\n")
   -> 0.0004s
-- column_exists?(:application_settings, :id)
   -> 0.0302s
-- transaction_open?()
   -> 0.0000s
-- columns(:application_settings)
   -> 0.0296s
-- transaction_open?()
   -> 0.0000s
-- exec_query("SELECT COUNT(*) AS count FROM \"application_settings\"")
   -> 0.0006s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" ORDER BY \"application_settings\".\"id\" ASC LIMIT 1")
   -> 0.0003s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" WHERE \"application_settings\".\"id\" >= 1 ORDER BY \"application_settings\".\"id\" ASC LIMIT 1 OFFSET 1")
   -> 0.0003s
== 20201029143650 RenameApplicationSettingsToAllowDenyNames: migrated (0.6950s)

== 20201029144157 CleanupApplicationSettingsToAllowDenyRename: migrating ======
-- execute("DROP TRIGGER IF EXISTS trigger_1f0ae12498d3 ON application_settings")
   -> 0.0023s
-- execute("DROP FUNCTION IF EXISTS trigger_1f0ae12498d3()")
   -> 0.0003s
-- remove_column(:application_settings, :domain_blacklist_enabled)
   -> 0.0019s
-- execute("DROP TRIGGER IF EXISTS trigger_72b785aa2f14 ON application_settings")
   -> 0.0004s
-- execute("DROP FUNCTION IF EXISTS trigger_72b785aa2f14()")
   -> 0.0003s
-- remove_column(:application_settings, :domain_blacklist)
   -> 0.0005s
-- execute("DROP TRIGGER IF EXISTS trigger_3d531acd472b ON application_settings")
   -> 0.0004s
-- execute("DROP FUNCTION IF EXISTS trigger_3d531acd472b()")
   -> 0.0003s
-- remove_column(:application_settings, :domain_whitelist)
   -> 0.0005s
-- execute("DROP TRIGGER IF EXISTS trigger_b94599a87e4b ON application_settings")
   -> 0.0004s
-- execute("DROP FUNCTION IF EXISTS trigger_b94599a87e4b()")
   -> 0.0006s
== 20201029144157 CleanupApplicationSettingsToAllowDenyRename: migrated (0.0102s)

10:57 $ be rake db:rollback
== 20201029144157 CleanupApplicationSettingsToAllowDenyRename: reverting ======
-- column_exists?(:application_settings, :id)
   -> 0.0353s
-- transaction_open?()
   -> 0.0000s
-- columns(:application_settings)
   -> 0.0297s
-- add_column(:application_settings, :domain_blacklist_enabled, :boolean, {:limit=>nil, :precision=>nil, :scale=>nil})
   -> 0.0020s
-- change_column_default(:application_settings, :domain_blacklist_enabled, "false")
   -> 0.0322s
-- transaction_open?()
   -> 0.0000s
-- exec_query("SELECT COUNT(*) AS count FROM \"application_settings\"")
   -> 0.0010s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" ORDER BY \"application_settings\".\"id\" ASC LIMIT 1")
   -> 0.0006s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" WHERE \"application_settings\".\"id\" >= 1 ORDER BY \"application_settings\".\"id\" ASC LIMIT 1 OFFSET 1")
   -> 0.0003s
-- execute("UPDATE \"application_settings\" SET \"domain_blacklist_enabled\" = \"application_settings\".\"domain_denylist_enabled\" WHERE \"application_settings\".\"id\" >= 1")
   -> 0.0013s
-- indexes(:application_settings)
   -> 0.0032s
-- foreign_keys(:application_settings)
   -> 0.0021s
-- transaction_open?()
   -> 0.0000s
-- column_exists?(:application_settings, :domain_denylist_enabled)
   -> 0.0291s
-- column_exists?(:application_settings, :domain_blacklist_enabled)
   -> 0.0293s
-- current_schema()
   -> 0.0012s
-- quote_table_name(:application_settings)
   -> 0.0000s
-- quote_column_name(:domain_blacklist_enabled)
   -> 0.0000s
-- quote_column_name(:domain_denylist_enabled)
   -> 0.0000s
-- execute("CREATE OR REPLACE FUNCTION trigger_1f0ae12498d3()\nRETURNS trigger AS\n$BODY$\nBEGIN\n  NEW.\"domain_denylist_enabled\" := NEW.\"domain_blacklist_enabled\";\n  RETURN NEW;\nEND;\n$BODY$\nLANGUAGE 'plpgsql'\nVOLATILE\n")
   -> 0.0018s
-- execute("DROP TRIGGER IF EXISTS trigger_1f0ae12498d3\nON \"application_settings\"\n")
   -> 0.0004s
-- execute("CREATE TRIGGER trigger_1f0ae12498d3\nBEFORE INSERT OR UPDATE\nON \"application_settings\"\nFOR EACH ROW\nEXECUTE FUNCTION trigger_1f0ae12498d3()\n")
   -> 0.0007s
-- column_exists?(:application_settings, :id)
   -> 0.0320s
-- transaction_open?()
   -> 0.0000s
-- columns(:application_settings)
   -> 0.0302s
-- add_column(:application_settings, :domain_blacklist, :text, {:limit=>nil, :precision=>nil, :scale=>nil})
   -> 0.0014s
-- transaction_open?()
   -> 0.0000s
-- exec_query("SELECT COUNT(*) AS count FROM \"application_settings\"")
   -> 0.0006s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" ORDER BY \"application_settings\".\"id\" ASC LIMIT 1")
   -> 0.0003s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" WHERE \"application_settings\".\"id\" >= 1 ORDER BY \"application_settings\".\"id\" ASC LIMIT 1 OFFSET 1")
   -> 0.0002s
-- execute("UPDATE \"application_settings\" SET \"domain_blacklist\" = \"application_settings\".\"domain_denylist\" WHERE \"application_settings\".\"id\" >= 1")
   -> 0.0007s
-- indexes(:application_settings)
   -> 0.0027s
-- foreign_keys(:application_settings)
   -> 0.0016s
-- transaction_open?()
   -> 0.0000s
-- column_exists?(:application_settings, :domain_denylist)
   -> 0.0295s
-- column_exists?(:application_settings, :domain_blacklist)
   -> 0.0294s
-- current_schema()
   -> 0.0002s
-- quote_table_name(:application_settings)
   -> 0.0000s
-- quote_column_name(:domain_blacklist)
   -> 0.0000s
-- quote_column_name(:domain_denylist)
   -> 0.0000s
-- execute("CREATE OR REPLACE FUNCTION trigger_72b785aa2f14()\nRETURNS trigger AS\n$BODY$\nBEGIN\n  NEW.\"domain_denylist\" := NEW.\"domain_blacklist\";\n  RETURN NEW;\nEND;\n$BODY$\nLANGUAGE 'plpgsql'\nVOLATILE\n")
   -> 0.0011s
-- execute("DROP TRIGGER IF EXISTS trigger_72b785aa2f14\nON \"application_settings\"\n")
   -> 0.0002s
-- execute("CREATE TRIGGER trigger_72b785aa2f14\nBEFORE INSERT OR UPDATE\nON \"application_settings\"\nFOR EACH ROW\nEXECUTE FUNCTION trigger_72b785aa2f14()\n")
   -> 0.0005s
-- column_exists?(:application_settings, :id)
   -> 0.0318s
-- transaction_open?()
   -> 0.0000s
-- columns(:application_settings)
   -> 0.0295s
-- add_column(:application_settings, :domain_whitelist, :text, {:limit=>nil, :precision=>nil, :scale=>nil})
   -> 0.0010s
-- transaction_open?()
   -> 0.0000s
-- exec_query("SELECT COUNT(*) AS count FROM \"application_settings\"")
   -> 0.0006s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" ORDER BY \"application_settings\".\"id\" ASC LIMIT 1")
   -> 0.0003s
-- exec_query("SELECT \"application_settings\".\"id\" FROM \"application_settings\" WHERE \"application_settings\".\"id\" >= 1 ORDER BY \"application_settings\".\"id\" ASC LIMIT 1 OFFSET 1")
   -> 0.0003s
-- execute("UPDATE \"application_settings\" SET \"domain_whitelist\" = \"application_settings\".\"domain_allowlist\" WHERE \"application_settings\".\"id\" >= 1")
   -> 0.0007s
-- indexes(:application_settings)
   -> 0.0027s
-- foreign_keys(:application_settings)
   -> 0.0015s
-- transaction_open?()
   -> 0.0000s
-- column_exists?(:application_settings, :domain_allowlist)
   -> 0.0310s
-- column_exists?(:application_settings, :domain_whitelist)
   -> 0.0302s
-- current_schema()
   -> 0.0002s
-- quote_table_name(:application_settings)
   -> 0.0000s
-- quote_column_name(:domain_whitelist)
   -> 0.0000s
-- quote_column_name(:domain_allowlist)
   -> 0.0000s
-- execute("CREATE OR REPLACE FUNCTION trigger_3d531acd472b()\nRETURNS trigger AS\n$BODY$\nBEGIN\n  NEW.\"domain_allowlist\" := NEW.\"domain_whitelist\";\n  RETURN NEW;\nEND;\n$BODY$\nLANGUAGE 'plpgsql'\nVOLATILE\n")
   -> 0.0007s
-- execute("DROP TRIGGER IF EXISTS trigger_3d531acd472b\nON \"application_settings\"\n")
   -> 0.0001s
-- execute("CREATE TRIGGER trigger_3d531acd472b\nBEFORE INSERT OR UPDATE\nON \"application_settings\"\nFOR EACH ROW\nEXECUTE FUNCTION trigger_3d531acd472b()\n")
   -> 0.0005s
-- column_exists?(:application_settings, :id)
   -> 0.0301s
-- transaction_open?()
   -> 0.0000s
-- columns(:application_settings)
   -> 0.0298s
-- execute("DROP TRIGGER IF EXISTS trigger_b94599a87e4b\nON \"application_settings\"\n")
NOTICE:  trigger "trigger_b94599a87e4b" for relation "application_settings" does not exist, skipping
   -> 0.0003s
-- execute("CREATE TRIGGER trigger_b94599a87e4b\nBEFORE INSERT OR UPDATE\nON \"application_settings\"\nFOR EACH ROW\nEXECUTE FUNCTION trigger_b94599a87e4b()\n")
   -> 0.0004s
== 20201029144157 CleanupApplicationSettingsToAllowDenyRename: reverted (0.6256s)

✔ ~/projects/gdk/gitlab [243555-update-whitelist-blacklist-to-allowlist-denylist-in-signup-restrictions-window-2 ↑·568|✚ 2⚑ 14]
10:58 $ be rake db:rollback
== 20201029143650 RenameApplicationSettingsToAllowDenyNames: reverting ========
-- execute("DROP TRIGGER IF EXISTS trigger_1f0ae12498d3 ON application_settings")
   -> 0.0014s
-- execute("DROP FUNCTION IF EXISTS trigger_1f0ae12498d3()")
   -> 0.0004s
-- remove_column(:application_settings, :domain_denylist_enabled)
   -> 0.0012s
-- execute("DROP TRIGGER IF EXISTS trigger_72b785aa2f14 ON application_settings")
   -> 0.0005s
-- execute("DROP FUNCTION IF EXISTS trigger_72b785aa2f14()")
   -> 0.0004s
-- remove_column(:application_settings, :domain_denylist)
   -> 0.0008s
-- execute("DROP TRIGGER IF EXISTS trigger_3d531acd472b ON application_settings")
   -> 0.0005s
-- execute("DROP FUNCTION IF EXISTS trigger_3d531acd472b()")
   -> 0.0004s
-- remove_column(:application_settings, :domain_allowlist)
   -> 0.0008s
-- execute("DROP TRIGGER IF EXISTS trigger_b94599a87e4b ON application_settings")
   -> 0.0006s
-- execute("DROP FUNCTION IF EXISTS trigger_b94599a87e4b()")
   -> 0.0004s
== 20201029143650 RenameApplicationSettingsToAllowDenyNames: reverted (0.0103s)
Does this MR meet the acceptance criteria?

Conformity

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
Related to #243555
Edited Nov 04, 2020 by Doug Stull
Use allowlist/denylist in application settings backend
