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 e3c044ff979d4a22335019bece7c5d76463c08d0..cc62ca354d1da04e4d282d7e0ddabfae614be9e6 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 0000000000000000000000000000000000000000..87cfa5e8b1b59d9a177479eb7896aca6bbad905e --- /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: https://gitlab.com/gitlab-org/gitlab/-/issues/362603 +milestone: '15.1' +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 0000000000000000000000000000000000000000..895400aedc459d0fd53615d67c44707ef66976d8 --- /dev/null +++ b/db/migrate/20220516092207_add_globally_allowed_ips_to_application_setting.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +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 0000000000000000000000000000000000000000..887a7da0a741ca0efc629646751a4fc77e1d070a --- /dev/null +++ b/db/migrate/20220516123101_add_text_limit_to_globally_allowed_ips_on_application_settings.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +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 0000000000000000000000000000000000000000..8731d03e39e0150de47a4904c0d9bbfc2b68d0d4 --- /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 0000000000000000000000000000000000000000..ea64cc0ea46a93b377c727ee7cd928c19d33b07d --- /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 fa9d9e8f778f3566e38ac4802215b4ac7a2be678..6c9cac654e861eb9372b67ffda01a422e72ab10f 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 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)), @@ -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 0d86f004b9f62238eef47bbdd8b53baeee2605d2..636cb675f4d96153345881f67df17d81d7ed0c4b 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 c56491748706dc339179f2cd2594668aef0c0433..541ae6b86e45f3010e34c6dbf0442196603a7b85 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 f7cacd1cb7a41caf6951843fb32815577aa43887..15f8c7d9c4a4c877fef34d5ede8d66de0fdf35dc 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 0000000000000000000000000000000000000000..d422ba0e2d72497ca792217f7e79fdbc5a5141ea --- /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 d43001cca2fa3655d1195f20f9cf4b1734c08594..716c8dd5c78bfad0b77dde0a65a459142fc35613 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 fc475c2b179c4933671dc39609780413eca47a93..bc2da0fb36207a66859273a2a12bb8d37bd401d1 100644 --- a/ee/lib/gitlab/ip_restriction/enforcer.rb +++ b/ee/lib/gitlab/ip_restriction/enforcer.rb @@ -32,10 +32,31 @@ 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_globally_configured_addresses? && !allowed + allowed = true if globally_configured_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, + globally_allowed: allow_globally_configured_addresses? + ) allowed end + + def allow_globally_configured_addresses? + return false if globally_configured_ip_ranges.empty? + + Feature.enabled?(:group_ip_restrictions_allow_global, group) + end + + def globally_configured_ip_ranges + ::Gitlab::CIDR.new(Gitlab::CurrentSettings.globally_allowed_ips) + 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 c4116b92a5261048cf2b78e29cb068af7c3b20e0..d58192b1f02861a26ab427f2bc8744cf7bdad2cb 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 134007426dc2410dc1feca38bf23ba74b9c8ba08..06205d219561c9b921995150eb57eeebe313c5c5 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 'global allowlist 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: group) + end + + context 'global ranges are set' do + it { is_expected.to be_truthy } + end + + context 'global 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 208d90f4e2f9041fede603adb022f3f50b2febd3..2e2cefbb66b2d67bd1342a83b2a6ce2134e9fb44 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 96137675f033e2c8008ea6af0c54e0d6e7ec1af9..081bf9683c763572406cb05ff841c549a3ee253a 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 ""