diff options
| -rw-r--r-- | app/models/application_setting.rb | 6 | ||||
| -rw-r--r-- | app/models/application_setting_implementation.rb | 4 | ||||
| -rw-r--r-- | app/validators/qualified_domain_array_validator.rb | 4 | ||||
| -rw-r--r-- | app/views/admin/application_settings/_outbound.html.haml | 2 | ||||
| -rw-r--r-- | locale/gitlab.pot | 6 | ||||
| -rw-r--r-- | spec/models/application_setting_spec.rb | 2 | ||||
| -rw-r--r-- | spec/support/shared_examples/application_setting_examples.rb | 11 | ||||
| -rw-r--r-- | spec/validators/qualified_domain_array_validator_spec.rb | 30 |
8 files changed, 36 insertions, 29 deletions
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index a769a8f07fd..9dbcef8abaa 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -42,9 +42,9 @@ class ApplicationSetting < ApplicationRecord validates :uuid, presence: true validates :outbound_local_requests_whitelist, - length: { maximum: 1_000, message: N_('is too long (maximum is 1000 entries)') } - - validates :outbound_local_requests_whitelist, qualified_domain_array: true, allow_blank: true + length: { maximum: 1_000, message: N_('is too long (maximum is 1000 entries)') }, + allow_nil: false, + qualified_domain_array: true validates :session_expire_delay, presence: true, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 1e612bd0e78..6efd07a6008 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -163,6 +163,8 @@ module ApplicationSettingImplementation def outbound_local_requests_whitelist_arrays strong_memoize(:outbound_local_requests_whitelist_arrays) do + next [[], []] unless self.outbound_local_requests_whitelist + ip_whitelist = [] domain_whitelist = [] @@ -284,6 +286,8 @@ module ApplicationSettingImplementation end def domain_strings_to_array(values) + return [] unless values + values .split(DOMAIN_LIST_SEPARATOR) .reject(&:empty?) diff --git a/app/validators/qualified_domain_array_validator.rb b/app/validators/qualified_domain_array_validator.rb index 986c146a9db..c3a79d21ac0 100644 --- a/app/validators/qualified_domain_array_validator.rb +++ b/app/validators/qualified_domain_array_validator.rb @@ -23,9 +23,9 @@ class QualifiedDomainArrayValidator < ActiveModel::EachValidator private def validate_value_present(record, attribute, value) - return unless value.blank? + return unless value.nil? - record.errors.add(attribute, _('entries cannot be blank')) + record.errors.add(attribute, _('entries cannot be nil')) end def validate_host_length(record, attribute, value) diff --git a/app/views/admin/application_settings/_outbound.html.haml b/app/views/admin/application_settings/_outbound.html.haml index e58bb526c11..4fecdb59e1d 100644 --- a/app/views/admin/application_settings/_outbound.html.haml +++ b/app/views/admin/application_settings/_outbound.html.haml @@ -13,7 +13,7 @@ = _('Whitelist to allow requests to the local network from hooks and services') = f.text_area :outbound_local_requests_whitelist_raw, placeholder: "example.com, 192.168.1.1", class: 'form-control', rows: 8 %span.form-text.text-muted - = _('Requests to these domain(s)/address(es) on the local network will be allowed when local requests from hooks and services are disabled. IP ranges such as 1:0:0:0:0:0:0:0/124 or 127.0.0.0/28 are supported. Domain wildcards are not supported currently. Use comma, semicolon, or newline to separate multiple entries. The whitelist can hold a maximum of 4000 entries. Domains should use IDNA encoding. Ex: domain.com, 192.168.1.1, 127.0.0.0/28.') + = _('Requests to these domain(s)/address(es) on the local network will be allowed when local requests from hooks and services are not allowed. IP ranges such as 1:0:0:0:0:0:0:0/124 or 127.0.0.0/28 are supported. Domain wildcards are not supported currently. Use comma, semicolon, or newline to separate multiple entries. The whitelist can hold a maximum of 1000 entries. Domains should use IDNA encoding. Ex: example.com, 192.168.1.1, 127.0.0.0/28, xn--itlab-j1a.com.') .form-group .form-check diff --git a/locale/gitlab.pot b/locale/gitlab.pot index feae99fb95a..878ee5bc344 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9238,7 +9238,7 @@ msgstr "" msgid "Requests Profiles" msgstr "" -msgid "Requests to these domain(s)/address(es) on the local network will be allowed when local requests from hooks and services are disabled. IP ranges such as 1:0:0:0:0:0:0:0/124 or 127.0.0.0/28 are supported. Domain wildcards are not supported currently. Use comma, semicolon, or newline to separate multiple entries. The whitelist can hold a maximum of 4000 entries. Domains should use IDNA encoding. Ex: domain.com, 192.168.1.1, 127.0.0.0/28." +msgid "Requests to these domain(s)/address(es) on the local network will be allowed when local requests from hooks and services are not allowed. IP ranges such as 1:0:0:0:0:0:0:0/124 or 127.0.0.0/28 are supported. Domain wildcards are not supported currently. Use comma, semicolon, or newline to separate multiple entries. The whitelist can hold a maximum of 1000 entries. Domains should use IDNA encoding. Ex: example.com, 192.168.1.1, 127.0.0.0/28, xn--itlab-j1a.com." msgstr "" msgid "Require all users in this group to setup Two-factor authentication" @@ -13137,10 +13137,10 @@ msgstr "" msgid "encrypted: needs to be a :required, :optional or :migrating!" msgstr "" -msgid "entries cannot be blank" +msgid "entries cannot be larger than 255 characters" msgstr "" -msgid "entries cannot be larger than 255 characters" +msgid "entries cannot be nil" msgstr "" msgid "entries cannot contain HTML tags" diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index bd87bbd8d68..db80b85360f 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -45,7 +45,7 @@ describe ApplicationSetting do it { is_expected.to allow_value(['xn--itlab-j1a.com']).for(:outbound_local_requests_whitelist) } it { is_expected.not_to allow_value(['<h1></h1>']).for(:outbound_local_requests_whitelist) } it { is_expected.to allow_value(['gitlab.com']).for(:outbound_local_requests_whitelist) } - it { is_expected.to allow_value(nil).for(:outbound_local_requests_whitelist) } + it { is_expected.not_to allow_value(nil).for(:outbound_local_requests_whitelist) } it { is_expected.to allow_value([]).for(:outbound_local_requests_whitelist) } context "when user accepted let's encrypt terms of service" do diff --git a/spec/support/shared_examples/application_setting_examples.rb b/spec/support/shared_examples/application_setting_examples.rb index 2c600785ad3..0e6a8059376 100644 --- a/spec/support/shared_examples/application_setting_examples.rb +++ b/spec/support/shared_examples/application_setting_examples.rb @@ -40,6 +40,11 @@ RSpec.shared_examples 'string of domains' do |attribute| setting.method("#{attribute}_raw=").call("example;34543:garbage:fdh5654;") expect(setting.method(attribute).call).to contain_exactly('example', '34543:garbage:fdh5654') end + + it 'does not raise error with nil' do + setting.method("#{attribute}_raw=").call(nil) + expect(setting.method(attribute).call).to eq([]) + end end RSpec.shared_examples 'application settings examples' do @@ -75,6 +80,12 @@ RSpec.shared_examples 'application settings examples' do expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(ip_whitelist, domain_whitelist) end + + it 'does not raise error with nil' do + setting.outbound_local_requests_whitelist = nil + + expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly([], []) + end end describe 'usage ping settings' do diff --git a/spec/validators/qualified_domain_array_validator_spec.rb b/spec/validators/qualified_domain_array_validator_spec.rb index a96b00bfd1d..6beb4c67f6f 100644 --- a/spec/validators/qualified_domain_array_validator_spec.rb +++ b/spec/validators/qualified_domain_array_validator_spec.rb @@ -19,20 +19,19 @@ describe QualifiedDomainArrayValidator do subject { validator.validate(record) } - shared_examples 'cannot be blank' do - it 'returns error when attribute is blank' do - record.domain_array = [] + shared_examples 'can be nil' do + it 'allows when attribute is nil' do + record.domain_array = nil subject - expect(record.errors).to be_present - expect(record.errors.first[1]).to eq 'entries cannot be blank' + expect(record.errors).to be_empty end end - shared_examples 'can be nil' do - it 'allows when attribute is nil' do - record.domain_array = nil + shared_examples 'can be blank' do + it 'allows when attribute is blank' do + record.domain_array = [] subject @@ -43,7 +42,7 @@ describe QualifiedDomainArrayValidator do describe 'validations' do let(:validator) { described_class.new(attributes: [:domain_array]) } - it_behaves_like 'cannot be blank' + it_behaves_like 'can be blank' it 'returns error when attribute is nil' do record.domain_array = nil @@ -51,6 +50,7 @@ describe QualifiedDomainArrayValidator do subject expect(record.errors).to be_present + expect(record.errors.first[1]).to eq('entries cannot be nil') end it 'allows when domain is valid' do @@ -91,21 +91,13 @@ describe QualifiedDomainArrayValidator do let(:validator) { described_class.new(attributes: [:domain_array], allow_nil: true) } it_behaves_like 'can be nil' - - it_behaves_like 'cannot be blank' + it_behaves_like 'can be blank' end context 'when allow_blank is set to true' do let(:validator) { described_class.new(attributes: [:domain_array], allow_blank: true) } it_behaves_like 'can be nil' - - it 'allows when attribute is blank' do - record.domain_array = [] - - subject - - expect(record.errors).to be_empty - end + it_behaves_like 'can be blank' end end |