Skip to content

Add outbound setting for system hooks

What does this MR do?

This MR:

  • adds a new outbound requests setting Allow requests to the local network from system hooks. Because system hooks to local network are currently allowed by default we want to add a new setting in order to have control, in case admins do not want local requests to be allowed
  • renames existing setting allow_local_requests_from_hooks_and_services to allow_local_requests_from_web_hooks_and_services to be more specific (since there is a new setting for system hooks)
  • it also respects https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/30350 domain whitelist

More information regarding the motivation behind this change here: https://gitlab.com/gitlab-org/gitlab-ce/issues/55474

image

DB migrations output

➜  gitlab git:(georgekoltsov/55474-outbound-setting-system-hooks) rails db:migrate
== 20190726101050 RenameAllowLocalRequestsFromHooksAndServicesApplicationSetting: migrating
-- transaction_open?()
   -> 0.0000s
-- columns(:application_settings)
   -> 0.0122s
-- add_column(:application_settings, :allow_local_requests_from_web_hooks_and_services, :boolean, {:limit=>nil, :precision=>nil, :scale=>nil})
   -> 0.0011s
-- change_column_default(:application_settings, :allow_local_requests_from_web_hooks_and_services, "false")
   -> 0.0124s
-- quote_table_name(:application_settings)
   -> 0.0000s
-- quote_column_name(:allow_local_requests_from_hooks_and_services)
   -> 0.0000s
-- quote_column_name(:allow_local_requests_from_web_hooks_and_services)
   -> 0.0000s
-- execute("CREATE OR REPLACE FUNCTION trigger_806273a4d8be()\nRETURNS trigger AS\n$BODY$\nBEGIN\n  NEW.\"allow_local_requests_from_web_hooks_and_services\" := NEW.\"allow_local_requests_from_hooks_and_services\";\n  RETURN NEW;\nEND;\n$BODY$\nLANGUAGE 'plpgsql'\nVOLATILE\n")
   -> 0.0037s
-- execute("CREATE TRIGGER trigger_806273a4d8be\nBEFORE INSERT OR UPDATE\nON \"application_settings\"\nFOR EACH ROW\nEXECUTE PROCEDURE trigger_806273a4d8be()\n")
   -> 0.0007s
-- transaction_open?()
   -> 0.0000s
-- exec_query("SELECT COUNT(*) AS count FROM \"application_settings\"")
   -> 0.0011s
-- 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.0005s
-- execute("UPDATE \"application_settings\" SET \"allow_local_requests_from_web_hooks_and_services\" = \"application_settings\".\"allow_local_requests_from_hooks_and_services\" WHERE \"application_settings\".\"id\" >= 1")
   -> 0.0010s
-- change_column_null(:application_settings, :allow_local_requests_from_web_hooks_and_services, false)
   -> 0.0007s
-- indexes(:application_settings)
   -> 0.0024s
-- foreign_keys(:application_settings)
   -> 0.0022s
== 20190726101050 RenameAllowLocalRequestsFromHooksAndServicesApplicationSetting: migrated (0.0593s)

== 20190726101133 AddAllowLocalRequestsFromSystemHooksToApplicationSettings: migrating
-- add_column(:application_settings, :allow_local_requests_from_system_hooks, :boolean, {:default=>true, :null=>false})
   -> 0.0062s
== 20190726101133 AddAllowLocalRequestsFromSystemHooksToApplicationSettings: migrated (0.0063s)

== 20190801114109 CleanupAllowLocalRequestsFromHooksAndServicesApplicationSettingRename: migrating
-- execute("DROP TRIGGER IF EXISTS trigger_806273a4d8be ON application_settings")
   -> 0.0007s
-- execute("DROP FUNCTION IF EXISTS trigger_806273a4d8be()")
   -> 0.0005s
-- remove_column(:application_settings, :allow_local_requests_from_hooks_and_services)
   -> 0.0008s
== 20190801114109 CleanupAllowLocalRequestsFromHooksAndServicesApplicationSettingRename: migrated (0.0168s)

Steps to reproduce

  1. rails db:migrate && gdk run
  2. Go to /admin/application_settings/network & enable Allow requests to the local network from system hooks
  3. Go to /admin/hooks. Create a new hook with URL http://localhost:3030 no need to run any service on that port
  4. Verify system hook is successfully created
  5. Test newly created system hook and verify while there is an error message it is not related to URL being blocked (or test it with something running on port 3030 to verify response is successful)
  6. Go to /admin/application_settings/network & disable Allow requests to the local network from system hooks
  7. Test previously created system hook and verify there is an error message saying that the url is blocked
  8. Try and create a new localhost system hook. Verify error message saying that localhost url is blocked
  9. Verify existing Allow requests to the local network from web hooks and services setting works as expected by enabling/disabling it and checking project webhooks

Does this MR meet the acceptance criteria?

Conformity

Edited by George Koltsov

Merge request reports