summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-08-26 07:43:05 +0000
committerGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-08-26 07:43:05 +0000
commit785ddcb2a47c53d22b5b7694f5b0bc14ca9cd2fb (patch)
tree85d0cfc0e241c770b08e6f695b98f0f3fa89f02e
parenta9ff1532818814fd9645fa8c673b3018ea1f91c6 (diff)
parent228608c2a226d777b32c129369f6b0b4329b31cf (diff)
downloadgitlab-ce-785ddcb2a47c53d22b5b7694f5b0bc14ca9cd2fb.tar.gz
Merge branch 'security-12-1-enable-image-proxy' into '12-1-stable'
Use image proxy to mitigate stealing ip addresses See merge request gitlab/gitlabhq!3231
-rw-r--r--app/helpers/application_settings_helper.rb4
-rw-r--r--app/models/application_setting.rb18
-rw-r--r--app/models/application_setting_implementation.rb38
-rw-r--r--app/services/application_settings/update_service.rb15
-rw-r--r--changelogs/unreleased/security-enable-image-proxy.yml5
-rw-r--r--config/initializers/asset_proxy_settings.rb6
-rw-r--r--config/initializers/fill_shards.rb4
-rw-r--r--db/migrate/20190219201635_add_asset_proxy_settings.rb16
-rw-r--r--db/schema.rb5
-rw-r--r--doc/api/settings.md17
-rw-r--r--doc/security/README.md1
-rw-r--r--doc/security/asset_proxy.md29
-rw-r--r--lib/api/entities.rb3
-rw-r--r--lib/api/settings.rb8
-rw-r--r--lib/api/validations/types/comma_separated_to_array.rb22
-rw-r--r--lib/banzai/filter/asset_proxy_filter.rb62
-rw-r--r--lib/banzai/filter/external_link_filter.rb18
-rw-r--r--lib/banzai/filter/image_link_filter.rb3
-rw-r--r--lib/banzai/filter/video_link_filter.rb15
-rw-r--r--lib/banzai/pipeline/ascii_doc_pipeline.rb5
-rw-r--r--lib/banzai/pipeline/gfm_pipeline.rb3
-rw-r--r--lib/banzai/pipeline/markup_pipeline.rb5
-rw-r--r--lib/banzai/pipeline/single_line_pipeline.rb3
-rw-r--r--spec/initializers/asset_proxy_setting_spec.rb13
-rw-r--r--spec/lib/banzai/filter/asset_proxy_filter_spec.rb95
-rw-r--r--spec/lib/banzai/filter/external_link_filter_spec.rb12
-rw-r--r--spec/lib/banzai/filter/image_link_filter_spec.rb7
-rw-r--r--spec/lib/banzai/filter/video_link_filter_spec.rb22
-rw-r--r--spec/lib/banzai/pipeline/gfm_pipeline_spec.rb44
-rw-r--r--spec/models/application_setting_spec.rb65
-rw-r--r--spec/requests/api/settings_spec.rb28
-rw-r--r--spec/services/application_settings/update_service_spec.rb33
-rw-r--r--spec/support/helpers/stub_configuration.rb4
33 files changed, 603 insertions, 25 deletions
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 4bf9b708401..91466c13a8f 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -162,6 +162,10 @@ module ApplicationSettingsHelper
:allow_local_requests_from_hooks_and_services,
:dns_rebinding_protection_enabled,
:archive_builds_in_human_readable,
+ :asset_proxy_enabled,
+ :asset_proxy_secret_key,
+ :asset_proxy_url,
+ :asset_proxy_whitelist,
:authorized_keys_enabled,
:auto_devops_enabled,
:auto_devops_domain,
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 8e558487c1c..d00d0b728f0 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -16,12 +16,19 @@ class ApplicationSetting < ApplicationRecord
# fix a lot of tests using allow_any_instance_of
include ApplicationSettingImplementation
+ attr_encrypted :asset_proxy_secret_key,
+ mode: :per_attribute_iv,
+ insecure_mode: true,
+ key: Settings.attr_encrypted_db_key_base_truncated,
+ algorithm: 'aes-256-cbc'
+
serialize :restricted_visibility_levels # rubocop:disable Cop/ActiveRecordSerialize
serialize :import_sources # rubocop:disable Cop/ActiveRecordSerialize
serialize :disabled_oauth_sign_in_sources, Array # rubocop:disable Cop/ActiveRecordSerialize
serialize :domain_whitelist, Array # rubocop:disable Cop/ActiveRecordSerialize
serialize :domain_blacklist, Array # rubocop:disable Cop/ActiveRecordSerialize
serialize :repository_storages # rubocop:disable Cop/ActiveRecordSerialize
+ serialize :asset_proxy_whitelist, Array # rubocop:disable Cop/ActiveRecordSerialize
ignore_column :koding_url
ignore_column :koding_enabled
@@ -180,6 +187,17 @@ class ApplicationSetting < ApplicationRecord
allow_nil: true,
numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than: 65536 }
+ validates :asset_proxy_url,
+ presence: true,
+ allow_blank: false,
+ url: true,
+ if: :asset_proxy_enabled?
+
+ validates :asset_proxy_secret_key,
+ presence: true,
+ allow_blank: false,
+ if: :asset_proxy_enabled?
+
SUPPORTED_KEY_TYPES.each do |type|
validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
end
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index df4caed175d..83546bb2a78 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -21,8 +21,9 @@ module ApplicationSettingImplementation
after_sign_up_text: nil,
akismet_enabled: false,
allow_local_requests_from_hooks_and_services: false,
- dns_rebinding_protection_enabled: true,
+ asset_proxy_enabled: false,
authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand
+ commit_email_hostname: default_commit_email_hostname,
container_registry_token_expire_delay: 5,
default_artifacts_expire_in: '30 days',
default_branch_protection: Settings.gitlab['default_branch_protection'],
@@ -31,7 +32,9 @@ module ApplicationSettingImplementation
default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'],
default_projects_limit: Settings.gitlab['default_projects_limit'],
default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'],
+ diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES,
disabled_oauth_sign_in_sources: [],
+ dns_rebinding_protection_enabled: true,
domain_whitelist: Settings.gitlab['domain_whitelist'],
dsa_key_restriction: 0,
ecdsa_key_restriction: 0,
@@ -50,6 +53,7 @@ module ApplicationSettingImplementation
housekeeping_gc_period: 200,
housekeeping_incremental_repack_period: 10,
import_sources: Settings.gitlab['import_sources'],
+ local_markdown_version: 0,
max_artifacts_size: Settings.artifacts['max_size'],
max_attachment_size: Settings.gitlab['max_attachment_size'],
mirror_available: true,
@@ -61,6 +65,7 @@ module ApplicationSettingImplementation
plantuml_url: nil,
polling_interval_multiplier: 1,
project_export_enabled: true,
+ protected_ci_variables: false,
recaptcha_enabled: false,
repository_checks_enabled: true,
repository_storages: ['default'],
@@ -92,11 +97,7 @@ module ApplicationSettingImplementation
user_default_external: false,
user_default_internal_regex: nil,
user_show_add_ssh_key_message: true,
- usage_stats_set_by_user_id: nil,
- diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES,
- commit_email_hostname: default_commit_email_hostname,
- protected_ci_variables: false,
- local_markdown_version: 0
+ usage_stats_set_by_user_id: nil
}
end
@@ -139,17 +140,20 @@ module ApplicationSettingImplementation
end
def domain_whitelist_raw=(values)
- self.domain_whitelist = []
- self.domain_whitelist = values.split(DOMAIN_LIST_SEPARATOR)
- self.domain_whitelist.reject! { |d| d.empty? }
- self.domain_whitelist
+ self.domain_whitelist = domain_string_to_array(values)
end
def domain_blacklist_raw=(values)
- self.domain_blacklist = []
- self.domain_blacklist = values.split(DOMAIN_LIST_SEPARATOR)
- self.domain_blacklist.reject! { |d| d.empty? }
- self.domain_blacklist
+ self.domain_blacklist = domain_string_to_array(values)
+ end
+
+ def asset_proxy_whitelist=(values)
+ values = domain_string_to_array(values) if values.is_a?(String)
+
+ # make sure we always whitelist the running host
+ values << Gitlab.config.gitlab.host unless values.include?(Gitlab.config.gitlab.host)
+
+ self[:asset_proxy_whitelist] = values
end
def domain_blacklist_file=(file)
@@ -276,4 +280,10 @@ module ApplicationSettingImplementation
def expire_performance_bar_allowed_user_ids_cache
Gitlab::PerformanceBar.expire_allowed_user_ids_cache
end
+
+ def domain_string_to_array(values)
+ domain_list = values.split(DOMAIN_LIST_SEPARATOR).map(&:strip)
+ domain_list.reject! { |d| d.empty? }
+ domain_list
+ end
end
diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb
index 7eeaf8aade1..04931108873 100644
--- a/app/services/application_settings/update_service.rb
+++ b/app/services/application_settings/update_service.rb
@@ -6,6 +6,8 @@ module ApplicationSettings
attr_reader :params, :application_setting
+ MARKDOWN_CACHE_INVALIDATING_PARAMS = %w(asset_proxy_enabled asset_proxy_url asset_proxy_secret_key asset_proxy_whitelist).freeze
+
def execute
validate_classification_label(application_setting, :external_authorization_service_default_label)
@@ -23,7 +25,13 @@ module ApplicationSettings
params[:usage_stats_set_by_user_id] = current_user.id
end
- @application_setting.update(@params)
+ @application_setting.assign_attributes(params)
+
+ if invalidate_markdown_cache?
+ @application_setting[:local_markdown_version] = @application_setting.local_markdown_version + 1
+ end
+
+ @application_setting.save
end
private
@@ -32,6 +40,11 @@ module ApplicationSettings
params.key?(:usage_ping_enabled) || params.key?(:version_check_enabled)
end
+ def invalidate_markdown_cache?
+ !params.key?(:local_markdown_version) &&
+ (@application_setting.changes.keys & MARKDOWN_CACHE_INVALIDATING_PARAMS).any?
+ end
+
def update_terms(terms)
return unless terms.present?
diff --git a/changelogs/unreleased/security-enable-image-proxy.yml b/changelogs/unreleased/security-enable-image-proxy.yml
new file mode 100644
index 00000000000..88b49ffd9e8
--- /dev/null
+++ b/changelogs/unreleased/security-enable-image-proxy.yml
@@ -0,0 +1,5 @@
+---
+title: Added image proxy to mitigate potential stealing of IP addresses
+merge_request:
+author:
+type: security
diff --git a/config/initializers/asset_proxy_settings.rb b/config/initializers/asset_proxy_settings.rb
new file mode 100644
index 00000000000..92247aba1b8
--- /dev/null
+++ b/config/initializers/asset_proxy_settings.rb
@@ -0,0 +1,6 @@
+#
+# Asset proxy settings
+#
+ActiveSupport.on_load(:active_record) do
+ Banzai::Filter::AssetProxyFilter.initialize_settings
+end
diff --git a/config/initializers/fill_shards.rb b/config/initializers/fill_shards.rb
index 18e067c8854..cad662e12f3 100644
--- a/config/initializers/fill_shards.rb
+++ b/config/initializers/fill_shards.rb
@@ -1,3 +1,5 @@
-if Shard.connected? && !Gitlab::Database.read_only?
+# The `table_exists?` check is needed because during our migration rollback testing,
+# `Shard.connected?` could be cached and return true even though the table doesn't exist
+if Shard.connected? && Shard.table_exists? && !Gitlab::Database.read_only?
Shard.populate!
end
diff --git a/db/migrate/20190219201635_add_asset_proxy_settings.rb b/db/migrate/20190219201635_add_asset_proxy_settings.rb
new file mode 100644
index 00000000000..beeb6400292
--- /dev/null
+++ b/db/migrate/20190219201635_add_asset_proxy_settings.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class AddAssetProxySettings < ActiveRecord::Migration[5.0]
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ def change
+ add_column :application_settings, :asset_proxy_enabled, :boolean, default: false, null: false
+ add_column :application_settings, :asset_proxy_url, :string
+ add_column :application_settings, :asset_proxy_whitelist, :text
+ add_column :application_settings, :encrypted_asset_proxy_secret_key, :text
+ add_column :application_settings, :encrypted_asset_proxy_secret_key_iv, :string
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 7779401b980..a68511b9c66 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -228,6 +228,11 @@ ActiveRecord::Schema.define(version: 2019_08_16_151221) do
t.boolean "lock_memberships_to_ldap", default: false, null: false
t.boolean "time_tracking_limit_to_hours", default: false, null: false
t.string "grafana_url", default: "/-/grafana", null: false
+ t.boolean "asset_proxy_enabled", default: false, null: false
+ t.string "asset_proxy_url"
+ t.text "asset_proxy_whitelist"
+ t.text "encrypted_asset_proxy_secret_key"
+ t.string "encrypted_asset_proxy_secret_key_iv"
t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id", using: :btree
t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id", using: :btree
t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree
diff --git a/doc/api/settings.md b/doc/api/settings.md
index ff48cac1f47..458a08edf83 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -39,6 +39,7 @@ Example response:
"session_expire_delay" : 10080,
"home_page_url" : null,
"default_snippet_visibility" : "private",
+ "outbound_local_requests_whitelist": [],
"domain_whitelist" : [],
"domain_blacklist_enabled" : false,
"domain_blacklist" : [],
@@ -63,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,
+ "asset_proxy_enabled": true,
+ "asset_proxy_url": "https://assets.example.com",
+ "asset_proxy_whitelist": ["example.com", "*.example.com", "your-instance.com"]
}
```
@@ -113,6 +117,7 @@ Example response:
"default_project_visibility": "internal",
"default_snippet_visibility": "private",
"default_group_visibility": "private",
+ "outbound_local_requests_whitelist": [],
"domain_whitelist": [],
"domain_blacklist_enabled" : false,
"domain_blacklist" : [],
@@ -136,6 +141,9 @@ Example response:
"user_show_add_ssh_key_message": true,
"file_template_project_id": 1,
"local_markdown_version": 0,
+ "asset_proxy_enabled": true,
+ "asset_proxy_url": "https://assets.example.com",
+ "asset_proxy_whitelist": ["example.com", "*.example.com", "your-instance.com"],
"geo_node_allowed_ips": "0.0.0.0/0, ::/0"
}
```
@@ -176,6 +184,10 @@ are listed in the descriptions of the relevant settings.
| `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. |
+| `asset_proxy_enabled` | boolean | no | (**If enabled, requires:** `asset_proxy_url`) Enable proxying of assets. GitLab restart is required to apply changes. |
+| `asset_proxy_secret_key` | string | no | Shared secret with the asset proxy server. GitLab restart is required to apply changes. |
+| `asset_proxy_url` | string | no | URL of the asset proxy server. GitLab restart is required to apply changes. |
+| `asset_proxy_whitelist` | string or array of strings | no | Assets that match these domain(s) will NOT be proxied. Wildcards allowed. Your GitLab installation URL is automatically whitelisted. GitLab restart is required to apply changes. |
| `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. |
@@ -193,6 +205,7 @@ are listed in the descriptions of the relevant settings.
| `domain_blacklist` | array of strings | required by: `domain_blacklist_enabled` | Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: `domain.com`, `*.domain.com`. |
| `domain_blacklist_enabled` | boolean | no | (**If enabled, requires:** `domain_blacklist`) Allows blocking sign-ups from emails from specific domains. |
| `domain_whitelist` | array of strings | no | Force people to use only corporate emails for sign-up. Default is `null`, meaning there is no restriction. |
+| `outbound_local_requests_whitelist` | array of strings | no | Define a list of trusted domains or ip addresses to which local requests are allowed when local requests for hooks and services are disabled.
| `dsa_key_restriction` | integer | no | The minimum allowed bit length of an uploaded DSA key. Default is `0` (no restriction). `-1` disables DSA keys. |
| `ecdsa_key_restriction` | integer | no | The minimum allowed curve size (in bits) of an uploaded ECDSA key. Default is `0` (no restriction). `-1` disables ECDSA keys. |
| `ed25519_key_restriction` | integer | no | The minimum allowed curve size (in bits) of an uploaded ED25519 key. Default is `0` (no restriction). `-1` disables ED25519 keys. |
@@ -227,7 +240,7 @@ are listed in the descriptions of the relevant settings.
| `gravatar_enabled` | boolean | no | Enable Gravatar. |
| `hashed_storage_enabled` | boolean | no | Create new projects using hashed storage paths: Enable immutable, hash-based paths and repository names to store repositories on disk. This prevents repositories from having to be moved or renamed when the Project URL changes and may improve disk I/O performance. (EXPERIMENTAL) |
| `help_page_hide_commercial_content` | boolean | no | Hide marketing-related entries from help. |
-| `help_page_support_url` | string | no | Alternate support URL for help page. |
+| `help_page_support_url` | string | no | Alternate support URL for help page and help dropdown. |
| `help_page_text` | string | no | Custom text displayed on the help page. |
| `help_text` | string | no | **(PREMIUM)** GitLab server administrator information |
| `hide_third_party_offers` | boolean | no | Do not display offers from third parties within GitLab. |
diff --git a/doc/security/README.md b/doc/security/README.md
index c48d5bc2065..77c2e465ebe 100644
--- a/doc/security/README.md
+++ b/doc/security/README.md
@@ -17,3 +17,4 @@ type: index
- [Enforce Two-factor authentication](two_factor_authentication.md)
- [Send email confirmation on sign-up](user_email_confirmation.md)
- [Security of running jobs](https://docs.gitlab.com/runner/security/)
+- [Proxying images](asset_proxy.md)
diff --git a/doc/security/asset_proxy.md b/doc/security/asset_proxy.md
new file mode 100644
index 00000000000..48cf9f59a7d
--- /dev/null
+++ b/doc/security/asset_proxy.md
@@ -0,0 +1,29 @@
+A possible security concern when managing a public facing GitLab instance is
+the ability to steal a users IP address by referencing images in issues, comments, etc.
+
+For example, adding `![Example image](http://example.com/example.png)` to
+an issue description will cause the image to be loaded from the external
+server in order to be displayed. However this also allows the external server
+to log the IP address of the user.
+
+One way to mitigate this is by proxying any external images to a server you
+control. GitLab handles this by allowing you to run the "Camo" server
+[cactus/go-camo](https://github.com/cactus/go-camo#how-it-works).
+The image request is sent to the Camo server, which then makes the request for
+the original image. This way an attacker only ever seems the IP address
+of your Camo server.
+
+Once you have your Camo server up and running, you can configure GitLab to
+proxy image requests to it. The following settings are supported:
+
+| Attribute | Description |
+| --------- | ----------- |
+| `asset_proxy_enabled` | (**If enabled, requires:** `asset_proxy_url`) Enable proxying of assets. |
+| `asset_proxy_secret_key` | Shared secret with the asset proxy server. |
+| `asset_proxy_url` | URL of the asset proxy server. |
+| `asset_proxy_whitelist` | Assets that match these domain(s) will NOT be proxied. Wildcards allowed. Your GitLab installation URL is automatically whitelisted. |
+
+
+These can be set via the [Application setting API](../api/settings.md)
+
+Note that a GitLab restart is required to apply any changes.
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 494da770279..2945b1054f0 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1160,6 +1160,9 @@ module API
attributes.delete(:performance_bar_allowed_group_path)
attributes.delete(:performance_bar_enabled)
+ # let's not expose the secret key in a response
+ attributes.delete(:asset_proxy_secret_key)
+
attributes
end
diff --git a/lib/api/settings.rb b/lib/api/settings.rb
index 4275d911708..c16ded0c10d 100644
--- a/lib/api/settings.rb
+++ b/lib/api/settings.rb
@@ -36,6 +36,10 @@ module API
given akismet_enabled: ->(val) { val } do
requires :akismet_api_key, type: String, desc: 'Generate API key at http://www.akismet.com'
end
+ optional :asset_proxy_enabled, type: Boolean, desc: 'Enable proxying of assets'
+ optional :asset_proxy_url, type: String, desc: 'URL of the asset proxy server'
+ optional :asset_proxy_secret_key, type: String, desc: 'Shared secret with the asset proxy server'
+ optional :asset_proxy_whitelist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Assets that match these domain(s) will NOT be proxied. Wildcards allowed. Your GitLab installation URL is automatically whitelisted.'
optional :container_registry_token_expire_delay, type: Integer, desc: 'Authorization token duration (minutes)'
optional :default_artifacts_expire_in, type: String, desc: "Set the default expiration time for each job's artifacts"
optional :default_project_creation, type: Integer, values: ::Gitlab::Access.project_creation_values, desc: 'Determine if developers can create projects in the group'
@@ -59,7 +63,7 @@ module API
optional :grafana_url, type: String, desc: 'Grafana URL'
optional :gravatar_enabled, type: Boolean, desc: 'Flag indicating if the Gravatar service is enabled'
optional :help_page_hide_commercial_content, type: Boolean, desc: 'Hide marketing-related entries from help'
- optional :help_page_support_url, type: String, desc: 'Alternate support URL for help page'
+ optional :help_page_support_url, type: String, desc: 'Alternate support URL for help page and help dropdown'
optional :help_page_text, type: String, desc: 'Custom text displayed on the help page'
optional :home_page_url, type: String, desc: 'We will redirect non-logged in users to this page'
optional :housekeeping_enabled, type: Boolean, desc: 'Enable automatic repository housekeeping (git repack, git gc)'
@@ -123,7 +127,7 @@ module API
optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.'
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 :local_markdown_version, type: Integer, desc: 'Local markdown version, increase this value when any cached markdown should be invalidated'
ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
optional :"#{type}_key_restriction",
diff --git a/lib/api/validations/types/comma_separated_to_array.rb b/lib/api/validations/types/comma_separated_to_array.rb
new file mode 100644
index 00000000000..b551878abd1
--- /dev/null
+++ b/lib/api/validations/types/comma_separated_to_array.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module API
+ module Validations
+ module Types
+ class CommaSeparatedToArray
+ def self.coerce
+ lambda do |value|
+ case value
+ when String
+ value.split(',').map(&:strip)
+ when Array
+ value.map { |v| v.to_s.split(',').map(&:strip) }.flatten
+ else
+ []
+ end
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/banzai/filter/asset_proxy_filter.rb b/lib/banzai/filter/asset_proxy_filter.rb
new file mode 100644
index 00000000000..0a9a52a73a1
--- /dev/null
+++ b/lib/banzai/filter/asset_proxy_filter.rb
@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+module Banzai
+ module Filter
+ # Proxy's images/assets to another server. Reduces mixed content warnings
+ # as well as hiding the customer's IP address when requesting images.
+ # Copies the original img `src` to `data-canonical-src` then replaces the
+ # `src` with a new url to the proxy server.
+ class AssetProxyFilter < HTML::Pipeline::CamoFilter
+ def initialize(text, context = nil, result = nil)
+ super
+ end
+
+ def validate
+ needs(:asset_proxy, :asset_proxy_secret_key) if asset_proxy_enabled?
+ end
+
+ def asset_host_whitelisted?(host)
+ context[:asset_proxy_domain_regexp] ? context[:asset_proxy_domain_regexp].match?(host) : false
+ end
+
+ def self.transform_context(context)
+ context[:disable_asset_proxy] = !Gitlab.config.asset_proxy.enabled
+
+ unless context[:disable_asset_proxy]
+ context[:asset_proxy_enabled] = !context[:disable_asset_proxy]
+ context[:asset_proxy] = Gitlab.config.asset_proxy.url
+ context[:asset_proxy_secret_key] = Gitlab.config.asset_proxy.secret_key
+ context[:asset_proxy_domain_regexp] = Gitlab.config.asset_proxy.domain_regexp
+ end
+
+ context
+ end
+
+ # called during an initializer. Caching the values in Gitlab.config significantly increased
+ # performance, rather than querying Gitlab::CurrentSettings.current_application_settings
+ # over and over. However, this does mean that the Rails servers need to get restarted
+ # whenever the application settings are changed
+ def self.initialize_settings
+ application_settings = Gitlab::CurrentSettings.current_application_settings
+ Gitlab.config['asset_proxy'] ||= Settingslogic.new({})
+
+ if application_settings.respond_to?(:asset_proxy_enabled)
+ Gitlab.config.asset_proxy['enabled'] = application_settings.asset_proxy_enabled
+ Gitlab.config.asset_proxy['url'] = application_settings.asset_proxy_url
+ Gitlab.config.asset_proxy['secret_key'] = application_settings.asset_proxy_secret_key
+ Gitlab.config.asset_proxy['whitelist'] = application_settings.asset_proxy_whitelist || [Gitlab.config.gitlab.host]
+ Gitlab.config.asset_proxy['domain_regexp'] = compile_whitelist(Gitlab.config.asset_proxy.whitelist)
+ else
+ Gitlab.config.asset_proxy['enabled'] = ::ApplicationSetting.defaults[:asset_proxy_enabled]
+ end
+ end
+
+ def self.compile_whitelist(domain_list)
+ return if domain_list.empty?
+
+ escaped = domain_list.map { |domain| Regexp.escape(domain).gsub('\*', '.*?') }
+ Regexp.new("^(#{escaped.join('|')})$", Regexp::IGNORECASE)
+ end
+ end
+ end
+end
diff --git a/lib/banzai/filter/external_link_filter.rb b/lib/banzai/filter/external_link_filter.rb
index 61ee3eac216..fb721fe12b1 100644
--- a/lib/banzai/filter/external_link_filter.rb
+++ b/lib/banzai/filter/external_link_filter.rb
@@ -14,10 +14,10 @@ module Banzai
# such as on `mailto:` links. Since we've been using it, do an
# initial parse for validity and then use Addressable
# for IDN support, etc
- uri = uri_strict(node['href'].to_s)
+ uri = uri_strict(node_src(node))
if uri
- node.set_attribute('href', uri.to_s)
- addressable_uri = addressable_uri(node['href'])
+ node.set_attribute(node_src_attribute(node), uri.to_s)
+ addressable_uri = addressable_uri(node_src(node))
else
addressable_uri = nil
end
@@ -35,6 +35,16 @@ module Banzai
private
+ # if this is a link to a proxied image, then `src` is already the correct
+ # proxied url, so work with the `data-canonical-src`
+ def node_src_attribute(node)
+ node['data-canonical-src'] ? 'data-canonical-src' : 'href'
+ end
+
+ def node_src(node)
+ node[node_src_attribute(node)]
+ end
+
def uri_strict(href)
URI.parse(href)
rescue URI::Error
@@ -72,7 +82,7 @@ module Banzai
return unless uri
return unless context[:emailable_links]
- unencoded_uri_str = Addressable::URI.unencode(node['href'])
+ unencoded_uri_str = Addressable::URI.unencode(node_src(node))
if unencoded_uri_str == node.content && idn?(uri)
node.content = uri.normalize
diff --git a/lib/banzai/filter/image_link_filter.rb b/lib/banzai/filter/image_link_filter.rb
index 01237303c27..ed0a01e6277 100644
--- a/lib/banzai/filter/image_link_filter.rb
+++ b/lib/banzai/filter/image_link_filter.rb
@@ -18,6 +18,9 @@ module Banzai
rel: 'noopener noreferrer'
)
+ # make sure the original non-proxied src carries over to the link
+ link['data-canonical-src'] = img['data-canonical-src'] if img['data-canonical-src']
+
link.children = img.clone
img.replace(link)
diff --git a/lib/banzai/filter/video_link_filter.rb b/lib/banzai/filter/video_link_filter.rb
index 0fff104cf91..a278fcfdb47 100644
--- a/lib/banzai/filter/video_link_filter.rb
+++ b/lib/banzai/filter/video_link_filter.rb
@@ -23,6 +23,14 @@ module Banzai
"'.#{ext}' = substring(@src, string-length(@src) - #{ext.size})"
end
+ if context[:asset_proxy_enabled].present?
+ src_query.concat(
+ UploaderHelper::VIDEO_EXT.map do |ext|
+ "'.#{ext}' = substring(@data-canonical-src, string-length(@data-canonical-src) - #{ext.size})"
+ end
+ )
+ end
+
"descendant-or-self::img[not(ancestor::a) and (#{src_query.join(' or ')})]"
end
end
@@ -48,6 +56,13 @@ module Banzai
target: '_blank',
rel: 'noopener noreferrer',
title: "Download '#{element['title'] || element['alt']}'")
+
+ # make sure the original non-proxied src carries over
+ if element['data-canonical-src']
+ video['data-canonical-src'] = element['data-canonical-src']
+ link['data-canonical-src'] = element['data-canonical-src']
+ end
+
download_paragraph = doc.document.create_element('p')
download_paragraph.children = link
diff --git a/lib/banzai/pipeline/ascii_doc_pipeline.rb b/lib/banzai/pipeline/ascii_doc_pipeline.rb
index d25b74b23b2..82b99d3de4a 100644
--- a/lib/banzai/pipeline/ascii_doc_pipeline.rb
+++ b/lib/banzai/pipeline/ascii_doc_pipeline.rb
@@ -6,12 +6,17 @@ module Banzai
def self.filters
FilterArray[
Filter::AsciiDocSanitizationFilter,
+ Filter::AssetProxyFilter,
Filter::SyntaxHighlightFilter,
Filter::ExternalLinkFilter,
Filter::PlantumlFilter,
Filter::AsciiDocPostProcessingFilter
]
end
+
+ def self.transform_context(context)
+ Filter::AssetProxyFilter.transform_context(context)
+ end
end
end
end
diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb
index 2c1006f708a..f419e54c264 100644
--- a/lib/banzai/pipeline/gfm_pipeline.rb
+++ b/lib/banzai/pipeline/gfm_pipeline.rb
@@ -17,6 +17,7 @@ module Banzai
Filter::SpacedLinkFilter,
Filter::SanitizationFilter,
+ Filter::AssetProxyFilter,
Filter::SyntaxHighlightFilter,
Filter::MathFilter,
@@ -60,7 +61,7 @@ module Banzai
def self.transform_context(context)
context[:only_path] = true unless context.key?(:only_path)
- context
+ Filter::AssetProxyFilter.transform_context(context)
end
end
end
diff --git a/lib/banzai/pipeline/markup_pipeline.rb b/lib/banzai/pipeline/markup_pipeline.rb
index ceba082cd4f..c86d5f08ded 100644
--- a/lib/banzai/pipeline/markup_pipeline.rb
+++ b/lib/banzai/pipeline/markup_pipeline.rb
@@ -6,11 +6,16 @@ module Banzai
def self.filters
@filters ||= FilterArray[
Filter::SanitizationFilter,
+ Filter::AssetProxyFilter,
Filter::ExternalLinkFilter,
Filter::PlantumlFilter,
Filter::SyntaxHighlightFilter
]
end
+
+ def self.transform_context(context)
+ Filter::AssetProxyFilter.transform_context(context)
+ end
end
end
end
diff --git a/lib/banzai/pipeline/single_line_pipeline.rb b/lib/banzai/pipeline/single_line_pipeline.rb
index 72374207a8f..9aff6880f56 100644
--- a/lib/banzai/pipeline/single_line_pipeline.rb
+++ b/lib/banzai/pipeline/single_line_pipeline.rb
@@ -7,6 +7,7 @@ module Banzai
@filters ||= FilterArray[
Filter::HtmlEntityFilter,
Filter::SanitizationFilter,
+ Filter::AssetProxyFilter,
Filter::EmojiFilter,
Filter::AutolinkFilter,
@@ -29,6 +30,8 @@ module Banzai
end
def self.transform_context(context)
+ context = Filter::AssetProxyFilter.transform_context(context)
+
super(context).merge(
no_sourcepos: true
)
diff --git a/spec/initializers/asset_proxy_setting_spec.rb b/spec/initializers/asset_proxy_setting_spec.rb
new file mode 100644
index 00000000000..42e4d4aa594
--- /dev/null
+++ b/spec/initializers/asset_proxy_setting_spec.rb
@@ -0,0 +1,13 @@
+require 'spec_helper'
+
+describe 'Asset proxy settings initialization' do
+ describe '#asset_proxy' do
+ it 'defaults to disabled' do
+ expect(Banzai::Filter::AssetProxyFilter).to receive(:initialize_settings)
+
+ require_relative '../../config/initializers/asset_proxy_settings'
+
+ expect(Gitlab.config.asset_proxy.enabled).to be_falsey
+ end
+ end
+end
diff --git a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb
new file mode 100644
index 00000000000..b7f45421b2a
--- /dev/null
+++ b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb
@@ -0,0 +1,95 @@
+require 'spec_helper'
+
+describe Banzai::Filter::AssetProxyFilter do
+ include FilterSpecHelper
+
+ def image(path)
+ %(<img src="#{path}" />)
+ end
+
+ it 'does not replace if disabled' do
+ stub_asset_proxy_setting(enabled: false)
+
+ context = described_class.transform_context({})
+ src = 'http://example.com/test.png'
+ doc = filter(image(src), context)
+
+ expect(doc.at_css('img')['src']).to eq src
+ end
+
+ context 'during initialization' do
+ after do
+ Gitlab.config.asset_proxy['enabled'] = false
+ end
+
+ it '#initialize_settings' do
+ stub_application_setting(asset_proxy_enabled: true)
+ stub_application_setting(asset_proxy_secret_key: 'shared-secret')
+ stub_application_setting(asset_proxy_url: 'https://assets.example.com')
+ stub_application_setting(asset_proxy_whitelist: %w(gitlab.com *.mydomain.com))
+
+ described_class.initialize_settings
+
+ expect(Gitlab.config.asset_proxy.enabled).to be_truthy
+ expect(Gitlab.config.asset_proxy.secret_key).to eq 'shared-secret'
+ expect(Gitlab.config.asset_proxy.url).to eq 'https://assets.example.com'
+ expect(Gitlab.config.asset_proxy.whitelist).to eq %w(gitlab.com *.mydomain.com)
+ expect(Gitlab.config.asset_proxy.domain_regexp).to eq /^(gitlab\.com|.*?\.mydomain\.com)$/i
+ end
+ end
+
+ context 'when properly configured' do
+ before do
+ stub_asset_proxy_setting(enabled: true)
+ stub_asset_proxy_setting(secret_key: 'shared-secret')
+ stub_asset_proxy_setting(url: 'https://assets.example.com')
+ stub_asset_proxy_setting(whitelist: %W(gitlab.com *.mydomain.com #{Gitlab.config.gitlab.host}))
+ stub_asset_proxy_setting(domain_regexp: described_class.compile_whitelist(Gitlab.config.asset_proxy.whitelist))
+ @context = described_class.transform_context({})
+ end
+
+ it 'replaces img src' do
+ src = 'http://example.com/test.png'
+ new_src = 'https://assets.example.com/08df250eeeef1a8cf2c761475ac74c5065105612/687474703a2f2f6578616d706c652e636f6d2f746573742e706e67'
+ doc = filter(image(src), @context)
+
+ expect(doc.at_css('img')['src']).to eq new_src
+ expect(doc.at_css('img')['data-canonical-src']).to eq src
+ end
+
+ it 'skips internal images' do
+ src = "#{Gitlab.config.gitlab.url}/test.png"
+ doc = filter(image(src), @context)
+
+ expect(doc.at_css('img')['src']).to eq src
+ end
+
+ it 'skip relative urls' do
+ src = "/test.png"
+ doc = filter(image(src), @context)
+
+ expect(doc.at_css('img')['src']).to eq src
+ end
+
+ it 'skips single domain' do
+ src = "http://gitlab.com/test.png"
+ doc = filter(image(src), @context)
+
+ expect(doc.at_css('img')['src']).to eq src
+ end
+
+ it 'skips single domain and ignores url in query string' do
+ src = "http://gitlab.com/test.png?url=http://example.com/test.png"
+ doc = filter(image(src), @context)
+
+ expect(doc.at_css('img')['src']).to eq src
+ end
+
+ it 'skips wildcarded domain' do
+ src = "http://images.mydomain.com/test.png"
+ doc = filter(image(src), @context)
+
+ expect(doc.at_css('img')['src']).to eq src
+ end
+ end
+end
diff --git a/spec/lib/banzai/filter/external_link_filter_spec.rb b/spec/lib/banzai/filter/external_link_filter_spec.rb
index 2acbe05f082..9cb5f835b7f 100644
--- a/spec/lib/banzai/filter/external_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/external_link_filter_spec.rb
@@ -154,6 +154,18 @@ describe Banzai::Filter::ExternalLinkFilter do
expect(doc_email.to_html).to include('http://xn--example-6p25f.com/</a>')
end
end
+
+ context 'autolinked image' do
+ let(:html) { %q(<a href="https://assets.example.com/6d8b/634c" data-canonical-src="http://exa%F0%9F%98%84mple.com/test.png"><img src="http://exa%F0%9F%98%84mple.com/test.png" data-canonical-src="http://exa%F0%9F%98%84mple.com/test.png"></a>) }
+ let(:doc) { filter(html) }
+
+ it_behaves_like 'an external link with rel attribute'
+
+ it 'adds a toolip with punycode' do
+ expect(doc.to_html).to include('class="has-tooltip"')
+ expect(doc.to_html).to include('title="http://xn--example-6p25f.com/test.png"')
+ end
+ end
end
context 'for links that look malicious' do
diff --git a/spec/lib/banzai/filter/image_link_filter_spec.rb b/spec/lib/banzai/filter/image_link_filter_spec.rb
index c84b98eb225..f9a830aeff9 100644
--- a/spec/lib/banzai/filter/image_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/image_link_filter_spec.rb
@@ -26,4 +26,11 @@ describe Banzai::Filter::ImageLinkFilter do
doc = filter(%Q(<p>test #{image('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg')} inline</p>))
expect(doc.to_html).to match %r{^<p>test <a[^>]*><img[^>]*></a> inline</p>$}
end
+
+ it 'keep the data-canonical-src' do
+ doc = filter(%q(<img src="http://assets.example.com/6cd/4d7" data-canonical-src="http://example.com/test.png" />))
+
+ expect(doc.at_css('img')['src']).to eq doc.at_css('a')['href']
+ expect(doc.at_css('img')['data-canonical-src']).to eq doc.at_css('a')['data-canonical-src']
+ end
end
diff --git a/spec/lib/banzai/filter/video_link_filter_spec.rb b/spec/lib/banzai/filter/video_link_filter_spec.rb
index 81dda0687f3..d63bf4d64c1 100644
--- a/spec/lib/banzai/filter/video_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/video_link_filter_spec.rb
@@ -47,4 +47,26 @@ describe Banzai::Filter::VideoLinkFilter do
expect(element['src']).to eq '/path/my_image.jpg'
end
end
+
+ context 'when asset proxy is enabled' do
+ it 'uses the correct src' do
+ stub_asset_proxy_setting(enabled: true)
+
+ proxy_src = 'https://assets.example.com/6d8b63'
+ canonical_src = 'http://example.com/test.mp4'
+ image = %(<img src="#{proxy_src}" data-canonical-src="#{canonical_src}" />)
+ container = filter(image, asset_proxy_enabled: true).children.first
+
+ expect(container['class']).to eq 'video-container'
+
+ video, paragraph = container.children
+
+ expect(video['src']).to eq proxy_src
+ expect(video['data-canonical-src']).to eq canonical_src
+
+ link = paragraph.children.first
+
+ expect(link['href']).to eq proxy_src
+ end
+ end
end
diff --git a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
index 469692f7b5a..02408622499 100644
--- a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
+++ b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
@@ -140,4 +140,48 @@ describe Banzai::Pipeline::GfmPipeline do
expect(output).to include(Gitlab::Routing.url_helpers.milestone_path(milestone))
end
end
+
+ describe 'asset proxy' do
+ let(:project) { create(:project, :public) }
+ let(:image) { '![proxy](http://example.com/test.png)' }
+ let(:proxy) { 'https://assets.example.com/08df250eeeef1a8cf2c761475ac74c5065105612/687474703a2f2f6578616d706c652e636f6d2f746573742e706e67' }
+ let(:version) { Gitlab::CurrentSettings.current_application_settings.local_markdown_version }
+
+ before do
+ stub_asset_proxy_setting(enabled: true)
+ stub_asset_proxy_setting(secret_key: 'shared-secret')
+ stub_asset_proxy_setting(url: 'https://assets.example.com')
+ stub_asset_proxy_setting(whitelist: %W(gitlab.com *.mydomain.com #{Gitlab.config.gitlab.host}))
+ stub_asset_proxy_setting(domain_regexp: Banzai::Filter::AssetProxyFilter.compile_whitelist(Gitlab.config.asset_proxy.whitelist))
+ end
+
+ it 'replaces a lazy loaded img src' do
+ output = described_class.to_html(image, project: project)
+ doc = Nokogiri::HTML.fragment(output)
+ result = doc.css('img').first
+
+ expect(result['data-src']).to eq(proxy)
+ end
+
+ it 'autolinks images to the proxy' do
+ output = described_class.to_html(image, project: project)
+ doc = Nokogiri::HTML.fragment(output)
+ result = doc.css('a').first
+
+ expect(result['href']).to eq(proxy)
+ expect(result['data-canonical-src']).to eq('http://example.com/test.png')
+ end
+
+ it 'properly adds tooltips to link for IDN images' do
+ image = '![proxy](http://exa😄mple.com/test.png)'
+ proxy = 'https://assets.example.com/6d8b634c412a23c6bfe1b2963f174febf5635ddd/687474703a2f2f6578612546302539462539382538346d706c652e636f6d2f746573742e706e67'
+ output = described_class.to_html(image, project: project)
+ doc = Nokogiri::HTML.fragment(output)
+ result = doc.css('a').first
+
+ expect(result['href']).to eq(proxy)
+ expect(result['data-canonical-src']).to eq('http://exa%F0%9F%98%84mple.com/test.png')
+ expect(result['title']).to eq 'http://xn--example-6p25f.com/test.png'
+ end
+ end
end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index ab6f6dfe720..904185f4140 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -344,6 +344,71 @@ describe ApplicationSetting do
end
end
end
+
+ context 'asset proxy settings' do
+ before do
+ subject.asset_proxy_enabled = true
+ end
+
+ describe '#asset_proxy_url' do
+ it { is_expected.not_to allow_value('').for(:asset_proxy_url) }
+ it { is_expected.to allow_value(http).for(:asset_proxy_url) }
+ it { is_expected.to allow_value(https).for(:asset_proxy_url) }
+ it { is_expected.not_to allow_value(ftp).for(:asset_proxy_url) }
+
+ it 'is not required when asset proxy is disabled' do
+ subject.asset_proxy_enabled = false
+ subject.asset_proxy_url = ''
+
+ expect(subject).to be_valid
+ end
+ end
+
+ describe '#asset_proxy_secret_key' do
+ it { is_expected.not_to allow_value('').for(:asset_proxy_secret_key) }
+ it { is_expected.to allow_value('anything').for(:asset_proxy_secret_key) }
+
+ it 'is not required when asset proxy is disabled' do
+ subject.asset_proxy_enabled = false
+ subject.asset_proxy_secret_key = ''
+
+ expect(subject).to be_valid
+ end
+
+ it 'is encrypted' do
+ subject.asset_proxy_secret_key = 'shared secret'
+
+ expect(subject.encrypted_asset_proxy_secret_key).to be_present
+ expect(subject.encrypted_asset_proxy_secret_key).not_to eq(subject.asset_proxy_secret_key)
+ end
+ end
+
+ describe '#asset_proxy_whitelist' do
+ context 'when given an Array' do
+ it 'sets the domains and adds current running host' do
+ setting.asset_proxy_whitelist = ['example.com', 'assets.example.com']
+ expect(setting.asset_proxy_whitelist).to eq(['example.com', 'assets.example.com', 'localhost'])
+ end
+ end
+
+ context 'when given a String' do
+ it 'sets multiple domains with spaces' do
+ setting.asset_proxy_whitelist = 'example.com *.example.com'
+ expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost'])
+ end
+
+ it 'sets multiple domains with newlines and a space' do
+ setting.asset_proxy_whitelist = "example.com\n *.example.com"
+ expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost'])
+ end
+
+ it 'sets multiple domains with commas' do
+ setting.asset_proxy_whitelist = "example.com, *.example.com"
+ expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost'])
+ end
+ end
+ end
+ end
end
context 'restrict creating duplicates' do
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index 8a60980fe80..f4697df99d0 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -158,5 +158,33 @@ describe API::Settings, 'Settings' do
expect(json_response['error']).to eq('plantuml_url is missing')
end
end
+
+ context 'asset_proxy settings' do
+ it 'updates application settings' do
+ put api('/application/settings', admin),
+ params: {
+ asset_proxy_enabled: true,
+ asset_proxy_url: 'http://assets.example.com',
+ asset_proxy_secret_key: 'shared secret',
+ asset_proxy_whitelist: ['example.com', '*.example.com']
+ }
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['asset_proxy_enabled']).to be(true)
+ expect(json_response['asset_proxy_url']).to eq('http://assets.example.com')
+ expect(json_response['asset_proxy_secret_key']).to be_nil
+ expect(json_response['asset_proxy_whitelist']).to eq(['example.com', '*.example.com', 'localhost'])
+ end
+
+ it 'allows a string for asset_proxy_whitelist' do
+ put api('/application/settings', admin),
+ params: {
+ asset_proxy_whitelist: 'example.com, *.example.com'
+ }
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['asset_proxy_whitelist']).to eq(['example.com', '*.example.com', 'localhost'])
+ end
+ end
end
end
diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb
index a641828faa5..7eb27016140 100644
--- a/spec/services/application_settings/update_service_spec.rb
+++ b/spec/services/application_settings/update_service_spec.rb
@@ -62,6 +62,39 @@ describe ApplicationSettings::UpdateService do
end
end
+ describe 'markdown cache invalidators' do
+ shared_examples 'invalidates markdown cache' do |attribute|
+ let(:params) { attribute }
+
+ it 'increments cache' do
+ expect { subject.execute }.to change(application_settings, :local_markdown_version).by(1)
+ end
+ end
+
+ it_behaves_like 'invalidates markdown cache', { asset_proxy_enabled: true }
+ it_behaves_like 'invalidates markdown cache', { asset_proxy_url: 'http://test.com' }
+ it_behaves_like 'invalidates markdown cache', { asset_proxy_secret_key: 'another secret' }
+ it_behaves_like 'invalidates markdown cache', { asset_proxy_whitelist: ['domain.com'] }
+
+ context 'when also setting the local_markdown_version' do
+ let(:params) { { asset_proxy_enabled: true, local_markdown_version: 12 } }
+
+ it 'does not increment' do
+ expect { subject.execute }.to change(application_settings, :local_markdown_version).to(12)
+ end
+ end
+
+ context 'do not invalidate if value does not change' do
+ let(:params) { { asset_proxy_enabled: true, asset_proxy_secret_key: 'secret', asset_proxy_url: 'http://test.com' } }
+
+ it 'does not increment' do
+ described_class.new(application_settings, admin, params).execute
+
+ expect { described_class.new(application_settings, admin, params).execute }.not_to change(application_settings, :local_markdown_version)
+ end
+ end
+ end
+
describe 'performance bar settings' do
using RSpec::Parameterized::TableSyntax
diff --git a/spec/support/helpers/stub_configuration.rb b/spec/support/helpers/stub_configuration.rb
index 049702be1f6..49b0619f05d 100644
--- a/spec/support/helpers/stub_configuration.rb
+++ b/spec/support/helpers/stub_configuration.rb
@@ -99,6 +99,10 @@ module StubConfiguration
allow(Gitlab.config.gitlab_shell).to receive_messages(to_settings(messages))
end
+ def stub_asset_proxy_setting(messages)
+ allow(Gitlab.config.asset_proxy).to receive_messages(to_settings(messages))
+ end
+
def stub_rack_attack_setting(messages)
allow(Gitlab.config.rack_attack).to receive(:git_basic_auth).and_return(messages)
allow(Gitlab.config.rack_attack.git_basic_auth).to receive_messages(to_settings(messages))