kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Merge branch 'georgekoltsov/55474-outbound-setting-system-hooks' into 'master'

Browse source 
Add outbound setting for system hooks

See merge request gitlab-org/gitlab-ce!31177
This commit is contained in:
Rémy Coutable 2019-08-05 08:55:16 +00:00
commit f74387d298
36 changed files with 259 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/helpers/application_settings_helper.rb
View file

@ -160,6 +160,8 @@ module ApplicationSettingsHelper
:akismet_api_key,
:akismet_enabled,
:allow_local_requests_from_hooks_and_services,
:allow_local_requests_from_web_hooks_and_services,
:allow_local_requests_from_system_hooks,
:dns_rebinding_protection_enabled,
:archive_builds_in_human_readable,
:authorized_keys_enabled,

3
app/models/application_setting_implementation.rb
View file

@ -21,7 +21,8 @@ module ApplicationSettingImplementation
{
after_sign_up_text: nil,
akismet_enabled: false,
allow_local_requests_from_hooks_and_services: false,
allow_local_requests_from_web_hooks_and_services: false,
allow_local_requests_from_system_hooks: true,
dns_rebinding_protection_enabled: true,
authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand
container_registry_token_expire_delay: 5,

4
app/models/hooks/system_hook.rb
View file

@ -14,8 +14,10 @@ class SystemHook < WebHook
default_value_for :repository_update_events, true
default_value_for :merge_requests_events, false
validates :url, system_hook_url: true
# Allow urls pointing localhost and the local network
def allow_local_requests?
true
Gitlab::CurrentSettings.allow_local_requests_from_system_hooks?
end
end

6
app/models/hooks/web_hook.rb
View file

@ -15,8 +15,8 @@ class WebHook < ApplicationRecord
has_many :web_hook_logs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
validates :url, presence: true, public_url: { allow_localhost: lambda(&:allow_local_requests?),
allow_local_network: lambda(&:allow_local_requests?) }
validates :url, presence: true
validates :url, public_url: true, unless: ->(hook) { hook.is_a?(SystemHook) }
validates :token, format: { without: /\n/ }
validates :push_events_branch_filter, branch_filter: true
@ -35,6 +35,6 @@ class WebHook < ApplicationRecord
# Allow urls pointing localhost and the local network
def allow_local_requests?
false
Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
end
end

6
app/services/web_hook_service.rb
View file

@ -17,8 +17,10 @@ class WebHookService
@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)
@request_options = {
timeout: Gitlab.config.gitlab.webhook_timeout,
allow_local_requests: hook.allow_local_requests?
}
end
def execute

2
app/validators/addressable_url_validator.rb
View file

@ -107,6 +107,6 @@ class AddressableUrlValidator < ActiveModel::EachValidator
# calls this validator.
#
# See https://gitlab.com/gitlab-org/gitlab-ee/issues/9833
ApplicationSetting.current&.allow_local_requests_from_hooks_and_services?
ApplicationSetting.current&.allow_local_requests_from_web_hooks_and_services?
end
end

18
app/validators/system_hook_url_validator.rb Normal file
View file

@ -0,0 +1,18 @@
# frozen_string_literal: true
# SystemHookUrlValidator
#
# Custom validator specific to SystemHook URLs. This validator works like AddressableUrlValidator but
# it blocks urls pointing to localhost or the local network depending on
# ApplicationSetting.allow_local_requests_from_system_hooks
#
# Example:
# class SystemHook < WebHook
# validates :url, system_hook_url: true
# end
#
class SystemHookUrlValidator < PublicUrlValidator
def self.allow_setting_local_requests?
ApplicationSetting.current&.allow_local_requests_from_system_hooks?
end
end

10
app/views/admin/application_settings/_outbound.html.haml
View file

@ -4,9 +4,13 @@
%fieldset
.form-group
.form-check
= f.check_box :allow_local_requests_from_hooks_and_services, class: 'form-check-input'
= f.label :allow_local_requests_from_hooks_and_services, class: 'form-check-label' do
Allow requests to the local network from hooks and services
= f.check_box :allow_local_requests_from_web_hooks_and_services, class: 'form-check-input'
= f.label :allow_local_requests_from_web_hooks_and_services, class: 'form-check-label' do
= _('Allow requests to the local network from web hooks and services')
.form-check
= f.check_box :allow_local_requests_from_system_hooks, class: 'form-check-input'
= f.label :allow_local_requests_from_system_hooks, class: 'form-check-label' do
= _('Allow requests to the local network from system hooks')
.form-group
= f.label :outbound_local_requests_whitelist_raw, class: 'label-bold' do

5
changelogs/unreleased/georgekoltsov-55474-outbound-setting-system-hooks.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Add new outbound network requests application setting for system hooks
merge_request: 31177
author:
type: added

17
db/migrate/20190726101050_rename_allow_local_requests_from_hooks_and_services_application_setting.rb Normal file
View file

@ -0,0 +1,17 @@
# frozen_string_literal: true
class RenameAllowLocalRequestsFromHooksAndServicesApplicationSetting < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
rename_column_concurrently :application_settings, :allow_local_requests_from_hooks_and_services, :allow_local_requests_from_web_hooks_and_services
end
def down
cleanup_concurrent_column_rename :application_settings, :allow_local_requests_from_web_hooks_and_services, :allow_local_requests_from_hooks_and_services
end
end

18
db/migrate/20190726101133_add_allow_local_requests_from_system_hooks_to_application_settings.rb Normal file
View file

@ -0,0 +1,18 @@
# frozen_string_literal: true
class AddAllowLocalRequestsFromSystemHooksToApplicationSettings < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
add_column(:application_settings, :allow_local_requests_from_system_hooks,
:boolean,
default: true,
null: false)
end
def down
remove_column(:application_settings, :allow_local_requests_from_system_hooks)
end
end

17
db/post_migrate/20190801114109_cleanup_allow_local_requests_from_hooks_and_services_application_setting_rename.rb Normal file
View file

@ -0,0 +1,17 @@
# frozen_string_literal: true
class CleanupAllowLocalRequestsFromHooksAndServicesApplicationSettingRename < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
cleanup_concurrent_column_rename :application_settings, :allow_local_requests_from_hooks_and_services, :allow_local_requests_from_web_hooks_and_services
end
def down
rename_column_concurrently :application_settings, :allow_local_requests_from_web_hooks_and_services, :allow_local_requests_from_hooks_and_services
end
end

5
db/schema.rb
View file

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2019_07_31_084415) do
ActiveRecord::Schema.define(version: 2019_08_01_114109) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm"
@ -183,7 +183,6 @@ ActiveRecord::Schema.define(version: 2019_07_31_084415) do
t.string "external_authorization_service_default_label"
t.boolean "pages_domain_verification_enabled", default: true, null: false
t.string "user_default_internal_regex"
t.boolean "allow_local_requests_from_hooks_and_services", default: false, null: false
t.float "external_authorization_service_timeout", default: 0.5
t.text "external_auth_client_cert"
t.text "encrypted_external_auth_client_key"
@ -230,6 +229,8 @@ ActiveRecord::Schema.define(version: 2019_07_31_084415) do
t.string "grafana_url", default: "/-/grafana", null: false
t.string "outbound_local_requests_whitelist", limit: 255, default: [], null: false, array: true
t.integer "raw_blob_request_limit", default: 300, null: false
t.boolean "allow_local_requests_from_web_hooks_and_services", default: false, null: false
t.boolean "allow_local_requests_from_system_hooks", default: true, null: false
t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id"

14
doc/api/settings.md
View file

@ -64,7 +64,10 @@ Example response:
"performance_bar_allowed_group_id": 42,
"instance_statistics_visibility_private": false,
"user_show_add_ssh_key_message": true,
"local_markdown_version": 0
"local_markdown_version": 0,
"allow_local_requests_from_hooks_and_services": true,
"allow_local_requests_from_web_hooks_and_services": true,
"allow_local_requests_from_system_hooks": false
}
```
@ -138,7 +141,10 @@ Example response:
"user_show_add_ssh_key_message": true,
"file_template_project_id": 1,
"local_markdown_version": 0,
"geo_node_allowed_ips": "0.0.0.0/0, ::/0"
"geo_node_allowed_ips": "0.0.0.0/0, ::/0",
"allow_local_requests_from_hooks_and_services": true,
"allow_local_requests_from_web_hooks_and_services": true,
"allow_local_requests_from_system_hooks": false
}
```
@ -177,7 +183,9 @@ are listed in the descriptions of the relevant settings.
| `akismet_api_key` | string | required by: `akismet_enabled` | API key for akismet spam protection. |
| `akismet_enabled` | boolean | no | (**If enabled, requires:** `akismet_api_key`) Enable or disable akismet spam protection. |
| `allow_group_owners_to_manage_ldap` | boolean | no | **(PREMIUM)** Set to `true` to allow group owners to manage LDAP |
| `allow_local_requests_from_hooks_and_services` | boolean | no | Allow requests to the local network from hooks and services. |
| `allow_local_requests_from_hooks_and_services` | boolean | no | (Deprecated: Use `allow_local_requests_from_web_hooks_and_services` instead) Allow requests to the local network from hooks and services. |
| `allow_local_requests_from_web_hooks_and_services` | boolean | no | Allow requests to the local network from web hooks and services. |
| `allow_local_requests_from_system_hooks` | boolean | no | Allow requests to the local network from system hooks. |
| `authorized_keys_enabled` | boolean | no | By default, we write to the `authorized_keys` file to support Git over SSH without additional configuration. GitLab can be optimized to authenticate SSH keys via the database file. Only disable this if you have configured your OpenSSH server to use the AuthorizedKeysCommand. |
| `auto_devops_domain` | string | no | Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages. |
| `auto_devops_enabled` | boolean | no | Enable Auto DevOps for projects by default. It will automatically build, test, and deploy applications based on a predefined CI/CD configuration. |

BIN
doc/security/img/outbound_requests_section.png
View file

Binary file not shown.
Side by side

Before

Width:  |  Height:  |  Size: 7.1 KiB

BIN
doc/security/img/outbound_requests_section_v12_2.png Normal file
View file

Binary file not shown.
Side by side

After

Width:  |  Height:  |  Size: 21 KiB

11
doc/security/webhooks.md
View file

@ -34,15 +34,16 @@ to 127.0.0.1, ::1 and 0.0.0.0, as well as IPv4 10.0.0.0/8, 172.16.0.0/12,
192.168.0.0/16 and IPv6 site-local (ffc0::/10) addresses won't be allowed.
This behavior can be overridden by enabling the option *"Allow requests to the
local network from hooks and services"* in the *"Outbound requests"* section
local network from web hooks and services"* in the *"Outbound requests"* section
inside the Admin area under **Settings**
(`/admin/application_settings/network`):
![Outbound requests admin settings](img/outbound_requests_section.png)
![Outbound requests admin settings](img/outbound_requests_section_v12_2.png)
>**Note:**
*System hooks* are exempt from this protection because they are set up by
admins.
NOTE: **Note:**
*System hooks* are enabled to make requests to local network by default since they are
set up by administrators. However, you can turn this off by disabling the
**Allow requests to the local network from system hooks** option.
<!-- ## Troubleshooting

2
lib/api/entities.rb
View file

@ -1162,6 +1162,7 @@ module API
attributes = ::ApplicationSettingsHelper.visible_attributes
attributes.delete(:performance_bar_allowed_group_path)
attributes.delete(:performance_bar_enabled)
attributes.delete(:allow_local_requests_from_hooks_and_services)
attributes
end
@ -1180,6 +1181,7 @@ module API
# support legacy names, can be removed in v5
expose :password_authentication_enabled_for_web, as: :password_authentication_enabled
expose :password_authentication_enabled_for_web, as: :signin_enabled
expose :allow_local_requests_from_web_hooks_and_services, as: :allow_local_requests_from_hooks_and_services
end
# deprecated old Release representation

6
lib/api/settings.rb
View file

@ -124,6 +124,7 @@ module API
optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
optional :instance_statistics_visibility_private, type: Boolean, desc: 'When set to `true` Instance statistics will only be available to admins'
optional :local_markdown_version, type: Integer, desc: "Local markdown version, increase this value when any cached markdown should be invalidated"
optional :allow_local_requests_from_hooks_and_services, type: Boolean, desc: 'Deprecated: Use :allow_local_requests_from_web_hooks_and_services instead. Allow requests to the local network from hooks and services.' # support legacy names, can be removed in v5
ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
optional :"#{type}_key_restriction",
@ -158,6 +159,11 @@ module API
attrs[:password_authentication_enabled_for_web] = attrs.delete(:password_authentication_enabled)
end
# support legacy names, can be removed in v5
if attrs.has_key?(:allow_local_requests_from_hooks_and_services)
attrs[:allow_local_requests_from_web_hooks_and_services] = attrs.delete(:allow_local_requests_from_hooks_and_services)
end
attrs = filter_attributes_using_license(attrs)
if ApplicationSettings::UpdateService.new(current_settings, current_user, attrs).execute

4
lib/gitlab/http_connection_adapter.rb
View file

@ -1,7 +1,7 @@
# frozen_string_literal: true
# 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
# of the global setting allow_local_requests_from_web_hooks_and_services this adapter
# will allow/block connection to internal IPs and/or urls.
#
# This functionality can be overridden by providing the setting the option
@ -38,7 +38,7 @@ module Gitlab
end
def allow_settings_local_requests?
Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
end
end
end

2
lib/gitlab/kubernetes/kube_client.rb
View file

@ -128,7 +128,7 @@ module Gitlab
private
def validate_url!
return if Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
return if Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
Gitlab::UrlBlocker.validate!(api_prefix, allow_local_network: false)
end

2
lib/gitlab/octokit/middleware.rb
View file

@ -16,7 +16,7 @@ module Gitlab
private
def allow_local_requests?
Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
end
end
end

6
locale/gitlab.pot
View file

@ -948,6 +948,12 @@ msgstr ""
msgid "Allow requests to the local network from hooks and services."
msgstr ""
msgid "Allow requests to the local network from system hooks"
msgstr ""
msgid "Allow requests to the local network from web hooks and services"
msgstr ""
msgid "Allow this key to push to repository as well? (Default only allows pull access.)"
msgstr ""

7
spec/features/admin/admin_settings_spec.rb
View file

@ -338,14 +338,17 @@ describe 'Admin updates settings' do
visit network_admin_application_settings_path
page.within('.as-outbound') do
check 'Allow requests to the local network from hooks and services'
check 'Allow requests to the local network from web hooks and services'
# Enabled by default
uncheck 'Allow requests to the local network from system hooks'
# Enabled by default
uncheck 'Enforce DNS rebinding attack protection'
click_button 'Save changes'
end
expect(page).to have_content "Application settings saved successfully"
expect(current_settings.allow_local_requests_from_hooks_and_services).to be true
expect(current_settings.allow_local_requests_from_web_hooks_and_services).to be true
expect(current_settings.allow_local_requests_from_system_hooks).to be false
expect(current_settings.dns_rebinding_protection_enabled).to be false
end
end

6
spec/lib/gitlab/http_spec.rb
View file

@ -23,14 +23,14 @@ describe Gitlab::HTTP do
end
end
describe 'allow_local_requests_from_hooks_and_services is' do
describe 'allow_local_requests_from_web_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)
allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(false)
end
it 'deny requests to localhost' do
@ -52,7 +52,7 @@ describe Gitlab::HTTP do
context 'enabled' do
before do
allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(true)
allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(true)
end
it 'allow requests to localhost' do

2
spec/lib/gitlab/kubernetes/kube_client_spec.rb
View file

@ -58,7 +58,7 @@ describe Gitlab::Kubernetes::KubeClient do
context 'when local requests are allowed' do
before do
stub_application_setting(allow_local_requests_from_hooks_and_services: true)
stub_application_setting(allow_local_requests_from_web_hooks_and_services: true)
end
it 'allows local addresses' do

8
spec/lib/gitlab/octokit/middleware_spec.rb
View file

@ -30,7 +30,7 @@ describe Gitlab::Octokit::Middleware do
context 'when localhost requests are not allowed' do
before do
stub_application_setting(allow_local_requests_from_hooks_and_services: false)
stub_application_setting(allow_local_requests_from_web_hooks_and_services: false)
end
it_behaves_like 'Local URL'
@ -38,7 +38,7 @@ describe Gitlab::Octokit::Middleware do
context 'when localhost requests are allowed' do
before do
stub_application_setting(allow_local_requests_from_hooks_and_services: true)
stub_application_setting(allow_local_requests_from_web_hooks_and_services: true)
end
it_behaves_like 'Public URL'
@ -50,7 +50,7 @@ describe Gitlab::Octokit::Middleware do
context 'when local network requests are not allowed' do
before do
stub_application_setting(allow_local_requests_from_hooks_and_services: false)
stub_application_setting(allow_local_requests_from_web_hooks_and_services: false)
end
it_behaves_like 'Local URL'
@ -58,7 +58,7 @@ describe Gitlab::Octokit::Middleware do
context 'when local network requests are allowed' do
before do
stub_application_setting(allow_local_requests_from_hooks_and_services: true)
stub_application_setting(allow_local_requests_from_web_hooks_and_services: true)
end
it_behaves_like 'Public URL'

2
spec/models/clusters/platforms/kubernetes_spec.rb
View file

@ -106,7 +106,7 @@ describe Clusters::Platforms::Kubernetes do
before do
allow(ApplicationSetting)
.to receive(:current)
.and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true))
.and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: true))
end
it { expect(kubernetes.save).to be_truthy }

2
spec/models/lfs_download_object_spec.rb
View file

@ -50,7 +50,7 @@ describe LfsDownloadObject do
before do
allow(ApplicationSetting)
.to receive(:current)
.and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: setting))
.and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: setting))
end
context 'are allowed' do

17
spec/requests/api/settings_spec.rb
View file

@ -25,6 +25,9 @@ describe API::Settings, 'Settings' do
expect(json_response['ed25519_key_restriction']).to eq(0)
expect(json_response['performance_bar_allowed_group_id']).to be_nil
expect(json_response['instance_statistics_visibility_private']).to be(false)
expect(json_response['allow_local_requests_from_hooks_and_services']).to be(false)
expect(json_response['allow_local_requests_from_web_hooks_and_services']).to be(false)
expect(json_response['allow_local_requests_from_system_hooks']).to be(true)
expect(json_response).not_to have_key('performance_bar_allowed_group_path')
expect(json_response).not_to have_key('performance_bar_enabled')
end
@ -67,7 +70,9 @@ describe API::Settings, 'Settings' do
instance_statistics_visibility_private: true,
diff_max_patch_bytes: 150_000,
default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE,
local_markdown_version: 3
local_markdown_version: 3,
allow_local_requests_from_web_hooks_and_services: true,
allow_local_requests_from_system_hooks: false
}
expect(response).to have_gitlab_http_status(200)
@ -95,6 +100,8 @@ describe API::Settings, 'Settings' do
expect(json_response['diff_max_patch_bytes']).to eq(150_000)
expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
expect(json_response['local_markdown_version']).to eq(3)
expect(json_response['allow_local_requests_from_web_hooks_and_services']).to eq(true)
expect(json_response['allow_local_requests_from_system_hooks']).to eq(false)
end
end
@ -117,6 +124,14 @@ describe API::Settings, 'Settings' do
expect(json_response['performance_bar_allowed_group_id']).to be_nil
end
it 'supports legacy allow_local_requests_from_hooks_and_services' do
put api("/application/settings", admin),
params: { allow_local_requests_from_hooks_and_services: true }
expect(response).to have_gitlab_http_status(200)
expect(json_response['allow_local_requests_from_hooks_and_services']).to eq(true)
end
context 'external policy classification settings' do
let(:settings) do
{

2
spec/services/projects/lfs_pointers/lfs_download_service_spec.rb
View file

@ -17,7 +17,7 @@ describe Projects::LfsPointers::LfsDownloadService do
before do
ApplicationSetting.create_from_defaults
stub_application_setting(allow_local_requests_from_hooks_and_services: local_request_setting)
stub_application_setting(allow_local_requests_from_web_hooks_and_services: local_request_setting)
allow(project).to receive(:lfs_enabled?).and_return(true)
end

4
spec/services/self_monitoring/project/create_service_spec.rb
View file

@ -37,7 +37,7 @@ describe SelfMonitoring::Project::CreateService do
allow(ApplicationSetting)
.to receive(:current)
.and_return(
ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true)
ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: true)
)
end
@ -95,7 +95,7 @@ describe SelfMonitoring::Project::CreateService do
allow(ApplicationSetting)
.to receive(:current)
.and_return(
ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: false)
ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: false)
)
end

34
spec/services/web_hook_service_spec.rb
View file

@ -19,16 +19,36 @@ describe WebHookService do
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
before do
stub_application_setting(setting_name => setting)
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
shared_examples_for 'respects outbound network setting' do
context 'when local requests are allowed' do
let(:setting) { true }
it { expect(hook.request_options[:allow_local_requests]).to be_truthy }
end
context 'when local requests are not allowed' do
let(:setting) { false }
it { expect(hook.request_options[:allow_local_requests]).to be_falsey }
end
end
context 'when SystemHook' do
let(:setting_name) { :allow_local_requests_from_system_hooks }
let(:hook) { described_class.new(build(:system_hook), data, :system_hook) }
include_examples 'respects outbound network setting'
end
context 'when ProjectHook' do
let(:setting_name) { :allow_local_requests_from_web_hooks_and_services }
let(:hook) { described_class.new(build(:project_hook), data, :project_hook) }
include_examples 'respects outbound network setting'
end
end

59
spec/support/shared_examples/url_validator_examples.rb
View file

@ -1,12 +1,12 @@
# frozen_string_literal: true
RSpec.shared_examples 'url validator examples' do |schemes|
let(:validator) { described_class.new(attributes: [:link_url], **options) }
let!(:badge) { build(:badge, link_url: 'http://www.example.com') }
subject { validator.validate(badge) }
describe '#validate' do
let(:validator) { described_class.new(attributes: [:link_url], **options) }
let(:badge) { build(:badge, link_url: 'http://www.example.com') }
subject { validator.validate(badge) }
context 'with no options' do
let(:options) { {} }
@ -42,3 +42,52 @@ RSpec.shared_examples 'url validator examples' do |schemes|
end
end
end
RSpec.shared_examples 'public url validator examples' do |setting|
let(:validator) { described_class.new(attributes: [:link_url]) }
let(:badge) { build(:badge, link_url: 'http://www.example.com') }
subject { validator.validate(badge) }
context 'by default' do
it 'blocks urls pointing to localhost' do
badge.link_url = 'https://127.0.0.1'
subject
expect(badge.errors).to be_present
end
it 'blocks urls pointing to the local network' do
badge.link_url = 'https://192.168.1.1'
subject
expect(badge.errors).to be_present
end
end
context 'when local requests are allowed' do
let!(:settings) { create(:application_setting) }
before do
stub_application_setting(setting)
end
it 'does not block urls pointing to localhost' do
badge.link_url = 'https://127.0.0.1'
subject
expect(badge.errors).not_to be_present
end
it 'does not block urls pointing to the local network' do
badge.link_url = 'https://192.168.1.1'
subject
expect(badge.errors).not_to be_present
end
end
end

24
spec/validators/public_url_validator_spec.rb
View file

@ -2,27 +2,5 @@ require 'spec_helper'
describe PublicUrlValidator do
include_examples 'url validator examples', AddressableUrlValidator::DEFAULT_OPTIONS[:schemes]
context 'by default' do
let(:validator) { described_class.new(attributes: [:link_url]) }
let!(:badge) { build(:badge, link_url: 'http://www.example.com') }
subject { validator.validate(badge) }
it 'blocks urls pointing to localhost' do
badge.link_url = 'https://127.0.0.1'
subject
expect(badge.errors).to be_present
end
it 'blocks urls pointing to the local network' do
badge.link_url = 'https://192.168.1.1'
subject
expect(badge.errors).to be_present
end
end
include_examples 'public url validator examples', allow_local_requests_from_web_hooks_and_services: true
end

8
spec/validators/system_hook_url_validator_spec.rb Normal file
View file

@ -0,0 +1,8 @@
# frozen_string_literal: true
require 'spec_helper'
describe SystemHookUrlValidator do
include_examples 'url validator examples', AddressableUrlValidator::DEFAULT_OPTIONS[:schemes]
include_examples 'public url validator examples', allow_local_requests_from_system_hooks: true
end