Skip to content

Add deny all requests setting to UrlBlocker: Part 2

Luke Duncalfe requested to merge 377371-deny-all-requests-addressable-validation into master

What does this MR do and why?

!110599 (merged) introduced a new deny_all_requests_except_allowed to application settings.

This merge request addresses the problem outlined in #377371 (comment 1271330392) that previously led the ApplicationSetting model to be saved in an invalid state due to the new setting.

This change has ApplicationSetting refer to the current, potentially unpersisted, state of its deny_all_requests_except_allowed property to control whether the validation enforces the setting or not.

This means an instance cannot enable deny_all_requests_except_allowed while a URL in the application settings would break the rule. This prevents the application settings from being able to become invalid. The admin must first change that URL property or add it to the allow list before they can enable the deny setting.

QA steps:

  1. Enable the feature flag: 
     Feature.enable(:deny_all_requests_except_allowed)
  2. Visit the admin Settings > Network and expand the Outbound requests section, or visit /admin/application_settings/network#js-outbound-settings directly.
  3. Check the Block all requests... checkbox.
  4. Click Save changes.
  5. You should be prevented from saving the settings, and see an error like below:
    image

This is because by default GitLab instances should have a public_runner_releases_url property that would violate the setting.

For now, until we handle a better way to allow admins to change the public_runner_releases_url property #377371 (comment 1294228003), you can have your instance enable the setting by adding gitlab.com to the list of allowed domains:

  1. Uncheck the Block all requests... checkbox.
  2. Add gitlab.com:443 to the Local IP addresses and domain names that hooks and integrations can access text area of the form.
  3. Save.

You will be able to toggle the Block all requests... checkbox and save.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #377371 (closed)

Edited by Luke Duncalfe

Merge request reports