From 4e5201c2945c5d3b127ab02786b7dd9becee0a3c Mon Sep 17 00:00:00 2001 From: Robert May Date: Fri, 13 May 2022 16:59:29 +0100 Subject: [PATCH 1/9] Add support for configuring internal IPs Adds initial support for configuring internal IP ranges in addition to the custom IP lists at group level. For now this is added as an environment variable but in future would likely be an application setting to configure the in-use internal network IP ranges. Changelog: fixed --- .../_visibility_and_access.html.haml | 2 + .../group_ip_restrictions_allow_global.yml | 8 ++++ ...ally_allowed_ips_to_application_setting.rb | 13 +++++++ ...lly_allowed_ips_on_application_settings.rb | 16 ++++++++ db/schema_migrations/20220516092207 | 1 + db/schema_migrations/20220516123101 | 1 + db/structure.sql | 2 + .../admin/application_settings_controller.rb | 3 +- .../helpers/ee/application_settings_helper.rb | 1 + ee/app/models/ee/application_setting.rb | 13 ++++++- .../_globally_allowed_ips.html.haml | 9 +++++ ee/lib/gitlab/cidr.rb | 2 + ee/lib/gitlab/ip_restriction/enforcer.rb | 25 ++++++++++++- .../application_settings_controller_spec.rb | 37 ++++++++++++------- .../ee/gitlab/ip_restriction/enforcer_spec.rb | 24 ++++++++++++ ee/spec/models/application_setting_spec.rb | 23 +++++++++++- locale/gitlab.pot | 6 +++ 17 files changed, 167 insertions(+), 19 deletions(-) create mode 100644 config/feature_flags/development/group_ip_restrictions_allow_global.yml create mode 100644 db/migrate/20220516092207_add_globally_allowed_ips_to_application_setting.rb create mode 100644 db/migrate/20220516123101_add_text_limit_to_globally_allowed_ips_on_application_settings.rb create mode 100644 db/schema_migrations/20220516092207 create mode 100644 db/schema_migrations/20220516123101 create mode 100644 ee/app/views/admin/application_settings/_globally_allowed_ips.html.haml diff --git a/app/views/admin/application_settings/_visibility_and_access.html.haml b/app/views/admin/application_settings/_visibility_and_access.html.haml index e3c044ff979d4a..cc62ca354d1da0 100644 --- a/app/views/admin/application_settings/_visibility_and_access.html.haml +++ b/app/views/admin/application_settings/_visibility_and_access.html.haml @@ -63,4 +63,6 @@ %label.label-bold= s_('AdminSettings|Feed token') = f.gitlab_ui_checkbox_component :disable_feed_token, s_('AdminSettings|Disable feed token') + = render_if_exists 'admin/application_settings/globally_allowed_ips', form: f + = f.submit _('Save changes'), class: "gl-button btn btn-confirm" diff --git a/config/feature_flags/development/group_ip_restrictions_allow_global.yml b/config/feature_flags/development/group_ip_restrictions_allow_global.yml new file mode 100644 index 00000000000000..ee7e18a115db9a --- /dev/null +++ b/config/feature_flags/development/group_ip_restrictions_allow_global.yml @@ -0,0 +1,8 @@ +--- +name: group_ip_restrictions_allow_global +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87579 +rollout_issue_url: +milestone: '15.0' +type: development +group: group::source code +default_enabled: false diff --git a/db/migrate/20220516092207_add_globally_allowed_ips_to_application_setting.rb b/db/migrate/20220516092207_add_globally_allowed_ips_to_application_setting.rb new file mode 100644 index 00000000000000..36f724cba60efb --- /dev/null +++ b/db/migrate/20220516092207_add_globally_allowed_ips_to_application_setting.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddGloballyAllowedIpsToApplicationSetting < Gitlab::Database::Migration[2.0] + # rubocop:disable Migration/AddLimitToTextColumns + # limit is added in 20220516123101_add_text_limit_to_globally_allowed_ips_on_application_settings + def change + add_column :application_settings, :globally_allowed_ips, :text, null: false, default: "" + end + # rubocop:enable Migration/AddLimitToTextColumns +end diff --git a/db/migrate/20220516123101_add_text_limit_to_globally_allowed_ips_on_application_settings.rb b/db/migrate/20220516123101_add_text_limit_to_globally_allowed_ips_on_application_settings.rb new file mode 100644 index 00000000000000..f49987167e295a --- /dev/null +++ b/db/migrate/20220516123101_add_text_limit_to_globally_allowed_ips_on_application_settings.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddTextLimitToGloballyAllowedIpsOnApplicationSettings < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + def up + add_text_limit :application_settings, :globally_allowed_ips, 255 + end + + def down + remove_text_limit :application_settings, :globally_allowed_ips + end +end diff --git a/db/schema_migrations/20220516092207 b/db/schema_migrations/20220516092207 new file mode 100644 index 00000000000000..8731d03e39e015 --- /dev/null +++ b/db/schema_migrations/20220516092207 @@ -0,0 +1 @@ +c55f8c6e45e933207eae2036cd2705530bce5c79ff54ac3e33cef111949c736d \ No newline at end of file diff --git a/db/schema_migrations/20220516123101 b/db/schema_migrations/20220516123101 new file mode 100644 index 00000000000000..ea64cc0ea46a93 --- /dev/null +++ b/db/schema_migrations/20220516123101 @@ -0,0 +1 @@ +e1421ae1b1f021c5aa1546b7ffdbab4fb26e6fbbe0d1d0d4f57cb39735bc0221 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index fa9d9e8f778f35..ef5c8ecc8271a5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11305,6 +11305,7 @@ CREATE TABLE application_settings ( container_registry_pre_import_timeout integer DEFAULT 1800 NOT NULL, container_registry_import_timeout integer DEFAULT 600 NOT NULL, pipeline_limit_per_project_user_sha integer DEFAULT 0 NOT NULL, + globally_allowed_ips character varying(255) DEFAULT ''::character varying NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), @@ -11326,6 +11327,7 @@ CREATE TABLE application_settings ( CONSTRAINT check_5bcba483c4 CHECK ((char_length(sentry_environment) <= 255)), CONSTRAINT check_718b4458ae CHECK ((char_length(personal_access_token_prefix) <= 20)), CONSTRAINT check_7227fad848 CHECK ((char_length(rate_limiting_response_text) <= 255)), + CONSTRAINT check_734cc9407a CHECK ((char_length(globally_allowed_ips) <= 255)), CONSTRAINT check_7ccfe2764a CHECK ((char_length(arkose_labs_namespace) <= 255)), CONSTRAINT check_85a39b68ff CHECK ((char_length(encrypted_ci_jwt_signing_key_iv) <= 255)), CONSTRAINT check_8dca35398a CHECK ((char_length(public_runner_releases_url) <= 255)), diff --git a/ee/app/controllers/ee/admin/application_settings_controller.rb b/ee/app/controllers/ee/admin/application_settings_controller.rb index 0d86f004b9f622..636cb675f4d961 100644 --- a/ee/app/controllers/ee/admin/application_settings_controller.rb +++ b/ee/app/controllers/ee/admin/application_settings_controller.rb @@ -66,7 +66,8 @@ def visible_application_setting_attributes required_ci_templates: :required_instance_ci_template, disable_name_update_for_users: :updating_name_disabled_for_users, package_forwarding: [:npm_package_requests_forwarding, :pypi_package_requests_forwarding], - default_branch_protection_restriction_in_groups: :group_owners_can_manage_default_branch_protection + default_branch_protection_restriction_in_groups: :group_owners_can_manage_default_branch_protection, + group_ip_restriction: :globally_allowed_ips }.each do |license_feature, attribute_names| if License.feature_available?(license_feature) attrs += Array.wrap(attribute_names) diff --git a/ee/app/helpers/ee/application_settings_helper.rb b/ee/app/helpers/ee/application_settings_helper.rb index c56491748706dc..541ae6b86e45f3 100644 --- a/ee/app/helpers/ee/application_settings_helper.rb +++ b/ee/app/helpers/ee/application_settings_helper.rb @@ -110,6 +110,7 @@ def self.possible_licensed_attributes pypi_package_requests_forwarding maintenance_mode maintenance_mode_message + globally_allowed_ips ] end diff --git a/ee/app/models/ee/application_setting.rb b/ee/app/models/ee/application_setting.rb index f7cacd1cb7a41c..15f8c7d9c4a4c8 100644 --- a/ee/app/models/ee/application_setting.rb +++ b/ee/app/models/ee/application_setting.rb @@ -92,12 +92,14 @@ module ApplicationSetting validates :future_subscriptions, json_schema: { filename: 'future_subscriptions' } - validates :geo_node_allowed_ips, length: { maximum: 255 }, presence: true - validates :required_instance_ci_template, presence: true, allow_nil: true + validates :geo_node_allowed_ips, length: { maximum: 255 }, presence: true validate :check_geo_node_allowed_ips + validates :globally_allowed_ips, length: { maximum: 255 }, allow_blank: true + validate :check_globally_allowed_ips + validates :max_personal_access_token_lifetime, allow_blank: true, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: 365 } @@ -160,6 +162,7 @@ def defaults future_subscriptions: [], geo_node_allowed_ips: '0.0.0.0/0, ::/0', git_two_factor_session_expiry: 15, + globally_allowed_ips: '', lock_memberships_to_ldap: false, maintenance_mode: false, max_personal_access_token_lifetime: nil, @@ -439,6 +442,12 @@ def check_geo_node_allowed_ips errors.add(:geo_node_allowed_ips, e.message) end + def check_globally_allowed_ips + ::Gitlab::CIDR.new(globally_allowed_ips) + rescue ::Gitlab::CIDR::ValidationError => e + errors.add(:globally_allowed_ips, e.message) + end + def check_elasticsearch_url_scheme # ElasticSearch only exposes a RESTful API, hence we need # to use the HTTP protocol on all URLs. diff --git a/ee/app/views/admin/application_settings/_globally_allowed_ips.html.haml b/ee/app/views/admin/application_settings/_globally_allowed_ips.html.haml new file mode 100644 index 00000000000000..d422ba0e2d7249 --- /dev/null +++ b/ee/app/views/admin/application_settings/_globally_allowed_ips.html.haml @@ -0,0 +1,9 @@ +- return unless License.feature_available?(:group_ip_restriction) + +- f = local_assigns.fetch(:form) + +.form-group + = f.label :globally_allowed_ips, _('Globally-allowed IP ranges'), class: 'label-bold' + = f.text_field :globally_allowed_ips, class: 'form-control gl-form-input', placeholder: '10.0.0.0/8, 192.168.1.0/24', :'aria-describedby' => 'globally_allowed_ips_help_block' + %span.form-text.text-muted#globally_allowed_ips_help_block + = _('Specify IP ranges that are always allowed for inbound traffic, for use with group-level IP restrictions. Runner and Pages daemon internal IPs should be listed here so that they can access project artifacts.') diff --git a/ee/lib/gitlab/cidr.rb b/ee/lib/gitlab/cidr.rb index d43001cca2fa36..716c8dd5c78bfa 100644 --- a/ee/lib/gitlab/cidr.rb +++ b/ee/lib/gitlab/cidr.rb @@ -7,6 +7,8 @@ class CIDR attr_reader :cidrs + delegate :empty?, to: :cidrs + def initialize(values) @cidrs = parse_cidrs(values) end diff --git a/ee/lib/gitlab/ip_restriction/enforcer.rb b/ee/lib/gitlab/ip_restriction/enforcer.rb index fc475c2b179c49..cbd8a59e15c0a8 100644 --- a/ee/lib/gitlab/ip_restriction/enforcer.rb +++ b/ee/lib/gitlab/ip_restriction/enforcer.rb @@ -32,10 +32,33 @@ def allows_address?(address) allowed = root_ancestor_ip_restrictions.any? { |ip_restriction| ip_restriction.allows_address?(address) } - logger.info(message: 'Attempting to access IP restricted group', group_full_path: group.full_path, ip_address: address, allowed: allowed) + # Check against configured internal ranges + if allow_internal_addresses? && !allowed + allowed = true if internal_ip_ranges.match?(address) + end + + logger.info( + message: 'Attempting to access IP restricted group', + group_full_path: group.full_path, + ip_address: address, + allowed: allowed, + internal_allowed: allow_internal_addresses? + ) allowed end + + def allow_internal_addresses? + return false if internal_ip_ranges.empty? + + Feature.enabled?(:group_ip_restrictions_allow_global, group) + end + + def internal_ip_ranges + ip_ranges = Gitlab::CurrentSettings.globally_allowed_ips + + ::Gitlab::CIDR.new(ip_ranges) + end end end end diff --git a/ee/spec/controllers/admin/application_settings_controller_spec.rb b/ee/spec/controllers/admin/application_settings_controller_spec.rb index c4116b92a52610..d58192b1f02861 100644 --- a/ee/spec/controllers/admin/application_settings_controller_spec.rb +++ b/ee/spec/controllers/admin/application_settings_controller_spec.rb @@ -18,28 +18,30 @@ it 'updates the EE specific application settings' do settings = { - help_text: 'help_text', - repository_size_limit: 1024, - shared_runners_minutes: 60, - geo_status_timeout: 30, - check_namespace_plan: true, - authorized_keys_enabled: true, - slack_app_enabled: true, - slack_app_id: 'slack_app_id', - slack_app_secret: 'slack_app_secret', - slack_app_signing_secret: 'slack_app_signing_secret', - slack_app_verification_token: 'slack_app_verification_token', - allow_group_owners_to_manage_ldap: false, - lock_memberships_to_ldap: true, - geo_node_allowed_ips: '0.0.0.0/0, ::/0' + help_text: 'help_text', + repository_size_limit: 1024, + shared_runners_minutes: 60, + geo_status_timeout: 30, + check_namespace_plan: true, + authorized_keys_enabled: true, + slack_app_enabled: true, + slack_app_id: 'slack_app_id', + slack_app_secret: 'slack_app_secret', + slack_app_signing_secret: 'slack_app_signing_secret', + slack_app_verification_token: 'slack_app_verification_token', + allow_group_owners_to_manage_ldap: false, + lock_memberships_to_ldap: true, + geo_node_allowed_ips: '0.0.0.0/0, ::/0' } put :update, params: { application_setting: settings } expect(response).to redirect_to(general_admin_application_settings_path) + settings.except(:repository_size_limit).each do |setting, value| expect(ApplicationSetting.current.public_send(setting)).to eq(value) end + expect(ApplicationSetting.current.repository_size_limit).to eq(settings[:repository_size_limit].megabytes) end @@ -190,6 +192,13 @@ it_behaves_like 'settings for licensed features' end + context 'globally allowed IPs' do + let(:settings) { { globally_allowed_ips: '10.0.0.0/8, 192.168.1.0/24' } } + let(:feature) { :group_ip_restriction } + + it_behaves_like 'settings for licensed features' + end + context 'required instance ci template' do let(:settings) { { required_instance_ci_template: 'Auto-DevOps' } } let(:feature) { :required_ci_templates } diff --git a/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb b/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb index 134007426dc241..703248bf26fa7b 100644 --- a/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb +++ b/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb @@ -14,6 +14,9 @@ context 'with restriction' do before do + stub_feature_flags(group_ip_restrictions_allow_global: false) + stub_application_setting(globally_allowed_ips: "10.0.0.0/8, 192.168.0.0/24") + ranges.each do |range| create(:ip_restriction, group: group, range: range) end @@ -30,6 +33,27 @@ it { is_expected.to be_falsey } end + + context 'internal ranges feature is enabled' do + let(:current_ip) { '10.64.0.1' } + let(:ranges) { ['192.168.1.0/24'] } + + before do + stub_feature_flags(group_ip_restrictions_allow_global: true) + end + + context 'internal ranges are set' do + it { is_expected.to be_truthy } + end + + context 'internal ranges are not set' do + before do + stub_application_setting(globally_allowed_ips: "") + end + + it { is_expected.to be_falsey } + end + end end end diff --git a/ee/spec/models/application_setting_spec.rb b/ee/spec/models/application_setting_spec.rb index 208d90f4e2f904..2e2cefbb66b2d6 100644 --- a/ee/spec/models/application_setting_spec.rb +++ b/ee/spec/models/application_setting_spec.rb @@ -124,7 +124,7 @@ it { is_expected.not_to allow_value(nil).for(:secret_detection_revocation_token_types_url) } end - context 'when validating allowed_ips' do + context 'when validating geo_node_allowed_ips' do where(:allowed_ips, :is_valid) do "192.1.1.1" | true "192.1.1.0/24" | true @@ -145,6 +145,27 @@ end end + context 'when validating globally_allowed_ips' do + where(:allowed_ips, :is_valid) do + "192.1.1.1" | true + "192.1.1.0/24" | true + "192.1.1.0/24, 192.1.20.23" | true + "192.1.1.0/24, 192.23.0.0/16" | true + "192.1.1.0/34" | false + "192.1.1.257" | false + "192.1.1.257, 192.1.1.1" | false + "300.1.1.0/34" | false + end + + with_them do + specify do + setting.update_column(:globally_allowed_ips, allowed_ips) + + expect(setting.reload.valid?).to eq(is_valid) + end + end + end + context 'when validating elasticsearch_url' do where(:elasticsearch_url, :is_valid) do "http://es.localdomain" | true diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 96137675f033e2..081bf9683c7635 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -17432,6 +17432,9 @@ msgstr "" msgid "GlobalSearch|in project" msgstr "" +msgid "Globally-allowed IP ranges" +msgstr "" + msgid "Go Back" msgstr "" @@ -35937,6 +35940,9 @@ msgstr "" msgid "Specified URL cannot be used: \"%{reason}\"" msgstr "" +msgid "Specify IP ranges that are always allowed for inbound traffic, for use with group-level IP restrictions. Runner and Pages daemon internal IPs should be listed here so that they can access project artifacts." +msgstr "" + msgid "Specify an email address regex pattern to identify default internal users." msgstr "" -- GitLab From ebe3bad78f23f45c86be14e1a9e83e9b87025615 Mon Sep 17 00:00:00 2001 From: Robert May Date: Tue, 17 May 2022 09:38:47 +0100 Subject: [PATCH 2/9] Rename methods for clarity --- ee/lib/gitlab/ip_restriction/enforcer.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ee/lib/gitlab/ip_restriction/enforcer.rb b/ee/lib/gitlab/ip_restriction/enforcer.rb index cbd8a59e15c0a8..93499c10f6c875 100644 --- a/ee/lib/gitlab/ip_restriction/enforcer.rb +++ b/ee/lib/gitlab/ip_restriction/enforcer.rb @@ -33,8 +33,8 @@ def allows_address?(address) allowed = root_ancestor_ip_restrictions.any? { |ip_restriction| ip_restriction.allows_address?(address) } # Check against configured internal ranges - if allow_internal_addresses? && !allowed - allowed = true if internal_ip_ranges.match?(address) + if allow_globally_configured_addresses? && !allowed + allowed = true if globally_configured_ip_ranges.match?(address) end logger.info( @@ -42,19 +42,19 @@ def allows_address?(address) group_full_path: group.full_path, ip_address: address, allowed: allowed, - internal_allowed: allow_internal_addresses? + internal_allowed: allow_globally_configured_addresses? ) allowed end - def allow_internal_addresses? - return false if internal_ip_ranges.empty? + def allow_globally_configured_addresses? + return false if globally_configured_ip_ranges.empty? Feature.enabled?(:group_ip_restrictions_allow_global, group) end - def internal_ip_ranges + def globally_configured_ip_ranges ip_ranges = Gitlab::CurrentSettings.globally_allowed_ips ::Gitlab::CIDR.new(ip_ranges) -- GitLab From 6918630d9bd68a56caabcdb230fd70b53051dfa9 Mon Sep 17 00:00:00 2001 From: Robert May Date: Tue, 17 May 2022 10:51:09 +0100 Subject: [PATCH 3/9] Fix db structure error --- db/structure.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/structure.sql b/db/structure.sql index ef5c8ecc8271a5..6c9cac654e861e 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11305,7 +11305,7 @@ CREATE TABLE application_settings ( container_registry_pre_import_timeout integer DEFAULT 1800 NOT NULL, container_registry_import_timeout integer DEFAULT 600 NOT NULL, pipeline_limit_per_project_user_sha integer DEFAULT 0 NOT NULL, - globally_allowed_ips character varying(255) DEFAULT ''::character varying NOT NULL, + globally_allowed_ips text DEFAULT ''::text NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), -- GitLab From 3c1c5d02cbf2c2e5e7891f007642a92602c5aace Mon Sep 17 00:00:00 2001 From: Robert May Date: Tue, 17 May 2022 11:50:26 +0100 Subject: [PATCH 4/9] Add rollout issue link --- .../development/group_ip_restrictions_allow_global.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/development/group_ip_restrictions_allow_global.yml b/config/feature_flags/development/group_ip_restrictions_allow_global.yml index ee7e18a115db9a..fe8a4a7ddfb36f 100644 --- a/config/feature_flags/development/group_ip_restrictions_allow_global.yml +++ b/config/feature_flags/development/group_ip_restrictions_allow_global.yml @@ -1,7 +1,7 @@ --- name: group_ip_restrictions_allow_global introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87579 -rollout_issue_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/362603 milestone: '15.0' type: development group: group::source code -- GitLab From 61c89151a0709d8ea62533ab271ce69ea302d167 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 17 May 2022 18:18:36 +0000 Subject: [PATCH 5/9] Apply 3 suggestion(s) to 2 file(s) --- ...92207_add_globally_allowed_ips_to_application_setting.rb | 2 -- ee/lib/gitlab/ip_restriction/enforcer.rb | 6 ++---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/db/migrate/20220516092207_add_globally_allowed_ips_to_application_setting.rb b/db/migrate/20220516092207_add_globally_allowed_ips_to_application_setting.rb index 36f724cba60efb..895400aedc459d 100644 --- a/db/migrate/20220516092207_add_globally_allowed_ips_to_application_setting.rb +++ b/db/migrate/20220516092207_add_globally_allowed_ips_to_application_setting.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true # See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class AddGloballyAllowedIpsToApplicationSetting < Gitlab::Database::Migration[2.0] # rubocop:disable Migration/AddLimitToTextColumns # limit is added in 20220516123101_add_text_limit_to_globally_allowed_ips_on_application_settings diff --git a/ee/lib/gitlab/ip_restriction/enforcer.rb b/ee/lib/gitlab/ip_restriction/enforcer.rb index 93499c10f6c875..bc2da0fb36207a 100644 --- a/ee/lib/gitlab/ip_restriction/enforcer.rb +++ b/ee/lib/gitlab/ip_restriction/enforcer.rb @@ -42,7 +42,7 @@ def allows_address?(address) group_full_path: group.full_path, ip_address: address, allowed: allowed, - internal_allowed: allow_globally_configured_addresses? + globally_allowed: allow_globally_configured_addresses? ) allowed @@ -55,9 +55,7 @@ def allow_globally_configured_addresses? end def globally_configured_ip_ranges - ip_ranges = Gitlab::CurrentSettings.globally_allowed_ips - - ::Gitlab::CIDR.new(ip_ranges) + ::Gitlab::CIDR.new(Gitlab::CurrentSettings.globally_allowed_ips) end end end -- GitLab From d2d9f85a0745a3d32d45d61b71e85300535c9754 Mon Sep 17 00:00:00 2001 From: Robert May Date: Wed, 18 May 2022 09:59:54 +0100 Subject: [PATCH 6/9] Improve code coverage --- ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb b/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb index 703248bf26fa7b..b630ee233a01ce 100644 --- a/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb +++ b/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb @@ -39,7 +39,7 @@ let(:ranges) { ['192.168.1.0/24'] } before do - stub_feature_flags(group_ip_restrictions_allow_global: true) + stub_feature_flags(group_ip_restrictions_allow_global: group) end context 'internal ranges are set' do -- GitLab From 72baab08176f3008c746ee272300cec183bded65 Mon Sep 17 00:00:00 2001 From: Robert May Date: Wed, 18 May 2022 10:02:07 +0100 Subject: [PATCH 7/9] Update context name --- ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb b/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb index b630ee233a01ce..06205d219561c9 100644 --- a/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb +++ b/ee/spec/lib/ee/gitlab/ip_restriction/enforcer_spec.rb @@ -34,7 +34,7 @@ it { is_expected.to be_falsey } end - context 'internal ranges feature is enabled' do + context 'global allowlist feature is enabled' do let(:current_ip) { '10.64.0.1' } let(:ranges) { ['192.168.1.0/24'] } @@ -42,11 +42,11 @@ stub_feature_flags(group_ip_restrictions_allow_global: group) end - context 'internal ranges are set' do + context 'global ranges are set' do it { is_expected.to be_truthy } end - context 'internal ranges are not set' do + context 'global ranges are not set' do before do stub_application_setting(globally_allowed_ips: "") end -- GitLab From bc1b945524d2d451d9127770365d1138f9a94104 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 18 May 2022 14:35:02 +0000 Subject: [PATCH 8/9] Apply 1 suggestion(s) to 1 file(s) --- ...xt_limit_to_globally_allowed_ips_on_application_settings.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/db/migrate/20220516123101_add_text_limit_to_globally_allowed_ips_on_application_settings.rb b/db/migrate/20220516123101_add_text_limit_to_globally_allowed_ips_on_application_settings.rb index f49987167e295a..887a7da0a741ca 100644 --- a/db/migrate/20220516123101_add_text_limit_to_globally_allowed_ips_on_application_settings.rb +++ b/db/migrate/20220516123101_add_text_limit_to_globally_allowed_ips_on_application_settings.rb @@ -1,8 +1,5 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class AddTextLimitToGloballyAllowedIpsOnApplicationSettings < Gitlab::Database::Migration[2.0] disable_ddl_transaction! -- GitLab From ab58352076c89ee73cb91b32f0c597e553039bbf Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 18 May 2022 17:25:34 +0000 Subject: [PATCH 9/9] Update milestone --- .../development/group_ip_restrictions_allow_global.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/development/group_ip_restrictions_allow_global.yml b/config/feature_flags/development/group_ip_restrictions_allow_global.yml index fe8a4a7ddfb36f..87cfa5e8b1b59d 100644 --- a/config/feature_flags/development/group_ip_restrictions_allow_global.yml +++ b/config/feature_flags/development/group_ip_restrictions_allow_global.yml @@ -2,7 +2,7 @@ name: group_ip_restrictions_allow_global introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87579 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/362603 -milestone: '15.0' +milestone: '15.1' type: development group: group::source code default_enabled: false -- GitLab