New upstream version 10.5.6+dfsg
This commit is contained in:
parent
0aa07055e0
commit
2058b5ccaf
47 changed files with 544 additions and 133 deletions
|
@ -33,3 +33,6 @@ Gitlab/ModuleWithInstanceVariables:
|
|||
# We ignore spec helpers because it usually doesn't matter
|
||||
- spec/support/**/*.rb
|
||||
- features/steps/**/*.rb
|
||||
|
||||
Gitlab/HTTParty:
|
||||
Enabled: true
|
||||
|
|
|
@ -2,6 +2,14 @@
|
|||
documentation](doc/development/changelog.md) for instructions on adding your own
|
||||
entry.
|
||||
|
||||
## 10.5.6 (2018-03-16)
|
||||
|
||||
### Security (2 changes)
|
||||
|
||||
- Fixed some SSRF vulnerabilities in services, hooks and integrations. !2337
|
||||
- Fix GitLab Auth0 integration signing in the wrong user.
|
||||
|
||||
|
||||
## 10.5.5 (2018-03-15)
|
||||
|
||||
### Fixed (3 changes)
|
||||
|
|
2
Gemfile
2
Gemfile
|
@ -25,7 +25,7 @@ gem 'devise', '~> 4.2'
|
|||
gem 'doorkeeper', '~> 4.2.0'
|
||||
gem 'doorkeeper-openid_connect', '~> 1.2.0'
|
||||
gem 'omniauth', '~> 1.4.2'
|
||||
gem 'omniauth-auth0', '~> 1.4.1'
|
||||
gem 'omniauth-auth0', '~> 2.0.0'
|
||||
gem 'omniauth-azure-oauth2', '~> 0.0.9'
|
||||
gem 'omniauth-cas3', '~> 1.1.4'
|
||||
gem 'omniauth-facebook', '~> 4.0.0'
|
||||
|
|
|
@ -529,8 +529,8 @@ GEM
|
|||
omniauth (1.4.2)
|
||||
hashie (>= 1.2, < 4)
|
||||
rack (>= 1.0, < 3)
|
||||
omniauth-auth0 (1.4.1)
|
||||
omniauth-oauth2 (~> 1.1)
|
||||
omniauth-auth0 (2.0.0)
|
||||
omniauth-oauth2 (~> 1.4)
|
||||
omniauth-authentiq (0.3.1)
|
||||
omniauth-oauth2 (~> 1.3, >= 1.3.1)
|
||||
omniauth-azure-oauth2 (0.0.9)
|
||||
|
@ -1106,7 +1106,7 @@ DEPENDENCIES
|
|||
octokit (~> 4.6.2)
|
||||
oj (~> 2.17.4)
|
||||
omniauth (~> 1.4.2)
|
||||
omniauth-auth0 (~> 1.4.1)
|
||||
omniauth-auth0 (~> 2.0.0)
|
||||
omniauth-authentiq (~> 0.3.1)
|
||||
omniauth-azure-oauth2 (~> 0.0.9)
|
||||
omniauth-cas3 (~> 1.1.4)
|
||||
|
|
2
VERSION
2
VERSION
|
@ -1 +1 @@
|
|||
10.5.5
|
||||
10.5.6
|
||||
|
|
|
@ -95,6 +95,14 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
|
|||
handle_omniauth
|
||||
end
|
||||
|
||||
def auth0
|
||||
if oauth['uid'].blank?
|
||||
fail_auth0_login
|
||||
else
|
||||
handle_omniauth
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def handle_omniauth
|
||||
|
@ -170,6 +178,12 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
|
|||
redirect_to new_user_session_path
|
||||
end
|
||||
|
||||
def fail_auth0_login
|
||||
flash[:alert] = 'Wrong extern UID provided. Make sure Auth0 is configured correctly.'
|
||||
|
||||
redirect_to new_user_session_path
|
||||
end
|
||||
|
||||
def handle_disabled_provider
|
||||
label = Gitlab::OAuth::Provider.label_for(oauth['provider'])
|
||||
flash[:alert] = "Signing in using #{label} has been disabled"
|
||||
|
|
|
@ -245,7 +245,8 @@ module ApplicationSettingsHelper
|
|||
:usage_ping_enabled,
|
||||
:user_default_external,
|
||||
:user_oauth_applications,
|
||||
:version_check_enabled
|
||||
:version_check_enabled,
|
||||
:allow_local_requests_from_hooks_and_services
|
||||
]
|
||||
end
|
||||
end
|
||||
|
|
|
@ -330,7 +330,8 @@ class ApplicationSetting < ActiveRecord::Base
|
|||
usage_ping_enabled: Settings.gitlab['usage_ping_enabled'],
|
||||
gitaly_timeout_fast: 10,
|
||||
gitaly_timeout_medium: 30,
|
||||
gitaly_timeout_default: 55
|
||||
gitaly_timeout_default: 55,
|
||||
allow_local_requests_from_hooks_and_services: false
|
||||
}
|
||||
end
|
||||
|
||||
|
|
|
@ -37,6 +37,9 @@ class Project < ActiveRecord::Base
|
|||
attachments: 2
|
||||
}.freeze
|
||||
|
||||
# Valids ports to import from
|
||||
VALID_IMPORT_PORTS = [22, 80, 443].freeze
|
||||
|
||||
cache_markdown_field :description, pipeline: :description
|
||||
|
||||
delegate :feature_available?, :builds_enabled?, :wiki_enabled?,
|
||||
|
|
|
@ -1,6 +1,4 @@
|
|||
class AssemblaService < Service
|
||||
include HTTParty
|
||||
|
||||
prop_accessor :token, :subdomain
|
||||
validates :token, presence: true, if: :activated?
|
||||
|
||||
|
@ -31,6 +29,6 @@ class AssemblaService < Service
|
|||
return unless supported_events.include?(data[:object_kind])
|
||||
|
||||
url = "https://atlas.assembla.com/spaces/#{subdomain}/github_tool?secret_key=#{token}"
|
||||
AssemblaService.post(url, body: { payload: data }.to_json, headers: { 'Content-Type' => 'application/json' })
|
||||
Gitlab::HTTP.post(url, body: { payload: data }.to_json, headers: { 'Content-Type' => 'application/json' })
|
||||
end
|
||||
end
|
||||
|
|
|
@ -117,10 +117,10 @@ class BambooService < CiService
|
|||
url = build_url(path)
|
||||
|
||||
if username.blank? && password.blank?
|
||||
HTTParty.get(url, verify: false)
|
||||
Gitlab::HTTP.get(url, verify: false)
|
||||
else
|
||||
url << '&os_authType=basic'
|
||||
HTTParty.get(url, verify: false,
|
||||
Gitlab::HTTP.get(url, verify: false,
|
||||
basic_auth: {
|
||||
username: username,
|
||||
password: password
|
||||
|
|
|
@ -71,7 +71,7 @@ class BuildkiteService < CiService
|
|||
end
|
||||
|
||||
def calculate_reactive_cache(sha, ref)
|
||||
response = HTTParty.get(commit_status_path(sha), verify: false)
|
||||
response = Gitlab::HTTP.get(commit_status_path(sha), verify: false)
|
||||
|
||||
status =
|
||||
if response.code == 200 && response['status']
|
||||
|
|
|
@ -1,6 +1,4 @@
|
|||
class CampfireService < Service
|
||||
include HTTParty
|
||||
|
||||
prop_accessor :token, :subdomain, :room
|
||||
validates :token, presence: true, if: :activated?
|
||||
|
||||
|
@ -31,7 +29,6 @@ class CampfireService < Service
|
|||
def execute(data)
|
||||
return unless supported_events.include?(data[:object_kind])
|
||||
|
||||
self.class.base_uri base_uri
|
||||
message = build_message(data)
|
||||
speak(self.room, message, auth)
|
||||
end
|
||||
|
@ -69,14 +66,14 @@ class CampfireService < Service
|
|||
}
|
||||
}
|
||||
}
|
||||
res = self.class.post(path, auth.merge(body))
|
||||
res = Gitlab::HTTP.post(path, base_uri: base_uri, **auth.merge(body))
|
||||
res.code == 201 ? res : nil
|
||||
end
|
||||
|
||||
# Returns a list of rooms, or [].
|
||||
# https://github.com/basecamp/campfire-api/blob/master/sections/rooms.md#get-rooms
|
||||
def rooms(auth)
|
||||
res = self.class.get("/rooms.json", auth)
|
||||
res = Gitlab::HTTP.get("/rooms.json", base_uri: base_uri, **auth)
|
||||
res.code == 200 ? res["rooms"] : []
|
||||
end
|
||||
|
||||
|
|
|
@ -49,7 +49,7 @@ class DroneCiService < CiService
|
|||
end
|
||||
|
||||
def calculate_reactive_cache(sha, ref)
|
||||
response = HTTParty.get(commit_status_path(sha, ref), verify: enable_ssl_verification)
|
||||
response = Gitlab::HTTP.get(commit_status_path(sha, ref), verify: enable_ssl_verification)
|
||||
|
||||
status =
|
||||
if response.code == 200 && response['status']
|
||||
|
|
|
@ -1,6 +1,4 @@
|
|||
class ExternalWikiService < Service
|
||||
include HTTParty
|
||||
|
||||
prop_accessor :external_wiki_url
|
||||
|
||||
validates :external_wiki_url, presence: true, url: true, if: :activated?
|
||||
|
@ -24,7 +22,7 @@ class ExternalWikiService < Service
|
|||
end
|
||||
|
||||
def execute(_data)
|
||||
@response = HTTParty.get(properties['external_wiki_url'], verify: true) rescue nil
|
||||
@response = Gitlab::HTTP.get(properties['external_wiki_url'], verify: true) rescue nil
|
||||
if @response != 200
|
||||
nil
|
||||
end
|
||||
|
|
|
@ -77,13 +77,13 @@ class IssueTrackerService < Service
|
|||
result = false
|
||||
|
||||
begin
|
||||
response = HTTParty.head(self.project_url, verify: true)
|
||||
response = Gitlab::HTTP.head(self.project_url, verify: true)
|
||||
|
||||
if response
|
||||
message = "#{self.type} received response #{response.code} when attempting to connect to #{self.project_url}"
|
||||
result = true
|
||||
end
|
||||
rescue HTTParty::Error, Timeout::Error, SocketError, Errno::ECONNRESET, Errno::ECONNREFUSED, OpenSSL::SSL::SSLError => error
|
||||
rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Errno::ECONNRESET, Errno::ECONNREFUSED, OpenSSL::SSL::SSLError => error
|
||||
message = "#{self.type} had an error when trying to connect to #{self.project_url}: #{error.message}"
|
||||
end
|
||||
Rails.logger.info(message)
|
||||
|
|
|
@ -52,7 +52,7 @@ class MockCiService < CiService
|
|||
#
|
||||
#
|
||||
def commit_status(sha, ref)
|
||||
response = HTTParty.get(commit_status_path(sha), verify: false)
|
||||
response = Gitlab::HTTP.get(commit_status_path(sha), verify: false)
|
||||
read_commit_status(response)
|
||||
rescue Errno::ECONNREFUSED
|
||||
:error
|
||||
|
|
|
@ -1,6 +1,4 @@
|
|||
class PackagistService < Service
|
||||
include HTTParty
|
||||
|
||||
prop_accessor :username, :token, :server
|
||||
|
||||
validates :username, presence: true, if: :activated?
|
||||
|
|
|
@ -1,6 +1,4 @@
|
|||
class PivotaltrackerService < Service
|
||||
include HTTParty
|
||||
|
||||
API_ENDPOINT = 'https://www.pivotaltracker.com/services/v5/source_commits'.freeze
|
||||
|
||||
prop_accessor :token, :restrict_to_branch
|
||||
|
@ -52,7 +50,7 @@ class PivotaltrackerService < Service
|
|||
'message' => commit[:message]
|
||||
}
|
||||
}
|
||||
PivotaltrackerService.post(
|
||||
Gitlab::HTTP.post(
|
||||
API_ENDPOINT,
|
||||
body: message.to_json,
|
||||
headers: {
|
||||
|
|
|
@ -1,6 +1,5 @@
|
|||
class PushoverService < Service
|
||||
include HTTParty
|
||||
base_uri 'https://api.pushover.net/1'
|
||||
BASE_URI = 'https://api.pushover.net/1'.freeze
|
||||
|
||||
prop_accessor :api_key, :user_key, :device, :priority, :sound
|
||||
validates :api_key, :user_key, :priority, presence: true, if: :activated?
|
||||
|
@ -99,6 +98,6 @@ class PushoverService < Service
|
|||
pushover_data[:sound] = sound
|
||||
end
|
||||
|
||||
PushoverService.post('/messages.json', body: pushover_data)
|
||||
Gitlab::HTTP.post('/messages.json', base_uri: BASE_URI, body: pushover_data)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -83,7 +83,7 @@ class TeamcityService < CiService
|
|||
|
||||
branch = Gitlab::Git.ref_name(data[:ref])
|
||||
|
||||
HTTParty.post(
|
||||
Gitlab::HTTP.post(
|
||||
build_url('httpAuth/app/rest/buildQueue'),
|
||||
body: "<build branchName=\"#{branch}\">"\
|
||||
"<buildType id=\"#{build_type}\"/>"\
|
||||
|
@ -134,7 +134,7 @@ class TeamcityService < CiService
|
|||
end
|
||||
|
||||
def get_path(path)
|
||||
HTTParty.get(build_url(path), verify: false,
|
||||
Gitlab::HTTP.get(build_url(path), verify: false,
|
||||
basic_auth: {
|
||||
username: username,
|
||||
password: password
|
||||
|
|
|
@ -28,7 +28,7 @@ module Projects
|
|||
|
||||
def add_repository_to_project
|
||||
if project.external_import? && !unknown_url?
|
||||
raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url)
|
||||
raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url, valid_ports: Project::VALID_IMPORT_PORTS)
|
||||
end
|
||||
|
||||
# We should skip the repository for a GitHub import or GitLab project import,
|
||||
|
|
|
@ -14,16 +14,17 @@ class SubmitUsagePingService
|
|||
def execute
|
||||
return false unless Gitlab::CurrentSettings.usage_ping_enabled?
|
||||
|
||||
response = HTTParty.post(
|
||||
response = Gitlab::HTTP.post(
|
||||
URL,
|
||||
body: Gitlab::UsageData.to_json(force_refresh: true),
|
||||
allow_local_requests: true,
|
||||
headers: { 'Content-type' => 'application/json' }
|
||||
)
|
||||
|
||||
store_metrics(response)
|
||||
|
||||
true
|
||||
rescue HTTParty::Error => e
|
||||
rescue Gitlab::HTTP::Error => e
|
||||
Rails.logger.info "Unable to contact GitLab, Inc.: #{e}"
|
||||
|
||||
false
|
||||
|
|
|
@ -3,23 +3,20 @@ class WebHookService
|
|||
attr_reader :body, :headers, :code
|
||||
|
||||
def initialize
|
||||
@headers = HTTParty::Response::Headers.new({})
|
||||
@headers = Gitlab::HTTP::Response::Headers.new({})
|
||||
@body = ''
|
||||
@code = 'internal error'
|
||||
end
|
||||
end
|
||||
|
||||
include HTTParty
|
||||
|
||||
# HTTParty timeout
|
||||
default_timeout Gitlab.config.gitlab.webhook_timeout
|
||||
|
||||
attr_accessor :hook, :data, :hook_name
|
||||
attr_accessor :hook, :data, :hook_name, :request_options
|
||||
|
||||
def initialize(hook, data, hook_name)
|
||||
@hook = hook
|
||||
@data = data
|
||||
@hook_name = hook_name.to_s
|
||||
@request_options = { timeout: Gitlab.config.gitlab.webhook_timeout }
|
||||
@request_options.merge!(allow_local_requests: true) if @hook.is_a?(SystemHook)
|
||||
end
|
||||
|
||||
def execute
|
||||
|
@ -73,11 +70,12 @@ class WebHookService
|
|||
end
|
||||
|
||||
def make_request(url, basic_auth = false)
|
||||
self.class.post(url,
|
||||
Gitlab::HTTP.post(url,
|
||||
body: data.to_json,
|
||||
headers: build_headers(hook_name),
|
||||
verify: hook.enable_ssl_verification,
|
||||
basic_auth: basic_auth)
|
||||
basic_auth: basic_auth,
|
||||
**request_options)
|
||||
end
|
||||
|
||||
def make_request_with_auth
|
||||
|
|
|
@ -4,7 +4,7 @@
|
|||
# protect against Server-side Request Forgery (SSRF).
|
||||
class ImportableUrlValidator < ActiveModel::EachValidator
|
||||
def validate_each(record, attribute, value)
|
||||
if Gitlab::UrlBlocker.blocked_url?(value)
|
||||
if Gitlab::UrlBlocker.blocked_url?(value, valid_ports: Project::VALID_IMPORT_PORTS)
|
||||
record.errors.add(attribute, "imports are not allowed from that URL")
|
||||
end
|
||||
end
|
||||
|
|
|
@ -858,5 +858,14 @@
|
|||
.col-sm-10
|
||||
= f.number_field :throttle_authenticated_web_period_in_seconds, class: 'form-control'
|
||||
|
||||
%fieldset
|
||||
%legend Outbound requests
|
||||
.form-group
|
||||
.col-sm-offset-2.col-sm-10
|
||||
.checkbox
|
||||
= f.label :allow_local_requests_from_hooks_and_services do
|
||||
= f.check_box :allow_local_requests_from_hooks_and_services
|
||||
Allow requests to the local network from hooks and services
|
||||
|
||||
.form-actions
|
||||
= f.submit 'Save', class: 'btn btn-save'
|
||||
|
|
|
@ -0,0 +1,18 @@
|
|||
class AddAllowLocalRequestsFromHooksAndServicesToApplicationSettings < ActiveRecord::Migration
|
||||
include Gitlab::Database::MigrationHelpers
|
||||
|
||||
DOWNTIME = false
|
||||
|
||||
disable_ddl_transaction!
|
||||
|
||||
def up
|
||||
add_column_with_default(:application_settings, :allow_local_requests_from_hooks_and_services,
|
||||
:boolean,
|
||||
default: false,
|
||||
allow_null: false)
|
||||
end
|
||||
|
||||
def down
|
||||
remove_column(:application_settings, :allow_local_requests_from_hooks_and_services)
|
||||
end
|
||||
end
|
|
@ -0,0 +1,25 @@
|
|||
class RemoveEmptyExternUidAuth0Identities < ActiveRecord::Migration
|
||||
include Gitlab::Database::MigrationHelpers
|
||||
|
||||
DOWNTIME = false
|
||||
|
||||
disable_ddl_transaction!
|
||||
|
||||
class Identity < ActiveRecord::Base
|
||||
self.table_name = 'identities'
|
||||
include EachBatch
|
||||
end
|
||||
|
||||
def up
|
||||
broken_auth0_identities.each_batch do |identity|
|
||||
identity.delete_all
|
||||
end
|
||||
end
|
||||
|
||||
def broken_auth0_identities
|
||||
Identity.where(provider: 'auth0', extern_uid: [nil, ''])
|
||||
end
|
||||
|
||||
def down
|
||||
end
|
||||
end
|
|
@ -155,6 +155,7 @@ ActiveRecord::Schema.define(version: 20180308052825) do
|
|||
t.integer "gitaly_timeout_medium", default: 30, null: false
|
||||
t.integer "gitaly_timeout_fast", default: 10, null: false
|
||||
t.boolean "authorized_keys_enabled", default: true, null: false
|
||||
t.boolean "allow_local_requests_from_hooks_and_services", default: false, null: false
|
||||
t.string "auto_devops_domain"
|
||||
t.boolean "pages_domain_verification_enabled", default: true, null: false
|
||||
end
|
||||
|
|
|
@ -56,7 +56,8 @@ for initial settings.
|
|||
"name" => "auth0",
|
||||
"args" => { client_id: 'YOUR_AUTH0_CLIENT_ID',
|
||||
client_secret: 'YOUR_AUTH0_CLIENT_SECRET',
|
||||
namespace: 'YOUR_AUTH0_DOMAIN'
|
||||
domain: 'YOUR_AUTH0_DOMAIN',
|
||||
scope: 'openid profile email'
|
||||
}
|
||||
}
|
||||
]
|
||||
|
@ -69,8 +70,8 @@ for initial settings.
|
|||
args: {
|
||||
client_id: 'YOUR_AUTH0_CLIENT_ID',
|
||||
client_secret: 'YOUR_AUTH0_CLIENT_SECRET',
|
||||
namespace: 'YOUR_AUTH0_DOMAIN'
|
||||
}
|
||||
domain: 'YOUR_AUTH0_DOMAIN',
|
||||
scope: 'openid profile email' }
|
||||
}
|
||||
```
|
||||
|
||||
|
|
11
lib/gitlab/http.rb
Normal file
11
lib/gitlab/http.rb
Normal file
|
@ -0,0 +1,11 @@
|
|||
# This class is used as a proxy for all outbounding http connection
|
||||
# coming from callbacks, services and hooks. The direct use of the HTTParty
|
||||
# is discouraged because it can lead to several security problems, like SSRF
|
||||
# calling internal IP or services.
|
||||
module Gitlab
|
||||
class HTTP
|
||||
include HTTParty # rubocop:disable Gitlab/HTTParty
|
||||
|
||||
connection_adapter ProxyHTTPConnectionAdapter
|
||||
end
|
||||
end
|
34
lib/gitlab/proxy_http_connection_adapter.rb
Normal file
34
lib/gitlab/proxy_http_connection_adapter.rb
Normal file
|
@ -0,0 +1,34 @@
|
|||
# This class is part of the Gitlab::HTTP wrapper. Depending on the value
|
||||
# of the global setting allow_local_requests_from_hooks_and_services this adapter
|
||||
# will allow/block connection to internal IPs and/or urls.
|
||||
#
|
||||
# This functionality can be overriden by providing the setting the option
|
||||
# allow_local_requests = true in the request. For example:
|
||||
# Gitlab::HTTP.get('http://www.gitlab.com', allow_local_requests: true)
|
||||
#
|
||||
# This option will take precedence over the global setting.
|
||||
module Gitlab
|
||||
class ProxyHTTPConnectionAdapter < HTTParty::ConnectionAdapter
|
||||
def connection
|
||||
if !allow_local_requests? && blocked_url?
|
||||
raise URI::InvalidURIError
|
||||
end
|
||||
|
||||
super
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def blocked_url?
|
||||
Gitlab::UrlBlocker.blocked_url?(uri, allow_private_networks: false)
|
||||
end
|
||||
|
||||
def allow_local_requests?
|
||||
options.fetch(:allow_local_requests, allow_settings_local_requests?)
|
||||
end
|
||||
|
||||
def allow_settings_local_requests?
|
||||
Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
|
||||
end
|
||||
end
|
||||
end
|
|
@ -3,11 +3,7 @@ require 'resolv'
|
|||
module Gitlab
|
||||
class UrlBlocker
|
||||
class << self
|
||||
# Used to specify what hosts and port numbers should be prohibited for project
|
||||
# imports.
|
||||
VALID_PORTS = [22, 80, 443].freeze
|
||||
|
||||
def blocked_url?(url)
|
||||
def blocked_url?(url, allow_private_networks: true, valid_ports: [])
|
||||
return false if url.nil?
|
||||
|
||||
blocked_ips = ["127.0.0.1", "::1", "0.0.0.0"]
|
||||
|
@ -18,12 +14,15 @@ module Gitlab
|
|||
# Allow imports from the GitLab instance itself but only from the configured ports
|
||||
return false if internal?(uri)
|
||||
|
||||
return true if blocked_port?(uri.port)
|
||||
return true if blocked_port?(uri.port, valid_ports)
|
||||
return true if blocked_user_or_hostname?(uri.user)
|
||||
return true if blocked_user_or_hostname?(uri.hostname)
|
||||
|
||||
server_ips = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM).map(&:ip_address)
|
||||
addrs_info = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM)
|
||||
server_ips = addrs_info.map(&:ip_address)
|
||||
|
||||
return true if (blocked_ips & server_ips).any?
|
||||
return true if !allow_private_networks && private_network?(addrs_info)
|
||||
rescue Addressable::URI::InvalidURIError
|
||||
return true
|
||||
rescue SocketError
|
||||
|
@ -35,10 +34,10 @@ module Gitlab
|
|||
|
||||
private
|
||||
|
||||
def blocked_port?(port)
|
||||
return false if port.blank?
|
||||
def blocked_port?(port, valid_ports)
|
||||
return false if port.blank? || valid_ports.blank?
|
||||
|
||||
port < 1024 && !VALID_PORTS.include?(port)
|
||||
port < 1024 && !valid_ports.include?(port)
|
||||
end
|
||||
|
||||
def blocked_user_or_hostname?(value)
|
||||
|
@ -61,6 +60,10 @@ module Gitlab
|
|||
(uri.port.blank? || uri.port == config.gitlab_shell.ssh_port)
|
||||
end
|
||||
|
||||
def private_network?(addrs_info)
|
||||
addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? }
|
||||
end
|
||||
|
||||
def config
|
||||
Gitlab.config
|
||||
end
|
||||
|
|
|
@ -22,16 +22,14 @@ module Mattermost
|
|||
# going.
|
||||
class Session
|
||||
include Doorkeeper::Helpers::Controller
|
||||
include HTTParty
|
||||
|
||||
LEASE_TIMEOUT = 60
|
||||
|
||||
base_uri Settings.mattermost.host
|
||||
|
||||
attr_accessor :current_resource_owner, :token
|
||||
attr_accessor :current_resource_owner, :token, :base_uri
|
||||
|
||||
def initialize(current_user)
|
||||
@current_resource_owner = current_user
|
||||
@base_uri = Settings.mattermost.host
|
||||
end
|
||||
|
||||
def with_session
|
||||
|
@ -73,18 +71,26 @@ module Mattermost
|
|||
|
||||
def get(path, options = {})
|
||||
handle_exceptions do
|
||||
self.class.get(path, options.merge(headers: @headers))
|
||||
Gitlab::HTTP.get(path, build_options(options))
|
||||
end
|
||||
end
|
||||
|
||||
def post(path, options = {})
|
||||
handle_exceptions do
|
||||
self.class.post(path, options.merge(headers: @headers))
|
||||
Gitlab::HTTP.post(path, build_options(options))
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def build_options(options)
|
||||
options.tap do |hash|
|
||||
hash[:headers] = @headers
|
||||
hash[:allow_local_requests] = true
|
||||
hash[:base_uri] = base_uri if base_uri.presence
|
||||
end
|
||||
end
|
||||
|
||||
def create
|
||||
raise Mattermost::NoSessionError unless oauth_uri
|
||||
raise Mattermost::NoSessionError unless token_uri
|
||||
|
@ -159,14 +165,14 @@ module Mattermost
|
|||
|
||||
def handle_exceptions
|
||||
yield
|
||||
rescue HTTParty::Error => e
|
||||
rescue Gitlab::HTTP::Error => e
|
||||
raise Mattermost::ConnectionError.new(e.message)
|
||||
rescue Errno::ECONNREFUSED => e
|
||||
raise Mattermost::ConnectionError.new(e.message)
|
||||
end
|
||||
|
||||
def parse_cookie(response)
|
||||
cookie_hash = CookieHash.new
|
||||
cookie_hash = Gitlab::HTTP::CookieHash.new
|
||||
response.get_fields('Set-Cookie').each { |c| cookie_hash.add_cookies(c) }
|
||||
cookie_hash
|
||||
end
|
||||
|
|
|
@ -9,14 +9,15 @@ module MicrosoftTeams
|
|||
result = false
|
||||
|
||||
begin
|
||||
response = HTTParty.post(
|
||||
response = Gitlab::HTTP.post(
|
||||
@webhook.to_str,
|
||||
headers: @header,
|
||||
allow_local_requests: true,
|
||||
body: body(options)
|
||||
)
|
||||
|
||||
result = true if response
|
||||
rescue HTTParty::Error, StandardError => error
|
||||
rescue Gitlab::HTTP::Error, StandardError => error
|
||||
Rails.logger.info("#{self.class.name}: Error while connecting to #{@webhook}: #{error.message}")
|
||||
end
|
||||
|
||||
|
|
62
rubocop/cop/gitlab/httparty.rb
Normal file
62
rubocop/cop/gitlab/httparty.rb
Normal file
|
@ -0,0 +1,62 @@
|
|||
require_relative '../../spec_helpers'
|
||||
|
||||
module RuboCop
|
||||
module Cop
|
||||
module Gitlab
|
||||
class HTTParty < RuboCop::Cop::Cop
|
||||
include SpecHelpers
|
||||
|
||||
MSG_SEND = <<~EOL.freeze
|
||||
Avoid calling `HTTParty` directly. Instead, use the Gitlab::HTTP
|
||||
wrapper. To allow request to localhost or the private network set
|
||||
the option :allow_local_requests in the request call.
|
||||
EOL
|
||||
|
||||
MSG_INCLUDE = <<~EOL.freeze
|
||||
Avoid including `HTTParty` directly. Instead, use the Gitlab::HTTP
|
||||
wrapper. To allow request to localhost or the private network set
|
||||
the option :allow_local_requests in the request call.
|
||||
EOL
|
||||
|
||||
def_node_matcher :includes_httparty?, <<~PATTERN
|
||||
(send nil? :include (const nil? :HTTParty))
|
||||
PATTERN
|
||||
|
||||
def_node_matcher :httparty_node?, <<~PATTERN
|
||||
(send (const nil? :HTTParty)...)
|
||||
PATTERN
|
||||
|
||||
def on_send(node)
|
||||
return if in_spec?(node)
|
||||
|
||||
add_offense(node, location: :expression, message: MSG_SEND) if httparty_node?(node)
|
||||
add_offense(node, location: :expression, message: MSG_INCLUDE) if includes_httparty?(node)
|
||||
end
|
||||
|
||||
def autocorrect(node)
|
||||
if includes_httparty?(node)
|
||||
autocorrect_includes_httparty(node)
|
||||
else
|
||||
autocorrect_httparty_node(node)
|
||||
end
|
||||
end
|
||||
|
||||
def autocorrect_includes_httparty(node)
|
||||
lambda do |corrector|
|
||||
corrector.remove(node.source_range)
|
||||
end
|
||||
end
|
||||
|
||||
def autocorrect_httparty_node(node)
|
||||
_, method_name, *arg_nodes = *node
|
||||
|
||||
replacement = "Gitlab::HTTP.#{method_name}(#{arg_nodes.map(&:source).join(', ')})"
|
||||
|
||||
lambda do |corrector|
|
||||
corrector.replace(node.source_range, replacement)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -1,5 +1,6 @@
|
|||
require_relative 'cop/gitlab/module_with_instance_variables'
|
||||
require_relative 'cop/gitlab/predicate_memoization'
|
||||
require_relative 'cop/gitlab/httparty'
|
||||
require_relative 'cop/include_sidekiq_worker'
|
||||
require_relative 'cop/line_break_around_conditional_block'
|
||||
require_relative 'cop/migration/add_column'
|
||||
|
|
|
@ -3,14 +3,17 @@ require 'spec_helper'
|
|||
describe OmniauthCallbacksController do
|
||||
include LoginHelpers
|
||||
|
||||
let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: provider) }
|
||||
let(:provider) { :github }
|
||||
let(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) }
|
||||
|
||||
before do
|
||||
mock_auth_hash(provider.to_s, 'my-uid', user.email)
|
||||
mock_auth_hash(provider.to_s, extern_uid, user.email)
|
||||
stub_omniauth_provider(provider, context: request)
|
||||
end
|
||||
|
||||
context 'github' do
|
||||
let(:extern_uid) { 'my-uid' }
|
||||
let(:provider) { :github }
|
||||
|
||||
it 'allows sign in' do
|
||||
post provider
|
||||
|
||||
|
@ -72,4 +75,18 @@ describe OmniauthCallbacksController do
|
|||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'auth0' do
|
||||
let(:extern_uid) { '' }
|
||||
let(:provider) { :auth0 }
|
||||
|
||||
it 'does not allow sign in without extern_uid' do
|
||||
post 'auth0'
|
||||
|
||||
expect(request.env['warden']).not_to be_authenticated
|
||||
expect(response.status).to eq(302)
|
||||
expect(controller).to set_flash[:alert].to('Wrong extern UID provided. Make sure Auth0 is configured correctly.')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
49
spec/lib/gitlab/http_spec.rb
Normal file
49
spec/lib/gitlab/http_spec.rb
Normal file
|
@ -0,0 +1,49 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Gitlab::HTTP do
|
||||
describe 'allow_local_requests_from_hooks_and_services is' do
|
||||
before do
|
||||
WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success')
|
||||
end
|
||||
|
||||
context 'disabled' do
|
||||
before do
|
||||
allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(false)
|
||||
end
|
||||
|
||||
it 'deny requests to localhost' do
|
||||
expect { described_class.get('http://localhost:3003') }.to raise_error(URI::InvalidURIError)
|
||||
end
|
||||
|
||||
it 'deny requests to private network' do
|
||||
expect { described_class.get('http://192.168.1.2:3003') }.to raise_error(URI::InvalidURIError)
|
||||
end
|
||||
|
||||
context 'if allow_local_requests set to true' do
|
||||
it 'override the global value and allow requests to localhost or private network' do
|
||||
expect { described_class.get('http://localhost:3003', allow_local_requests: true) }.not_to raise_error
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'enabled' do
|
||||
before do
|
||||
allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(true)
|
||||
end
|
||||
|
||||
it 'allow requests to localhost' do
|
||||
expect { described_class.get('http://localhost:3003') }.not_to raise_error
|
||||
end
|
||||
|
||||
it 'allow requests to private network' do
|
||||
expect { described_class.get('http://192.168.1.2:3003') }.not_to raise_error
|
||||
end
|
||||
|
||||
context 'if allow_local_requests set to false' do
|
||||
it 'override the global value and ban requests to localhost or private network' do
|
||||
expect { described_class.get('http://localhost:3003', allow_local_requests: false) }.to raise_error(URI::InvalidURIError)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -2,6 +2,8 @@ require 'spec_helper'
|
|||
|
||||
describe Gitlab::UrlBlocker do
|
||||
describe '#blocked_url?' do
|
||||
let(:valid_ports) { Project::VALID_IMPORT_PORTS }
|
||||
|
||||
it 'allows imports from configured web host and port' do
|
||||
import_url = "http://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.port}/t.git"
|
||||
expect(described_class.blocked_url?(import_url)).to be false
|
||||
|
@ -17,7 +19,7 @@ describe Gitlab::UrlBlocker do
|
|||
end
|
||||
|
||||
it 'returns true for bad port' do
|
||||
expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true
|
||||
expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git', valid_ports: valid_ports)).to be true
|
||||
end
|
||||
|
||||
it 'returns true for alternative version of 127.0.0.1 (0177.1)' do
|
||||
|
@ -71,6 +73,47 @@ describe Gitlab::UrlBlocker do
|
|||
it 'returns false for legitimate URL' do
|
||||
expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false
|
||||
end
|
||||
|
||||
context 'when allow_private_networks is' do
|
||||
let(:private_networks) { ['192.168.1.2', '10.0.0.2', '172.16.0.2'] }
|
||||
let(:fake_domain) { 'www.fakedomain.fake' }
|
||||
|
||||
context 'true (default)' do
|
||||
it 'does not block urls from private networks' do
|
||||
private_networks.each do |ip|
|
||||
stub_domain_resolv(fake_domain, ip)
|
||||
|
||||
expect(described_class).not_to be_blocked_url("http://#{fake_domain}")
|
||||
|
||||
unstub_domain_resolv
|
||||
|
||||
expect(described_class).not_to be_blocked_url("http://#{ip}")
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'false' do
|
||||
it 'blocks urls from private networks' do
|
||||
private_networks.each do |ip|
|
||||
stub_domain_resolv(fake_domain, ip)
|
||||
|
||||
expect(described_class).to be_blocked_url("http://#{fake_domain}", allow_private_networks: false)
|
||||
|
||||
unstub_domain_resolv
|
||||
|
||||
expect(described_class).to be_blocked_url("http://#{ip}", allow_private_networks: false)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def stub_domain_resolv(domain, ip)
|
||||
allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([double(ip_address: ip, ipv4_private?: true)])
|
||||
end
|
||||
|
||||
def unstub_domain_resolv
|
||||
allow(Addrinfo).to receive(:getaddrinfo).and_call_original
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
# Resolv does not support resolving UTF-8 domain names
|
||||
|
|
|
@ -4,10 +4,11 @@ describe Mattermost::Command do
|
|||
let(:params) { { 'token' => 'token', team_id: 'abc' } }
|
||||
|
||||
before do
|
||||
Mattermost::Session.base_uri('http://mattermost.example.com')
|
||||
session = Mattermost::Session.new(nil)
|
||||
session.base_uri = 'http://mattermost.example.com'
|
||||
|
||||
allow_any_instance_of(Mattermost::Client).to receive(:with_session)
|
||||
.and_yield(Mattermost::Session.new(nil))
|
||||
.and_yield(session)
|
||||
end
|
||||
|
||||
describe '#create' do
|
||||
|
|
|
@ -15,7 +15,7 @@ describe Mattermost::Session, type: :request do
|
|||
it { is_expected.to respond_to(:strategy) }
|
||||
|
||||
before do
|
||||
described_class.base_uri(mattermost_url)
|
||||
subject.base_uri = mattermost_url
|
||||
end
|
||||
|
||||
describe '#with session' do
|
||||
|
|
|
@ -2,10 +2,11 @@ require 'spec_helper'
|
|||
|
||||
describe Mattermost::Team do
|
||||
before do
|
||||
Mattermost::Session.base_uri('http://mattermost.example.com')
|
||||
session = Mattermost::Session.new(nil)
|
||||
session.base_uri = 'http://mattermost.example.com'
|
||||
|
||||
allow_any_instance_of(Mattermost::Client).to receive(:with_session)
|
||||
.and_yield(Mattermost::Session.new(nil))
|
||||
.and_yield(session)
|
||||
end
|
||||
|
||||
describe '#all' do
|
||||
|
|
|
@ -0,0 +1,22 @@
|
|||
require 'spec_helper'
|
||||
require Rails.root.join('db', 'post_migrate', '20180220150310_remove_empty_extern_uid_auth0_identities.rb')
|
||||
|
||||
describe RemoveEmptyExternUidAuth0Identities, :migration do
|
||||
let(:identities) { table(:identities) }
|
||||
|
||||
before do
|
||||
identities.create(provider: 'auth0', extern_uid: '')
|
||||
identities.create(provider: 'auth0', extern_uid: 'valid')
|
||||
identities.create(provider: 'github', extern_uid: '')
|
||||
|
||||
migrate!
|
||||
end
|
||||
|
||||
it 'leaves the correct auth0 identity' do
|
||||
expect(identities.where(provider: 'auth0').pluck(:extern_uid)).to eq(['valid'])
|
||||
end
|
||||
|
||||
it 'leaves the correct github identity' do
|
||||
expect(identities.where(provider: 'github').count).to eq(1)
|
||||
end
|
||||
end
|
|
@ -9,10 +9,11 @@ describe MattermostSlashCommandsService do
|
|||
let(:user) { create(:user) }
|
||||
|
||||
before do
|
||||
Mattermost::Session.base_uri("http://mattermost.example.com")
|
||||
session = Mattermost::Session.new(nil)
|
||||
session.base_uri = 'http://mattermost.example.com'
|
||||
|
||||
allow_any_instance_of(Mattermost::Client).to receive(:with_session)
|
||||
.and_yield(Mattermost::Session.new(nil))
|
||||
.and_yield(session)
|
||||
end
|
||||
|
||||
describe '#configure' do
|
||||
|
|
74
spec/rubocop/cop/gitlab/httparty_spec.rb
Normal file
74
spec/rubocop/cop/gitlab/httparty_spec.rb
Normal file
|
@ -0,0 +1,74 @@
|
|||
require 'spec_helper'
|
||||
require 'rubocop'
|
||||
require 'rubocop/rspec/support'
|
||||
require_relative '../../../../rubocop/cop/gitlab/httparty'
|
||||
|
||||
describe RuboCop::Cop::Gitlab::HTTParty do # rubocop:disable RSpec/FilePath
|
||||
include CopHelper
|
||||
|
||||
subject(:cop) { described_class.new }
|
||||
|
||||
shared_examples('registering include offense') do |options|
|
||||
let(:offending_lines) { options[:offending_lines] }
|
||||
|
||||
it 'registers an offense when the class includes HTTParty' do
|
||||
inspect_source(source)
|
||||
|
||||
aggregate_failures do
|
||||
expect(cop.offenses.size).to eq(offending_lines.size)
|
||||
expect(cop.offenses.map(&:line)).to eq(offending_lines)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
shared_examples('registering call offense') do |options|
|
||||
let(:offending_lines) { options[:offending_lines] }
|
||||
|
||||
it 'registers an offense when the class calls HTTParty' do
|
||||
inspect_source(source)
|
||||
|
||||
aggregate_failures do
|
||||
expect(cop.offenses.size).to eq(offending_lines.size)
|
||||
expect(cop.offenses.map(&:line)).to eq(offending_lines)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when source is a regular module' do
|
||||
it_behaves_like 'registering include offense', offending_lines: [2] do
|
||||
let(:source) do
|
||||
<<~RUBY
|
||||
module M
|
||||
include HTTParty
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when source is a regular class' do
|
||||
it_behaves_like 'registering include offense', offending_lines: [2] do
|
||||
let(:source) do
|
||||
<<~RUBY
|
||||
class Foo
|
||||
include HTTParty
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when HTTParty is called' do
|
||||
it_behaves_like 'registering call offense', offending_lines: [3] do
|
||||
let(:source) do
|
||||
<<~RUBY
|
||||
class Foo
|
||||
def bar
|
||||
HTTParty.get('http://example.com')
|
||||
end
|
||||
end
|
||||
RUBY
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -14,6 +14,20 @@ describe WebHookService do
|
|||
end
|
||||
let(:service_instance) { described_class.new(project_hook, data, :push_hooks) }
|
||||
|
||||
describe '#initialize' do
|
||||
it 'allow_local_requests is true if hook is a SystemHook' do
|
||||
instance = described_class.new(build(:system_hook), data, :system_hook)
|
||||
expect(instance.request_options[:allow_local_requests]).to be_truthy
|
||||
end
|
||||
|
||||
it 'allow_local_requests is false if hook is not a SystemHook' do
|
||||
%i(project_hook service_hook web_hook_log).each do |hook|
|
||||
instance = described_class.new(build(hook), data, hook)
|
||||
expect(instance.request_options[:allow_local_requests]).to be_falsey
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#execute' do
|
||||
before do
|
||||
project.hooks << [project_hook]
|
||||
|
|
Loading…
Reference in a new issue