summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2018-03-16 18:43:44 +0000
committerRobert Speicher <robert@gitlab.com>2018-03-16 18:43:44 +0000
commit9d9393d50e326e3360398908ce41d04bbbc9fbff (patch)
treef19e32048db3f42f6cbe2cab80135cfc2e6f7e15
parenta99baf5e1fc8d28ca0637c0cd5b13a54832f866b (diff)
parent1e21892f753aee9e6ec0ef612b53db07763bf03c (diff)
downloadgitlab-ce-9d9393d50e326e3360398908ce41d04bbbc9fbff.tar.gz
Merge branch '10-6-stable-rc6-critical' into '10-6-stable'
Preparation MR for 10.6RC6 See merge request gitlab/gitlabhq!2358
-rw-r--r--.rubocop.yml3
-rw-r--r--Gemfile2
-rw-r--r--Gemfile.lock6
-rw-r--r--app/assets/javascripts/notes.js68
-rw-r--r--app/controllers/omniauth_callbacks_controller.rb14
-rw-r--r--app/controllers/projects/discussions_controller.rb6
-rw-r--r--app/helpers/application_settings_helper.rb3
-rw-r--r--app/models/application_setting.rb3
-rw-r--r--app/models/project.rb3
-rw-r--r--app/models/project_services/assembla_service.rb4
-rw-r--r--app/models/project_services/bamboo_service.rb12
-rw-r--r--app/models/project_services/buildkite_service.rb2
-rw-r--r--app/models/project_services/campfire_service.rb7
-rw-r--r--app/models/project_services/drone_ci_service.rb2
-rw-r--r--app/models/project_services/external_wiki_service.rb4
-rw-r--r--app/models/project_services/issue_tracker_service.rb4
-rw-r--r--app/models/project_services/mock_ci_service.rb2
-rw-r--r--app/models/project_services/packagist_service.rb2
-rw-r--r--app/models/project_services/pivotaltracker_service.rb4
-rw-r--r--app/models/project_services/pushover_service.rb5
-rw-r--r--app/models/project_services/teamcity_service.rb12
-rw-r--r--app/services/projects/import_service.rb2
-rw-r--r--app/services/submit_usage_ping_service.rb5
-rw-r--r--app/services/web_hook_service.rb16
-rw-r--r--app/validators/importable_url_validator.rb2
-rw-r--r--app/views/admin/application_settings/_form.html.haml9
-rw-r--r--app/views/discussions/_diff_with_notes.html.haml31
-rw-r--r--app/views/discussions/_discussion.html.haml2
-rw-r--r--changelogs/unreleased/35475-lazy-diff.yml5
-rw-r--r--changelogs/unreleased/fix-auth0-unsafe-login.yml5
-rw-r--r--changelogs/unreleased/fj-15329-services-callbacks-ssrf.yml5
-rw-r--r--changelogs/unreleased/remove-unnecessary-validate-project.yml5
-rw-r--r--config/routes/project.rb2
-rw-r--r--db/migrate/20180223120443_create_user_interacted_projects_table.rb4
-rw-r--r--db/migrate/20180223144945_add_allow_local_requests_from_hooks_and_services_to_application_settings.rb18
-rw-r--r--db/post_migrate/20180220150310_remove_empty_extern_uid_auth0_identities.rb25
-rw-r--r--db/post_migrate/20180223124427_build_user_interacted_projects_table.rb142
-rw-r--r--db/schema.rb1
-rw-r--r--doc/integration/auth0.md7
-rw-r--r--lib/gitlab/database/migration_helpers.rb13
-rw-r--r--lib/gitlab/http.rb11
-rw-r--r--lib/gitlab/proxy_http_connection_adapter.rb34
-rw-r--r--lib/gitlab/url_blocker.rb23
-rw-r--r--lib/mattermost/session.rb22
-rw-r--r--lib/microsoft_teams/notifier.rb5
-rw-r--r--rubocop/cop/gitlab/httparty.rb62
-rw-r--r--rubocop/rubocop.rb1
-rw-r--r--spec/controllers/omniauth_callbacks_controller_spec.rb103
-rw-r--r--spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb1
-rw-r--r--spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb16
-rw-r--r--spec/lib/gitlab/http_spec.rb49
-rw-r--r--spec/lib/gitlab/url_blocker_spec.rb45
-rw-r--r--spec/lib/mattermost/command_spec.rb5
-rw-r--r--spec/lib/mattermost/session_spec.rb2
-rw-r--r--spec/lib/mattermost/team_spec.rb5
-rw-r--r--spec/migrations/remove_empty_extern_uid_auth0_identities_spec.rb22
-rw-r--r--spec/models/project_services/mattermost_slash_commands_service_spec.rb5
-rw-r--r--spec/rubocop/cop/gitlab/httparty_spec.rb74
-rw-r--r--spec/services/web_hook_service_spec.rb14
59 files changed, 666 insertions, 300 deletions
diff --git a/.rubocop.yml b/.rubocop.yml
index 293f61fb725..90dac884538 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -46,6 +46,9 @@ Gitlab/ModuleWithInstanceVariables:
- spec/support/**/*.rb
- features/steps/**/*.rb
+Gitlab/HTTParty:
+ Enabled: true
+
GitlabSecurity/PublicSend:
Enabled: true
Exclude:
diff --git a/Gemfile b/Gemfile
index 2793463fd81..bc30c31c38f 100644
--- a/Gemfile
+++ b/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'
diff --git a/Gemfile.lock b/Gemfile.lock
index fa99ec3febe..8957f1f3867 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -531,8 +531,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)
@@ -1113,7 +1113,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)
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index 6d1b2f452c0..c640003d958 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -16,10 +16,6 @@ import Autosize from 'autosize';
import 'vendor/jquery.caret'; // required by jquery.atwho
import 'vendor/jquery.atwho';
import AjaxCache from '~/lib/utils/ajax_cache';
-import Vue from 'vue';
-import syntaxHighlight from '~/syntax_highlight';
-import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue';
-import { __ } from '~/locale';
import axios from './lib/utils/axios_utils';
import { getLocationHash } from './lib/utils/url_utility';
import Flash from './flash';
@@ -103,13 +99,6 @@ export default class Notes {
$('.note-edit-form').clone()
.addClass('mr-note-edit-form').insertAfter('.note-edit-form');
}
-
- const hash = getLocationHash();
- const $anchor = hash && document.getElementById(hash);
-
- if ($anchor) {
- this.loadLazyDiff({ currentTarget: $anchor });
- }
}
setViewType(view) {
@@ -146,8 +135,6 @@ export default class Notes {
this.$wrapperEl.on('click', '.js-close-discussion-note-form', this.cancelDiscussionForm);
// toggle commit list
this.$wrapperEl.on('click', '.system-note-commit-list-toggler', this.toggleCommitList);
-
- this.$wrapperEl.on('click', '.js-toggle-lazy-diff', this.loadLazyDiff);
// fetch notes when tab becomes visible
this.$wrapperEl.on('visibilitychange', this.visibilityChange);
// when issue status changes, we need to refresh data
@@ -186,7 +173,6 @@ export default class Notes {
this.$wrapperEl.off('keydown', '.js-note-text');
this.$wrapperEl.off('click', '.js-comment-resolve-button');
this.$wrapperEl.off('click', '.system-note-commit-list-toggler');
- this.$wrapperEl.off('click', '.js-toggle-lazy-diff');
this.$wrapperEl.off('ajax:success', '.js-main-target-form');
this.$wrapperEl.off('ajax:success', '.js-discussion-note-form');
this.$wrapperEl.off('ajax:complete', '.js-main-target-form');
@@ -1221,60 +1207,6 @@ export default class Notes {
return this.notesCountBadge.text(parseInt(this.notesCountBadge.text(), 10) + updateCount);
}
- static renderPlaceholderComponent($container) {
- const el = $container.find('.js-code-placeholder').get(0);
- new Vue({ // eslint-disable-line no-new
- el,
- components: {
- SkeletonLoadingContainer,
- },
- render(createElement) {
- return createElement('skeleton-loading-container');
- },
- });
- }
-
- static renderDiffContent($container, data) {
- const { discussion_html } = data;
- const lines = $(discussion_html).find('.line_holder');
- lines.addClass('fade-in');
- $container.find('tbody').prepend(lines);
- const fileHolder = $container.find('.file-holder');
- $container.find('.line-holder-placeholder').remove();
- syntaxHighlight(fileHolder);
- }
-
- static renderDiffError($container) {
- $container.find('.line_content').html(
- $(`
- <div class="nothing-here-block">
- ${__('Unable to load the diff.')} <a class="js-toggle-lazy-diff" href="javascript:void(0)">Try again</a>?
- </div>
- `),
- );
- }
-
- loadLazyDiff(e) {
- const $container = $(e.currentTarget).closest('.js-toggle-container');
- Notes.renderPlaceholderComponent($container);
-
- $container.find('.js-toggle-lazy-diff').removeClass('js-toggle-lazy-diff');
-
- const tableEl = $container.find('tbody');
- if (tableEl.length === 0) return;
-
- const fileHolder = $container.find('.file-holder');
- const url = fileHolder.data('linesPath');
-
- axios.get(url)
- .then(({ data }) => {
- Notes.renderDiffContent($container, data);
- })
- .catch(() => {
- Notes.renderDiffError($container);
- });
- }
-
toggleCommitList(e) {
const $element = $(e.currentTarget);
const $closestSystemCommitList = $element.siblings('.system-note-commit-list');
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index 8440945ab43..fff249577a2 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -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::Auth::OAuth::Provider.label_for(oauth['provider'])
flash[:alert] = "Signing in using #{label} has been disabled"
diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb
index cba9a53dc4b..ee507009e50 100644
--- a/app/controllers/projects/discussions_controller.rb
+++ b/app/controllers/projects/discussions_controller.rb
@@ -19,12 +19,6 @@ class Projects::DiscussionsController < Projects::ApplicationController
render_discussion
end
- def show
- render json: {
- discussion_html: view_to_html_string('discussions/_diff_with_notes', discussion: discussion, expanded: true)
- }
- end
-
private
def render_discussion
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 4c4d7cca8a5..63e4a5dc45c 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -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
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 0dee6df525d..4281d7d267f 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -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
diff --git a/app/models/project.rb b/app/models/project.rb
index 5f9d9785d64..43e006ad3ee 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -38,6 +38,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?,
diff --git a/app/models/project_services/assembla_service.rb b/app/models/project_services/assembla_service.rb
index ae6af732ed4..4234b8044e5 100644
--- a/app/models/project_services/assembla_service.rb
+++ b/app/models/project_services/assembla_service.rb
@@ -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
diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index 42939ea0ec8..54e4b3278db 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/app/models/project_services/bamboo_service.rb
@@ -117,14 +117,14 @@ 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,
- basic_auth: {
- username: username,
- password: password
- })
+ Gitlab::HTTP.get(url, verify: false,
+ basic_auth: {
+ username: username,
+ password: password
+ })
end
end
end
diff --git a/app/models/project_services/buildkite_service.rb b/app/models/project_services/buildkite_service.rb
index fc30f6e3365..d2aaff8817a 100644
--- a/app/models/project_services/buildkite_service.rb
+++ b/app/models/project_services/buildkite_service.rb
@@ -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']
diff --git a/app/models/project_services/campfire_service.rb b/app/models/project_services/campfire_service.rb
index 8d7a4fceb08..cb4af73807b 100644
--- a/app/models/project_services/campfire_service.rb
+++ b/app/models/project_services/campfire_service.rb
@@ -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
diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb
index c93f1632652..71b10fc6bc1 100644
--- a/app/models/project_services/drone_ci_service.rb
+++ b/app/models/project_services/drone_ci_service.rb
@@ -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']
diff --git a/app/models/project_services/external_wiki_service.rb b/app/models/project_services/external_wiki_service.rb
index 720ad61162e..1553f169827 100644
--- a/app/models/project_services/external_wiki_service.rb
+++ b/app/models/project_services/external_wiki_service.rb
@@ -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
diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb
index 5fb15c383ca..df6dcd90985 100644
--- a/app/models/project_services/issue_tracker_service.rb
+++ b/app/models/project_services/issue_tracker_service.rb
@@ -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)
diff --git a/app/models/project_services/mock_ci_service.rb b/app/models/project_services/mock_ci_service.rb
index 72ddf9a4be3..2221459c90b 100644
--- a/app/models/project_services/mock_ci_service.rb
+++ b/app/models/project_services/mock_ci_service.rb
@@ -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
diff --git a/app/models/project_services/packagist_service.rb b/app/models/project_services/packagist_service.rb
index f68a0c1a3c3..ba62a5b7ac0 100644
--- a/app/models/project_services/packagist_service.rb
+++ b/app/models/project_services/packagist_service.rb
@@ -1,6 +1,4 @@
class PackagistService < Service
- include HTTParty
-
prop_accessor :username, :token, :server
validates :username, presence: true, if: :activated?
diff --git a/app/models/project_services/pivotaltracker_service.rb b/app/models/project_services/pivotaltracker_service.rb
index f9dfa2e91c3..3476e7d2283 100644
--- a/app/models/project_services/pivotaltracker_service.rb
+++ b/app/models/project_services/pivotaltracker_service.rb
@@ -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: {
diff --git a/app/models/project_services/pushover_service.rb b/app/models/project_services/pushover_service.rb
index e3a1ca2d45f..8777a44b72f 100644
--- a/app/models/project_services/pushover_service.rb
+++ b/app/models/project_services/pushover_service.rb
@@ -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
diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb
index cbe137452bd..145313b8e71 100644
--- a/app/models/project_services/teamcity_service.rb
+++ b/app/models/project_services/teamcity_service.rb
@@ -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,10 +134,10 @@ class TeamcityService < CiService
end
def get_path(path)
- HTTParty.get(build_url(path), verify: false,
- basic_auth: {
- username: username,
- password: password
- })
+ Gitlab::HTTP.get(build_url(path), verify: false,
+ basic_auth: {
+ username: username,
+ password: password
+ })
end
end
diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb
index f2d676af5c3..a34024f4f80 100644
--- a/app/services/projects/import_service.rb
+++ b/app/services/projects/import_service.rb
@@ -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,
diff --git a/app/services/submit_usage_ping_service.rb b/app/services/submit_usage_ping_service.rb
index 2623f253d98..ac029fad7ea 100644
--- a/app/services/submit_usage_ping_service.rb
+++ b/app/services/submit_usage_ping_service.rb
@@ -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
diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb
index 36e589d5aa8..809ce1303d8 100644
--- a/app/services/web_hook_service.rb
+++ b/app/services/web_hook_service.rb
@@ -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
diff --git a/app/validators/importable_url_validator.rb b/app/validators/importable_url_validator.rb
index 37a314adee6..3ec1594e202 100644
--- a/app/validators/importable_url_validator.rb
+++ b/app/validators/importable_url_validator.rb
@@ -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
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 81d7db04a3c..54b39df8cf3 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -860,5 +860,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'
diff --git a/app/views/discussions/_diff_with_notes.html.haml b/app/views/discussions/_diff_with_notes.html.haml
index 8680ec2e298..f9bfc01f213 100644
--- a/app/views/discussions/_diff_with_notes.html.haml
+++ b/app/views/discussions/_diff_with_notes.html.haml
@@ -2,12 +2,8 @@
- blob = discussion.blob
- discussions = { discussion.original_line_code => [discussion] }
- diff_file_class = diff_file.text? ? 'text-file' : 'js-image-file'
-- diff_data = {}
-- expanded = discussion.expanded? || local_assigns.fetch(:expanded, nil)
-- unless expanded
- - diff_data = { lines_path: project_merge_request_discussion_path(discussion.project, discussion.noteable, discussion) }
-.diff-file.file-holder{ class: diff_file_class, data: diff_data }
+.diff-file.file-holder{ class: diff_file_class }
.js-file-title.file-title.file-title-flex-parent
.file-header-content
= render "projects/diffs/file_header", diff_file: diff_file, url: discussion_path(discussion), show_toggle: false
@@ -15,24 +11,17 @@
- if diff_file.text?
.diff-content.code.js-syntax-highlight
%table
- - if expanded
- - discussions = { discussion.original_line_code => [discussion] }
- = render partial: "projects/diffs/line",
- collection: discussion.truncated_diff_lines,
- as: :line,
- locals: { diff_file: diff_file,
- discussions: discussions,
- discussion_expanded: true,
- plain: true }
- - else
- %tr.line_holder.line-holder-placeholder
- %td.old_line.diff-line-num
- %td.new_line.diff-line-num
- %td.line_content
- .js-code-placeholder
- = render "discussions/diff_discussion", discussions: [discussion], expanded: true
+ = render partial: "projects/diffs/line",
+ collection: discussion.truncated_diff_lines,
+ as: :line,
+ locals: { diff_file: diff_file,
+ discussions: discussions,
+ discussion_expanded: true,
+ plain: true }
- else
- partial = (diff_file.new_file? || diff_file.deleted_file?) ? 'single_image_diff' : 'replaced_image_diff'
+
= render partial: "projects/diffs/#{partial}", locals: { diff_file: diff_file, position: discussion.position.to_json, click_to_comment: false }
+
.note-container
= render partial: "discussions/notes", locals: { discussion: discussion, show_toggle: false, show_image_comment_badge: true, disable_collapse_class: true }
diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index e9589213f80..8b9fa3d6b05 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -8,7 +8,7 @@
.discussion.js-toggle-container{ data: { discussion_id: discussion.id } }
.discussion-header
.discussion-actions
- %button.note-action-button.discussion-toggle-button.js-toggle-button{ type: "button", class: ("js-toggle-lazy-diff" unless expanded) }
+ %button.note-action-button.discussion-toggle-button.js-toggle-button{ type: "button" }
- if expanded
= icon("chevron-up")
- else
diff --git a/changelogs/unreleased/35475-lazy-diff.yml b/changelogs/unreleased/35475-lazy-diff.yml
deleted file mode 100644
index bafa66ebe39..00000000000
--- a/changelogs/unreleased/35475-lazy-diff.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: lazy load diffs on merge request discussions
-merge_request:
-author:
-type: performance
diff --git a/changelogs/unreleased/fix-auth0-unsafe-login.yml b/changelogs/unreleased/fix-auth0-unsafe-login.yml
new file mode 100644
index 00000000000..01c6ea69dcc
--- /dev/null
+++ b/changelogs/unreleased/fix-auth0-unsafe-login.yml
@@ -0,0 +1,5 @@
+---
+title: Fix GitLab Auth0 integration signing in the wrong user
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/fj-15329-services-callbacks-ssrf.yml b/changelogs/unreleased/fj-15329-services-callbacks-ssrf.yml
new file mode 100644
index 00000000000..7fa6f6a5874
--- /dev/null
+++ b/changelogs/unreleased/fj-15329-services-callbacks-ssrf.yml
@@ -0,0 +1,5 @@
+---
+title: Fixed some SSRF vulnerabilities in services, hooks and integrations
+merge_request: 2337
+author:
+type: security
diff --git a/changelogs/unreleased/remove-unnecessary-validate-project.yml b/changelogs/unreleased/remove-unnecessary-validate-project.yml
deleted file mode 100644
index ebc8da03dd8..00000000000
--- a/changelogs/unreleased/remove-unnecessary-validate-project.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: 'Remove unecessary validate: true from belongs_to :project'
-merge_request:
-author:
-type: fixed
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 24a87b44fc7..710fe0ec325 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -132,7 +132,7 @@ constraints(ProjectUrlConstrainer.new) do
post :bulk_update
end
- resources :discussions, only: [:show], constraints: { id: /\h{40}/ } do
+ resources :discussions, only: [], constraints: { id: /\h{40}/ } do
member do
post :resolve
delete :resolve, action: :unresolve
diff --git a/db/migrate/20180223120443_create_user_interacted_projects_table.rb b/db/migrate/20180223120443_create_user_interacted_projects_table.rb
index 20749940b1e..8da8cf68088 100644
--- a/db/migrate/20180223120443_create_user_interacted_projects_table.rb
+++ b/db/migrate/20180223120443_create_user_interacted_projects_table.rb
@@ -3,13 +3,15 @@ class CreateUserInteractedProjectsTable < ActiveRecord::Migration
DOWNTIME = false
- disable_ddl_transaction!
+ INDEX_NAME = 'user_interacted_projects_non_unique_index'
def up
create_table :user_interacted_projects, id: false do |t|
t.references :user, null: false
t.references :project, null: false
end
+
+ add_index :user_interacted_projects, [:project_id, :user_id], name: INDEX_NAME
end
def down
diff --git a/db/migrate/20180223144945_add_allow_local_requests_from_hooks_and_services_to_application_settings.rb b/db/migrate/20180223144945_add_allow_local_requests_from_hooks_and_services_to_application_settings.rb
new file mode 100644
index 00000000000..c994a54698b
--- /dev/null
+++ b/db/migrate/20180223144945_add_allow_local_requests_from_hooks_and_services_to_application_settings.rb
@@ -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
diff --git a/db/post_migrate/20180220150310_remove_empty_extern_uid_auth0_identities.rb b/db/post_migrate/20180220150310_remove_empty_extern_uid_auth0_identities.rb
new file mode 100644
index 00000000000..2d5a8617169
--- /dev/null
+++ b/db/post_migrate/20180220150310_remove_empty_extern_uid_auth0_identities.rb
@@ -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
diff --git a/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb b/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb
index 5e729b1aa53..d1a29a5c71b 100644
--- a/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb
+++ b/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb
@@ -1,9 +1,14 @@
+require_relative '../migrate/20180223120443_create_user_interacted_projects_table.rb'
+# rubocop:disable AddIndex
+# rubocop:disable AddConcurrentForeignKey
class BuildUserInteractedProjectsTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
+ UNIQUE_INDEX_NAME = 'index_user_interacted_projects_on_project_id_and_user_id'
+
disable_ddl_transaction!
def up
@@ -13,16 +18,8 @@ class BuildUserInteractedProjectsTable < ActiveRecord::Migration
MysqlStrategy.new
end.up
- unless index_exists?(:user_interacted_projects, [:project_id, :user_id])
- add_concurrent_index :user_interacted_projects, [:project_id, :user_id], unique: true
- end
-
- unless foreign_key_exists?(:user_interacted_projects, :user_id)
- add_concurrent_foreign_key :user_interacted_projects, :users, column: :user_id, on_delete: :cascade
- end
-
- unless foreign_key_exists?(:user_interacted_projects, :project_id)
- add_concurrent_foreign_key :user_interacted_projects, :projects, column: :project_id, on_delete: :cascade
+ if index_exists_by_name?(:user_interacted_projects, CreateUserInteractedProjectsTable::INDEX_NAME)
+ remove_concurrent_index_by_name :user_interacted_projects, CreateUserInteractedProjectsTable::INDEX_NAME
end
end
@@ -37,31 +34,16 @@ class BuildUserInteractedProjectsTable < ActiveRecord::Migration
remove_foreign_key :user_interacted_projects, :projects
end
- if index_exists_by_name?(:user_interacted_projects, 'index_user_interacted_projects_on_project_id_and_user_id')
- remove_concurrent_index_by_name :user_interacted_projects, 'index_user_interacted_projects_on_project_id_and_user_id'
+ if index_exists_by_name?(:user_interacted_projects, UNIQUE_INDEX_NAME)
+ remove_concurrent_index_by_name :user_interacted_projects, UNIQUE_INDEX_NAME
end
- end
- private
-
- # Rails' index_exists? doesn't work when you only give it a table and index
- # name. As such we have to use some extra code to check if an index exists for
- # a given name.
- def index_exists_by_name?(table, index)
- indexes_for_table[table].include?(index)
- end
-
- def indexes_for_table
- @indexes_for_table ||= Hash.new do |hash, table_name|
- hash[table_name] = indexes(table_name).map(&:name)
+ unless index_exists_by_name?(:user_interacted_projects, CreateUserInteractedProjectsTable::INDEX_NAME)
+ add_concurrent_index :user_interacted_projects, [:project_id, :user_id], name: CreateUserInteractedProjectsTable::INDEX_NAME
end
end
- def foreign_key_exists?(table, column)
- foreign_keys(table).any? do |key|
- key.options[:column] == column.to_s
- end
- end
+ private
class PostgresStrategy < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
@@ -71,33 +53,86 @@ class BuildUserInteractedProjectsTable < ActiveRecord::Migration
def up
with_index(:events, [:author_id, :project_id], name: 'events_user_interactions_temp', where: 'project_id IS NOT NULL') do
- iteration = 0
- records = 0
- begin
- Rails.logger.info "Building user_interacted_projects table, batch ##{iteration}"
- result = execute <<~SQL
+ insert_missing_records
+
+ # Do this once without lock to speed up the second invocation
+ remove_duplicates
+ with_table_lock(:user_interacted_projects) do
+ remove_duplicates
+ create_unique_index
+ end
+
+ remove_without_project
+ with_table_lock(:user_interacted_projects, :projects) do
+ remove_without_project
+ create_fk :user_interacted_projects, :projects, :project_id
+ end
+
+ remove_without_user
+ with_table_lock(:user_interacted_projects, :users) do
+ remove_without_user
+ create_fk :user_interacted_projects, :users, :user_id
+ end
+ end
+
+ execute "ANALYZE user_interacted_projects"
+ end
+
+ private
+ def insert_missing_records
+ iteration = 0
+ records = 0
+ begin
+ Rails.logger.info "Building user_interacted_projects table, batch ##{iteration}"
+ result = execute <<~SQL
INSERT INTO user_interacted_projects (user_id, project_id)
SELECT e.user_id, e.project_id
FROM (SELECT DISTINCT author_id AS user_id, project_id FROM events WHERE project_id IS NOT NULL) AS e
LEFT JOIN user_interacted_projects ucp USING (user_id, project_id)
WHERE ucp.user_id IS NULL
LIMIT #{BATCH_SIZE}
- SQL
- iteration += 1
- records += result.cmd_tuples
- Rails.logger.info "Building user_interacted_projects table, batch ##{iteration} complete, created #{records} overall"
- Kernel.sleep(SLEEP_TIME) if result.cmd_tuples > 0
- rescue ActiveRecord::InvalidForeignKey => e
- Rails.logger.info "Retry on InvalidForeignKey: #{e}"
- retry
- end while result.cmd_tuples > 0
- end
+ SQL
+ iteration += 1
+ records += result.cmd_tuples
+ Rails.logger.info "Building user_interacted_projects table, batch ##{iteration} complete, created #{records} overall"
+ Kernel.sleep(SLEEP_TIME) if result.cmd_tuples > 0
+ end while result.cmd_tuples > 0
+ end
- execute "ANALYZE user_interacted_projects"
+ def remove_duplicates
+ execute <<~SQL
+ WITH numbered AS (select ctid, ROW_NUMBER() OVER (PARTITION BY (user_id, project_id)) as row_number, user_id, project_id from user_interacted_projects)
+ DELETE FROM user_interacted_projects WHERE ctid IN (SELECT ctid FROM numbered WHERE row_number > 1);
+ SQL
+ end
+ def remove_without_project
+ execute "DELETE FROM user_interacted_projects WHERE NOT EXISTS (SELECT 1 FROM projects WHERE id = user_interacted_projects.project_id)"
end
- private
+ def remove_without_user
+ execute "DELETE FROM user_interacted_projects WHERE NOT EXISTS (SELECT 1 FROM users WHERE id = user_interacted_projects.user_id)"
+ end
+
+ def create_fk(table, target, column)
+ return if foreign_key_exists?(table, column)
+
+ add_foreign_key table, target, column: column, on_delete: :cascade
+ end
+
+ def create_unique_index
+ return if index_exists_by_name?(:user_interacted_projects, UNIQUE_INDEX_NAME)
+
+ add_index :user_interacted_projects, [:project_id, :user_id], unique: true, name: UNIQUE_INDEX_NAME
+ end
+
+ # Protect table against concurrent data changes while still allowing reads
+ def with_table_lock(*tables)
+ ActiveRecord::Base.connection.transaction do
+ execute "LOCK TABLE #{tables.join(", ")} IN SHARE MODE"
+ yield
+ end
+ end
def with_index(*args)
add_concurrent_index(*args) unless index_exists?(*args)
@@ -118,7 +153,18 @@ class BuildUserInteractedProjectsTable < ActiveRecord::Migration
LEFT JOIN user_interacted_projects ucp USING (user_id, project_id)
WHERE ucp.user_id IS NULL
SQL
+
+ unless index_exists?(:user_interacted_projects, [:project_id, :user_id])
+ add_concurrent_index :user_interacted_projects, [:project_id, :user_id], unique: true, name: UNIQUE_INDEX_NAME
+ end
+
+ unless foreign_key_exists?(:user_interacted_projects, :user_id)
+ add_concurrent_foreign_key :user_interacted_projects, :users, column: :user_id, on_delete: :cascade
+ end
+
+ unless foreign_key_exists?(:user_interacted_projects, :project_id)
+ add_concurrent_foreign_key :user_interacted_projects, :projects, column: :project_id, on_delete: :cascade
+ end
end
end
-
end
diff --git a/db/schema.rb b/db/schema.rb
index ab4370e2754..70d4a3b6de9 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -157,6 +157,7 @@ ActiveRecord::Schema.define(version: 20180309160427) do
t.boolean "authorized_keys_enabled", default: true, null: false
t.string "auto_devops_domain"
t.boolean "pages_domain_verification_enabled", default: true, null: false
+ t.boolean "allow_local_requests_from_hooks_and_services", default: false, null: false
end
create_table "audit_events", force: :cascade do |t|
diff --git a/doc/integration/auth0.md b/doc/integration/auth0.md
index c39d7ab57c6..a75836a915a 100644
--- a/doc/integration/auth0.md
+++ b/doc/integration/auth0.md
@@ -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' }
}
```
diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb
index dbe6259fce7..21287a8efd0 100644
--- a/lib/gitlab/database/migration_helpers.rb
+++ b/lib/gitlab/database/migration_helpers.rb
@@ -859,6 +859,19 @@ into similar problems in the future (e.g. when new tables are created).
BackgroundMigrationWorker.perform_in(delay_interval * index, job_class_name, [start_id, end_id])
end
end
+
+ def foreign_key_exists?(table, column)
+ foreign_keys(table).any? do |key|
+ key.options[:column] == column.to_s
+ end
+ end
+
+ # Rails' index_exists? doesn't work when you only give it a table and index
+ # name. As such we have to use some extra code to check if an index exists for
+ # a given name.
+ def index_exists_by_name?(table, index)
+ indexes(table).map(&:name).include?(index)
+ end
end
end
end
diff --git a/lib/gitlab/http.rb b/lib/gitlab/http.rb
new file mode 100644
index 00000000000..96558872a37
--- /dev/null
+++ b/lib/gitlab/http.rb
@@ -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
diff --git a/lib/gitlab/proxy_http_connection_adapter.rb b/lib/gitlab/proxy_http_connection_adapter.rb
new file mode 100644
index 00000000000..c70d6f4cd84
--- /dev/null
+++ b/lib/gitlab/proxy_http_connection_adapter.rb
@@ -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
diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb
index 13150ddab67..0f9f939e204 100644
--- a/lib/gitlab/url_blocker.rb
+++ b/lib/gitlab/url_blocker.rb
@@ -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
diff --git a/lib/mattermost/session.rb b/lib/mattermost/session.rb
index ef08bd46e17..00e461ed61a 100644
--- a/lib/mattermost/session.rb
+++ b/lib/mattermost/session.rb
@@ -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
diff --git a/lib/microsoft_teams/notifier.rb b/lib/microsoft_teams/notifier.rb
index 3bef68a1bcb..c08d3e933a8 100644
--- a/lib/microsoft_teams/notifier.rb
+++ b/lib/microsoft_teams/notifier.rb
@@ -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
diff --git a/rubocop/cop/gitlab/httparty.rb b/rubocop/cop/gitlab/httparty.rb
new file mode 100644
index 00000000000..215f18b6993
--- /dev/null
+++ b/rubocop/cop/gitlab/httparty.rb
@@ -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
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index 9110237c538..530fe3409fb 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -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'
diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb
index c639ad32ec6..9fd129e4ee9 100644
--- a/spec/controllers/omniauth_callbacks_controller_spec.rb
+++ b/spec/controllers/omniauth_callbacks_controller_spec.rb
@@ -3,73 +3,90 @@ 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
- it 'allows sign in' do
- post provider
+ context 'github' do
+ let(:extern_uid) { 'my-uid' }
+ let(:provider) { :github }
- expect(request.env['warden']).to be_authenticated
- end
+ it 'allows sign in' do
+ post provider
+
+ expect(request.env['warden']).to be_authenticated
+ end
- shared_context 'sign_up' do
- let(:user) { double(email: 'new@example.com') }
+ shared_context 'sign_up' do
+ let(:user) { double(email: 'new@example.com') }
- before do
- stub_omniauth_setting(block_auto_created_users: false)
+ before do
+ stub_omniauth_setting(block_auto_created_users: false)
+ end
end
- end
- context 'sign up' do
- include_context 'sign_up'
+ context 'sign up' do
+ include_context 'sign_up'
- it 'is allowed' do
- post provider
+ it 'is allowed' do
+ post provider
- expect(request.env['warden']).to be_authenticated
+ expect(request.env['warden']).to be_authenticated
+ end
end
- end
- context 'when OAuth is disabled' do
- before do
- stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
- settings = Gitlab::CurrentSettings.current_application_settings
- settings.update(disabled_oauth_sign_in_sources: [provider.to_s])
- end
+ context 'when OAuth is disabled' do
+ before do
+ stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+ settings = Gitlab::CurrentSettings.current_application_settings
+ settings.update(disabled_oauth_sign_in_sources: [provider.to_s])
+ end
- it 'prevents login via POST' do
- post provider
+ it 'prevents login via POST' do
+ post provider
- expect(request.env['warden']).not_to be_authenticated
- end
+ expect(request.env['warden']).not_to be_authenticated
+ end
- it 'shows warning when attempting login' do
- post provider
+ it 'shows warning when attempting login' do
+ post provider
- expect(response).to redirect_to new_user_session_path
- expect(flash[:alert]).to eq('Signing in using GitHub has been disabled')
- end
+ expect(response).to redirect_to new_user_session_path
+ expect(flash[:alert]).to eq('Signing in using GitHub has been disabled')
+ end
- it 'allows linking the disabled provider' do
- user.identities.destroy_all
- sign_in(user)
+ it 'allows linking the disabled provider' do
+ user.identities.destroy_all
+ sign_in(user)
- expect { post provider }.to change { user.reload.identities.count }.by(1)
- end
+ expect { post provider }.to change { user.reload.identities.count }.by(1)
+ end
- context 'sign up' do
- include_context 'sign_up'
+ context 'sign up' do
+ include_context 'sign_up'
- it 'is prevented' do
- post provider
+ it 'is prevented' do
+ post provider
- expect(request.env['warden']).not_to be_authenticated
+ expect(request.env['warden']).not_to be_authenticated
+ 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
diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
index b4ad4b64d8e..3e83a549682 100644
--- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
+++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
@@ -108,7 +108,6 @@ describe 'Merge request > User resolves diff notes and discussions', :js do
it 'shows resolved discussion when toggled' do
find(".timeline-content .discussion[data-discussion-id='#{note.discussion_id}'] .discussion-toggle-button").click
- expect(page.find(".line-holder-placeholder")).to be_visible
expect(page.find(".timeline-content #note_#{note.id}")).to be_visible
end
end
diff --git a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb
index 565e375600b..8a834adbf17 100644
--- a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb
+++ b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb
@@ -5,18 +5,15 @@ describe 'Merge request > User scrolls to note on load', :js do
let(:user) { project.creator }
let(:merge_request) { create(:merge_request, source_project: project, author: user) }
let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) }
- let(:resolved_note) { create(:diff_note_on_merge_request, :resolved, noteable: merge_request, project: project) }
let(:fragment_id) { "#note_#{note.id}" }
- let(:collapsed_fragment_id) { "#note_#{resolved_note.id}" }
before do
sign_in(user)
page.current_window.resize_to(1000, 300)
- end
-
- it 'scrolls note into view' do
visit "#{project_merge_request_path(project, merge_request)}#{fragment_id}"
+ end
+ it 'scrolls down to fragment' do
page_height = page.current_window.size[1]
page_scroll_y = page.evaluate_script("window.scrollY")
fragment_position_top = page.evaluate_script("Math.round($('#{fragment_id}').offset().top)")
@@ -26,13 +23,4 @@ describe 'Merge request > User scrolls to note on load', :js do
expect(fragment_position_top).to be >= page_scroll_y
expect(fragment_position_top).to be < (page_scroll_y + page_height)
end
-
- it 'expands collapsed notes' do
- visit "#{project_merge_request_path(project, merge_request)}#{collapsed_fragment_id}"
- note_element = find(collapsed_fragment_id)
- note_container = note_element.ancestor('.js-toggle-container')
-
- expect(note_element.visible?).to eq true
- expect(note_container.find('.line_content.noteable_line.old', match: :first).visible?).to eq true
- end
end
diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb
new file mode 100644
index 00000000000..b0bc081a3c8
--- /dev/null
+++ b/spec/lib/gitlab/http_spec.rb
@@ -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
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
index d9b3c2350b1..2d35b026485 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -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
diff --git a/spec/lib/mattermost/command_spec.rb b/spec/lib/mattermost/command_spec.rb
index 369e7b181b9..8ba15ae0f38 100644
--- a/spec/lib/mattermost/command_spec.rb
+++ b/spec/lib/mattermost/command_spec.rb
@@ -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
diff --git a/spec/lib/mattermost/session_spec.rb b/spec/lib/mattermost/session_spec.rb
index 3db19d06305..c855643c4d8 100644
--- a/spec/lib/mattermost/session_spec.rb
+++ b/spec/lib/mattermost/session_spec.rb
@@ -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
diff --git a/spec/lib/mattermost/team_spec.rb b/spec/lib/mattermost/team_spec.rb
index e638ad7a2c9..9795a0c3e39 100644
--- a/spec/lib/mattermost/team_spec.rb
+++ b/spec/lib/mattermost/team_spec.rb
@@ -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
diff --git a/spec/migrations/remove_empty_extern_uid_auth0_identities_spec.rb b/spec/migrations/remove_empty_extern_uid_auth0_identities_spec.rb
new file mode 100644
index 00000000000..441c4295a40
--- /dev/null
+++ b/spec/migrations/remove_empty_extern_uid_auth0_identities_spec.rb
@@ -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
diff --git a/spec/models/project_services/mattermost_slash_commands_service_spec.rb b/spec/models/project_services/mattermost_slash_commands_service_spec.rb
index a5bdf9a9337..05d33cd3874 100644
--- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb
+++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb
@@ -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
diff --git a/spec/rubocop/cop/gitlab/httparty_spec.rb b/spec/rubocop/cop/gitlab/httparty_spec.rb
new file mode 100644
index 00000000000..510839a21d7
--- /dev/null
+++ b/spec/rubocop/cop/gitlab/httparty_spec.rb
@@ -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
diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb
index 21910e69d2e..2ef2e61babc 100644
--- a/spec/services/web_hook_service_spec.rb
+++ b/spec/services/web_hook_service_spec.rb
@@ -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]