diff options
| -rw-r--r-- | lib/api/api.rb | 5 | ||||
| -rw-r--r-- | lib/gitlab/github_import/client.rb | 6 | ||||
| -rw-r--r-- | lib/gitlab/url_blocker.rb | 4 | ||||
| -rw-r--r-- | lib/tasks/import.rake | 6 | ||||
| -rw-r--r-- | qa/.gitignore | 1 | ||||
| -rwxr-xr-x | qa/bin/qa | 2 | ||||
| -rw-r--r-- | qa/qa.rb | 1 | ||||
| -rw-r--r-- | qa/qa/page/mattermost/login.rb | 2 | ||||
| -rw-r--r-- | qa/qa/page/mattermost/main.rb | 2 | ||||
| -rw-r--r-- | qa/qa/runtime/scenario.rb | 22 | ||||
| -rw-r--r-- | qa/qa/scenario/bootable.rb | 47 | ||||
| -rw-r--r-- | qa/qa/scenario/entrypoint.rb | 2 | ||||
| -rw-r--r-- | qa/qa/scenario/test/integration/mattermost.rb | 3 | ||||
| -rw-r--r-- | qa/spec/runtime/scenario_spec.rb | 27 | ||||
| -rw-r--r-- | qa/spec/scenario/bootable_spec.rb | 24 | ||||
| -rw-r--r-- | spec/lib/gitlab/github_import/client_spec.rb | 27 | ||||
| -rw-r--r-- | spec/lib/gitlab/url_blocker_spec.rb | 16 | ||||
| -rw-r--r-- | spec/requests/api/projects_spec.rb | 6 | ||||
| -rw-r--r-- | spec/support/matchers/security_header_matcher.rb | 5 |
19 files changed, 198 insertions, 10 deletions
diff --git a/lib/api/api.rb b/lib/api/api.rb index c37e596eb9d..8094597d238 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -61,7 +61,10 @@ module API mount ::API::V3::Variables end - before { header['X-Frame-Options'] = 'SAMEORIGIN' } + before do + header['X-Frame-Options'] = 'SAMEORIGIN' + header['X-Content-Type-Options'] = 'nosniff' + end # The locale is set to the current user's locale when `current_user` is loaded after { Gitlab::I18n.use_default_locale } diff --git a/lib/gitlab/github_import/client.rb b/lib/gitlab/github_import/client.rb index c1c338487a7..5da9befa08e 100644 --- a/lib/gitlab/github_import/client.rb +++ b/lib/gitlab/github_import/client.rb @@ -129,6 +129,8 @@ module Gitlab # whether we are running in parallel mode or not. For more information see # `#rate_or_wait_for_rate_limit`. def with_rate_limit + return yield unless rate_limiting_enabled? + request_count_counter.increment raise_or_wait_for_rate_limit unless requests_remaining? @@ -170,6 +172,10 @@ module Gitlab octokit.rate_limit.resets_in + 5 end + def rate_limiting_enabled? + @rate_limiting_enabled ||= api_endpoint.include?('.github.com') + end + def api_endpoint custom_api_endpoint || default_api_endpoint end diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index fee1a127fd7..13150ddab67 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -22,10 +22,12 @@ module Gitlab return true if blocked_user_or_hostname?(uri.user) return true if blocked_user_or_hostname?(uri.hostname) - server_ips = Resolv.getaddresses(uri.hostname) + server_ips = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM).map(&:ip_address) return true if (blocked_ips & server_ips).any? rescue Addressable::URI::InvalidURIError return true + rescue SocketError + return false end false diff --git a/lib/tasks/import.rake b/lib/tasks/import.rake index 943cbe6d80c..aafbe52e5f8 100644 --- a/lib/tasks/import.rake +++ b/lib/tasks/import.rake @@ -101,8 +101,8 @@ end class GithubRepos def initialize(token, current_user, github_repo) - @client = Octokit::Client - .new(access_token: token, auto_paginate: true, per_page: 100) + @client = Gitlab::GithubImport::Client.new(token) + @client.octokit.auto_paginate = true @current_user = current_user @github_repo = github_repo @@ -130,7 +130,7 @@ class GithubRepos end def repos - @client.list_repositories + @client.octokit.list_repositories end end diff --git a/qa/.gitignore b/qa/.gitignore index 3fec32c8427..19ec17d0005 100644 --- a/qa/.gitignore +++ b/qa/.gitignore @@ -1 +1,2 @@ tmp/ +.ruby-version diff --git a/qa/bin/qa b/qa/bin/qa index cecdeac14db..f1704dc54e9 100755 --- a/qa/bin/qa +++ b/qa/bin/qa @@ -4,4 +4,4 @@ require_relative '../qa' QA::Scenario .const_get(ARGV.shift) - .perform(*ARGV) + .launch!(*ARGV) @@ -18,6 +18,7 @@ module QA ## # Support files # + autoload :Bootable, 'qa/scenario/bootable' autoload :Actable, 'qa/scenario/actable' autoload :Entrypoint, 'qa/scenario/entrypoint' autoload :Template, 'qa/scenario/template' diff --git a/qa/qa/page/mattermost/login.rb b/qa/qa/page/mattermost/login.rb index 2001dc5b230..42ab9c6f675 100644 --- a/qa/qa/page/mattermost/login.rb +++ b/qa/qa/page/mattermost/login.rb @@ -3,7 +3,7 @@ module QA module Mattermost class Login < Page::Base def initialize - visit(Runtime::Scenario.mattermost + '/login') + visit(Runtime::Scenario.mattermost_address + '/login') end def sign_in_using_oauth diff --git a/qa/qa/page/mattermost/main.rb b/qa/qa/page/mattermost/main.rb index e636d7676f4..4b8fc28e53f 100644 --- a/qa/qa/page/mattermost/main.rb +++ b/qa/qa/page/mattermost/main.rb @@ -3,7 +3,7 @@ module QA module Mattermost class Main < Page::Base def initialize - visit(Runtime::Scenario.mattermost) + visit(Runtime::Scenario.mattermost_address) end end end diff --git a/qa/qa/runtime/scenario.rb b/qa/qa/runtime/scenario.rb index 0c5e9787e17..7ef59046640 100644 --- a/qa/qa/runtime/scenario.rb +++ b/qa/qa/runtime/scenario.rb @@ -1,8 +1,28 @@ module QA module Runtime + ## + # Singleton approach to global test scenario arguments. + # module Scenario extend self - attr_accessor :mattermost + + attr_reader :attributes + + def define(attribute, value) + (@attributes ||= {}).store(attribute.to_sym, value) + + define_singleton_method(attribute) do + @attributes[attribute.to_sym].tap do |value| + if value.to_s.empty? + raise ArgumentError, "Empty `#{attribute}` attribute!" + end + end + end + end + + def method_missing(name, *) + raise ArgumentError, "Scenario attribute `#{name}` not defined!" + end end end end diff --git a/qa/qa/scenario/bootable.rb b/qa/qa/scenario/bootable.rb new file mode 100644 index 00000000000..22496bcc2fc --- /dev/null +++ b/qa/qa/scenario/bootable.rb @@ -0,0 +1,47 @@ +require 'optparse' + +module QA + module Scenario + module Bootable + Option = Struct.new(:name, :arg, :desc) + + def self.included(base) + base.extend(ClassMethods) + end + + module ClassMethods + def launch!(argv) + arguments = OptionParser.new do |parser| + options.to_a.each do |opt| + parser.on(opt.arg, opt.desc) do |value| + Runtime::Scenario.define(opt.name, value) + end + end + end + + arguments.parse!(argv) + + if has_attributes? + self.perform(**Runtime::Scenario.attributes) + else + self.perform(*argv) + end + end + + private + + def attribute(name, arg, desc) + options.push(Option.new(name, arg, desc)) + end + + def options + @options ||= [] + end + + def has_attributes? + options.any? + end + end + end + end +end diff --git a/qa/qa/scenario/entrypoint.rb b/qa/qa/scenario/entrypoint.rb index 33cb2696f8f..8f708fe38ab 100644 --- a/qa/qa/scenario/entrypoint.rb +++ b/qa/qa/scenario/entrypoint.rb @@ -5,6 +5,8 @@ module QA # including staging and on-premises installation. # class Entrypoint < Template + include Bootable + def self.tags(*tags) @tags = tags end diff --git a/qa/qa/scenario/test/integration/mattermost.rb b/qa/qa/scenario/test/integration/mattermost.rb index 0220da929ce..7d0702afdb1 100644 --- a/qa/qa/scenario/test/integration/mattermost.rb +++ b/qa/qa/scenario/test/integration/mattermost.rb @@ -10,7 +10,8 @@ module QA tags :core, :mattermost def perform(address, mattermost, *files) - Runtime::Scenario.mattermost = mattermost + Runtime::Scenario.define(:mattermost_address, mattermost) + super(address, *files) end end diff --git a/qa/spec/runtime/scenario_spec.rb b/qa/spec/runtime/scenario_spec.rb new file mode 100644 index 00000000000..7009192bcc0 --- /dev/null +++ b/qa/spec/runtime/scenario_spec.rb @@ -0,0 +1,27 @@ +describe QA::Runtime::Scenario do + subject do + Module.new.extend(described_class) + end + + it 'makes it possible to define global scenario attributes' do + subject.define(:my_attribute, 'some-value') + subject.define(:another_attribute, 'another-value') + + expect(subject.my_attribute).to eq 'some-value' + expect(subject.another_attribute).to eq 'another-value' + expect(subject.attributes) + .to eq(my_attribute: 'some-value', another_attribute: 'another-value') + end + + it 'raises error when attribute is not known' do + expect { subject.invalid_accessor } + .to raise_error ArgumentError, /invalid_accessor/ + end + + it 'raises error when attribute is empty' do + subject.define(:empty_attribute, '') + + expect { subject.empty_attribute } + .to raise_error ArgumentError, /empty_attribute/ + end +end diff --git a/qa/spec/scenario/bootable_spec.rb b/qa/spec/scenario/bootable_spec.rb new file mode 100644 index 00000000000..273aac7677e --- /dev/null +++ b/qa/spec/scenario/bootable_spec.rb @@ -0,0 +1,24 @@ +describe QA::Scenario::Bootable do + subject do + Class.new(QA::Scenario::Template) + .include(described_class) + end + + it 'makes it possible to define the scenario attribute' do + subject.class_eval do + attribute :something, '--something SOMETHING', 'Some attribute' + attribute :another, '--another ANOTHER', 'Some other attribute' + end + + expect(subject).to receive(:perform) + .with(something: 'test', another: 'other') + + subject.launch!(%w[--another other --something test]) + end + + it 'does not require attributes to be defined' do + expect(subject).to receive(:perform).with('some', 'argv') + + subject.launch!(%w[some argv]) + end +end diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb index 9cbd9bcc14e..5b2642d9473 100644 --- a/spec/lib/gitlab/github_import/client_spec.rb +++ b/spec/lib/gitlab/github_import/client_spec.rb @@ -185,6 +185,17 @@ describe Gitlab::GithubImport::Client do client.with_rate_limit { } end + + it 'ignores rate limiting when disabled' do + expect(client) + .to receive(:rate_limiting_enabled?) + .and_return(false) + + expect(client) + .not_to receive(:requests_remaining?) + + expect(client.with_rate_limit { 10 }).to eq(10) + end end describe '#requests_remaining?' do @@ -362,4 +373,20 @@ describe Gitlab::GithubImport::Client do end end end + + describe '#rate_limiting_enabled?' do + let(:client) { described_class.new('foo') } + + it 'returns true when using GitHub.com' do + expect(client.rate_limiting_enabled?).to eq(true) + end + + it 'returns false for GitHub enterprise installations' do + expect(client) + .to receive(:api_endpoint) + .and_return('https://github.kittens.com/') + + expect(client.rate_limiting_enabled?).to eq(false) + end + end end diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index f18823b61ef..d9b3c2350b1 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -20,6 +20,22 @@ describe Gitlab::UrlBlocker do expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true end + it 'returns true for alternative version of 127.0.0.1 (0177.1)' do + expect(described_class.blocked_url?('https://0177.1:65535/foo/foo.git')).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (0x7f.1)' do + expect(described_class.blocked_url?('https://0x7f.1:65535/foo/foo.git')).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (2130706433)' do + expect(described_class.blocked_url?('https://2130706433:65535/foo/foo.git')).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (127.000.000.001)' do + expect(described_class.blocked_url?('https://127.000.000.001:65535/foo/foo.git')).to be true + end + it 'returns true for a non-alphanumeric hostname' do stub_resolv diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index abe367d4e11..50f6c8b7d64 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -50,6 +50,12 @@ describe API::Projects do expect(json_response).to be_an Array expect(json_response.map { |p| p['id'] }).to contain_exactly(*projects.map(&:id)) end + + it 'returns the proper security headers' do + get api('/projects', current_user), filter + + expect(response).to include_security_headers + end end shared_examples_for 'projects response without N + 1 queries' do diff --git a/spec/support/matchers/security_header_matcher.rb b/spec/support/matchers/security_header_matcher.rb new file mode 100644 index 00000000000..f8518d13ebb --- /dev/null +++ b/spec/support/matchers/security_header_matcher.rb @@ -0,0 +1,5 @@ +RSpec::Matchers.define :include_security_headers do |expected| + match do |actual| + expect(actual.headers).to include('X-Content-Type-Options') + end +end |