diff options
author | Francisco Javier López <fjlopez@gitlab.com> | 2018-06-11 16:29:37 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2018-06-11 16:29:37 +0300 |
commit | 1418afc2d6e7699f08a1fc5f33b78ea847ac1451 (patch) | |
tree | 7f1cd2621237c4dd234651bd16d6e304989b731d /spec | |
parent | 180dc237152d60d05e4f75d8c936e81ba783b6cd (diff) |
Avoid checking the user format in every url validation
Diffstat (limited to 'spec')
-rw-r--r-- | spec/lib/gitlab/url_blocker_spec.rb | 46 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 11 | ||||
-rw-r--r-- | spec/models/remote_mirror_spec.rb | 7 | ||||
-rw-r--r-- | spec/validators/url_validator_spec.rb | 53 |
4 files changed, 96 insertions, 21 deletions
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index 81dbbb962dd..6f5f9938eca 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -58,20 +58,6 @@ describe Gitlab::UrlBlocker do end end - it 'returns true for a non-alphanumeric username' do - stub_resolv - - aggregate_failures do - expect(described_class).to be_blocked_url('ssh://-oProxyCommand=whoami@example.com/a') - - # The leading character here is a Unicode "soft hyphen" - expect(described_class).to be_blocked_url('ssh://oProxyCommand=whoami@example.com/a') - - # Unicode alphanumerics are allowed - expect(described_class).not_to be_blocked_url('ssh://ğitlab@example.com/a') - end - end - it 'returns true for invalid URL' do expect(described_class.blocked_url?('http://:8080')).to be true end @@ -120,6 +106,38 @@ describe Gitlab::UrlBlocker do allow(Addrinfo).to receive(:getaddrinfo).and_call_original end end + + context 'when enforce_user is' do + before do + stub_resolv + end + + context 'false (default)' do + it 'does not block urls with a non-alphanumeric username' do + expect(described_class).not_to be_blocked_url('ssh://-oProxyCommand=whoami@example.com/a') + + # The leading character here is a Unicode "soft hyphen" + expect(described_class).not_to be_blocked_url('ssh://oProxyCommand=whoami@example.com/a') + + # Unicode alphanumerics are allowed + expect(described_class).not_to be_blocked_url('ssh://ğitlab@example.com/a') + end + end + + context 'true' do + it 'blocks urls with a non-alphanumeric username' do + aggregate_failures do + expect(described_class).to be_blocked_url('ssh://-oProxyCommand=whoami@example.com/a', enforce_user: true) + + # The leading character here is a Unicode "soft hyphen" + expect(described_class).to be_blocked_url('ssh://oProxyCommand=whoami@example.com/a', enforce_user: true) + + # Unicode alphanumerics are allowed + expect(described_class).not_to be_blocked_url('ssh://ğitlab@example.com/a', enforce_user: true) + end + end + end + end end # Resolv does not support resolving UTF-8 domain names diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1a6ad3edd78..b9a9c4ebf42 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -238,20 +238,27 @@ describe Project do expect(project2.import_data).to be_nil end - it "does not allow blocked import_url localhost" do + it "does not allow import_url pointing to localhost" do project2 = build(:project, import_url: 'http://localhost:9000/t.git') expect(project2).to be_invalid expect(project2.errors[:import_url].first).to include('Requests to localhost are not allowed') end - it "does not allow blocked import_url port" do + it "does not allow import_url with invalid ports" do project2 = build(:project, import_url: 'http://github.com:25/t.git') expect(project2).to be_invalid expect(project2.errors[:import_url].first).to include('Only allowed ports are 22, 80, 443') end + it "does not allow import_url with invalid user" do + project2 = build(:project, import_url: 'http://$user:password@github.com/t.git') + + expect(project2).to be_invalid + expect(project2.errors[:import_url].first).to include('Username needs to start with an alphanumeric character') + end + describe 'project pending deletion' do let!(:project_pending_deletion) do create(:project, diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 1d94abe4195..4c086eeadfc 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -15,6 +15,13 @@ describe RemoteMirror do expect(remote_mirror).not_to be_valid end + + it 'does not allow url with an invalid user' do + remote_mirror = build(:remote_mirror, url: 'http://$user:password@invalid.invalid') + + expect(remote_mirror).to be_invalid + expect(remote_mirror.errors[:url].first).to include('Username needs to start with an alphanumeric character') + end end end diff --git a/spec/validators/url_validator_spec.rb b/spec/validators/url_validator_spec.rb index 2d719263fc8..93fe013d11c 100644 --- a/spec/validators/url_validator_spec.rb +++ b/spec/validators/url_validator_spec.rb @@ -50,13 +50,56 @@ describe UrlValidator do end end - context 'when ports is set' do - let(:validator) { described_class.new(attributes: [:link_url], ports: [443]) } + context 'when ports is' do + let(:validator) { described_class.new(attributes: [:link_url], ports: ports) } - it 'blocks urls with a different port' do - subject + context 'empty' do + let(:ports) { [] } - expect(badge.errors.empty?).to be false + it 'does not block any port' do + subject + + expect(badge.errors.empty?).to be true + end + end + + context 'set' do + let(:ports) { [443] } + + it 'blocks urls with a different port' do + subject + + expect(badge.errors.empty?).to be false + end + end + end + + context 'when enforce_user is' do + let(:url) { 'http://$user@example.com'} + let(:validator) { described_class.new(attributes: [:link_url], enforce_user: enforce_user) } + + context 'true' do + let(:enforce_user) { true } + + it 'checks user format' do + badge.link_url = url + + subject + + expect(badge.errors.empty?).to be false + end + end + + context 'false (default)' do + let(:enforce_user) { false } + + it 'does not check user format' do + badge.link_url = url + + subject + + expect(badge.errors.empty?).to be true + end end end end |