diff --git a/Gemfile b/Gemfile index 1063ce2678..d37a7fb69b 100644 --- a/Gemfile +++ b/Gemfile @@ -71,13 +71,7 @@ gem 'formtastic', '~> 5.0' gem 'htmlentities', '~>4.3', '>= 4.3.4' gem 'json', '~> 2.7', '>= 2.7.1' gem 'responders', '~> 3.2' # For respond_with support -# uri >= 1.0.0 switched the default parser from RFC2396 to RFC3986 (ruby/uri#107), -# removed URI::DEFAULT_PARSER, dropped URI.decode, and the RFC3986 parser has no -# `registry` component. We rely on all of these across models (Proxy, ProxyRule, -# WebHook, AuthenticationProvider) and controllers. Upgrading is worthwhile but -# requires careful inspection of every call-site and should be deployed/tested -# separately from the Faraday update. -gem 'uri', '< 1.0.0' +gem 'uri', '~> 1.0' gem 'mysql2', '~> 0.5.3' diff --git a/Gemfile.lock b/Gemfile.lock index 70dd69388a..cea067ffd5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -975,7 +975,7 @@ GEM rack unicorn uniform_notifier (1.18.0) - uri (0.13.3) + uri (1.1.1) useragent (0.16.11) version_gem (1.1.3) webmock (3.24.0) @@ -1170,7 +1170,7 @@ DEPENDENCIES uglifier unicorn unicorn-rails - uri (< 1.0.0) + uri (~> 1.0) webmock (~> 3.24.0) webrick (~> 1.8.2) will_paginate (~> 3.3) diff --git a/app/controllers/provider/admin/redhat/auth_controller.rb b/app/controllers/provider/admin/redhat/auth_controller.rb index 73b2fcde42..68f241410a 100644 --- a/app/controllers/provider/admin/redhat/auth_controller.rb +++ b/app/controllers/provider/admin/redhat/auth_controller.rb @@ -31,7 +31,7 @@ def authentication_provider def referrer_url url = params.permit(:referrer)[:referrer] if url - URI.decode(url) + CGI.unescapeURIComponent(url) else provider_admin_account_path end diff --git a/app/controllers/provider/admin/service_discovery/auth_controller.rb b/app/controllers/provider/admin/service_discovery/auth_controller.rb index a89bc49843..a4e79b3404 100644 --- a/app/controllers/provider/admin/service_discovery/auth_controller.rb +++ b/app/controllers/provider/admin/service_discovery/auth_controller.rb @@ -24,9 +24,9 @@ def show protected def referrer_url - url = params[:referrer] + url = params.permit(:referrer)[:referrer] if url - URI.decode(url) + CGI.unescapeURIComponent(url) else new_admin_service_path end diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb index 0c98dfd61c..b8b93885c7 100644 --- a/app/helpers/messages_helper.rb +++ b/app/helpers/messages_helper.rb @@ -24,9 +24,8 @@ def message_subject(message) def hyperlink_urls(text) text = h(text) - text.scan(URI.regexp(%w(http https))) do - #$& contains the whole match of the regural expression - url = $&.sub(/\.$/, '').sub(/\:$/,'') + text.scan(UriPatterns::HYPERLINK_SCANNER) do + url = Regexp.last_match(0).sub(/\.$/, '').sub(/:$/, '') text = text.sub(url, link_to(url, url)) end diff --git a/app/lib/three_scale/oauth2/service_discovery_client.rb b/app/lib/three_scale/oauth2/service_discovery_client.rb index b36871d4de..63c606afef 100644 --- a/app/lib/three_scale/oauth2/service_discovery_client.rb +++ b/app/lib/three_scale/oauth2/service_discovery_client.rb @@ -74,7 +74,7 @@ def query_options def call url = super # OpenShift builtin OAuth has a serious bug, it does not store correctly the redirect_uri - ServiceDiscovery::Config.rh_sso? ? url : URI.decode(url) + ServiceDiscovery::Config.rh_sso? ? url : CGI.unescapeURIComponent(url) rescue => e # Do better error management Rails.logger.debug("[Openshift OAuth] Error decoding callback URL for builtin <%s>. Error: %s\n%s" % [url.inspect, e.message, e.backtrace.join("\n")]) diff --git a/app/lib/uri_patterns.rb b/app/lib/uri_patterns.rb new file mode 100644 index 0000000000..e492ef09e6 --- /dev/null +++ b/app/lib/uri_patterns.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# The uri gem's RFC3986_PARSER does not provide composable string-pattern +# constants (.pattern only exists on RFC2396_PARSER). +# These RFC2396 patterns are used intentionally to preserve compatibility. +module UriPatterns + UNRESERVED = URI::RFC2396_REGEXP::PATTERN::UNRESERVED + ESCAPED = URI::RFC2396_REGEXP::PATTERN::ESCAPED + RESERVED = URI::RFC2396_REGEXP::PATTERN::RESERVED + + HOSTNAME = URI::RFC2396_PARSER.pattern[:HOSTNAME] + ABS_PATH = URI::RFC2396_PARSER.pattern[:ABS_PATH] + QUERY = URI::RFC2396_PARSER.pattern[:QUERY] + + # URL scanner for free-form text (e.g. message bodies). + HYPERLINK_SCANNER = %r{https?://(?:[^\s()\[\]>]|\([^\s()]*\)|\[[^\s\[\]]*\])+} +end diff --git a/app/models/authentication_provider.rb b/app/models/authentication_provider.rb index 23fab04503..a4ceff59b2 100644 --- a/app/models/authentication_provider.rb +++ b/app/models/authentication_provider.rb @@ -31,8 +31,9 @@ class AuthenticationProvider < ApplicationRecord validates :client_id, :client_secret, presence: true, if: :oauth_config_required? - with_options format: { with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), allow_blank: true, message: :invalid_url } do |ops| - ops.validates :site + with_options uri: { path: true, query: true, fragment: true }, allow_blank: true do |ops| + # Skip uri: for whitespace URLs — each subclass whitespace validator handles these + ops.validates :site, unless: -> { site.to_s.match?(/\s/) } ops.validates :token_url ops.validates :authorize_url ops.validates :user_info_url diff --git a/app/models/authentication_provider/custom.rb b/app/models/authentication_provider/custom.rb index 50b90e4a1f..58f5bb15d4 100644 --- a/app/models/authentication_provider/custom.rb +++ b/app/models/authentication_provider/custom.rb @@ -1,3 +1,5 @@ class AuthenticationProvider::Custom < AuthenticationProvider self.authorization_scope = :iam_tools + + validates :site, format: { without: /\s/, message: :contains_whitespace } end diff --git a/app/models/authentication_provider/github.rb b/app/models/authentication_provider/github.rb index 044db861d6..b300b431f9 100644 --- a/app/models/authentication_provider/github.rb +++ b/app/models/authentication_provider/github.rb @@ -1,6 +1,8 @@ class AuthenticationProvider::GitHub < AuthenticationProvider self.authorization_scope = :branding + validates :site, format: { without: /\s/, message: :contains_whitespace } + after_initialize :set_defaults, unless: :persisted? state_machine :branding_state, initial: :initial_state.to_proc do diff --git a/app/models/proxy.rb b/app/models/proxy.rb index 5253d5d0ae..02af5bb24e 100644 --- a/app/models/proxy.rb +++ b/app/models/proxy.rb @@ -29,12 +29,11 @@ class Proxy < ApplicationRecord # rubocop:disable Metrics/ClassLength validates :error_status_no_match, :error_status_auth_missing, :error_status_auth_failed, :error_status_limits_exceeded, presence: true - uri_pattern = URI::DEFAULT_PARSER.pattern + OPTIONAL_QUERY_FORMAT = "(?:\\?(#{UriPatterns::QUERY}))?" + URI_PATH_PART = Regexp.new('\A' + UriPatterns::ABS_PATH + OPTIONAL_QUERY_FORMAT + '\z') + HOST = Regexp.new('\A' + UriPatterns::HOSTNAME + '(:\d+)?' + '\z') URI_OR_LOCALHOST = /\A(https?:\/\/([a-zA-Z0-9._:\/?-])+|.*localhost.*)\Z/ - OPTIONAL_QUERY_FORMAT = "(?:\\?(#{uri_pattern.fetch(:QUERY)}))?" - URI_PATH_PART = Regexp.new('\A' + uri_pattern.fetch(:ABS_PATH) + OPTIONAL_QUERY_FORMAT + '\z') - HOST = Regexp.new('\A' + uri_pattern.fetch(:HOSTNAME) + '(:\d+)?' + '\z') OAUTH_PARAMS = /(\?|&)(scope=|state=|tok=)/ @@ -675,7 +674,7 @@ def call(attribute) begin uri = URI.parse(attribute_value) - value = URI::Generic.new(uri.scheme, uri.userinfo, uri.host, uri.port, uri.registry, uri.path, uri.opaque, uri.query, uri.fragment).to_s + value = URI::Generic.new(uri.scheme, uri.userinfo, uri.host, uri.port, nil, uri.path, uri.opaque, uri.query, uri.fragment).to_s @model[attribute] = value unless @model[attribute] == value rescue URI::InvalidURIError @model.errors.add(attribute, 'Invalid domain') diff --git a/app/models/proxy_rule.rb b/app/models/proxy_rule.rb index e51538127a..5d2609374e 100644 --- a/app/models/proxy_rule.rb +++ b/app/models/proxy_rule.rb @@ -30,32 +30,35 @@ class PatternParser REGEX_LITERAL = /[_\w]+/i REGEX_VARIABLE = /\{#{REGEX_LITERAL}\}/ - # pchar = unreserved | escaped | - # ":" | "@" | "&" | "=" | "+" | "$" | "," - param = / + UNRESERVED = UriPatterns::UNRESERVED + ESCAPED = UriPatterns::ESCAPED + RESERVED = UriPatterns::RESERVED + + # pchar = unreserved / pct-encoded / ":" / "@" / "&" / "=" / "+" / "," ($ excluded intentionally) + PARAM = / (?: - [#{URI::REGEXP::PATTERN::UNRESERVED}:@&=+,] # note that $ is in the RFC but is removed for our purpposes + [#{UNRESERVED}:@&=+,] # note that $ is in the RFC but is removed for our purpposes | - #{URI::REGEXP::PATTERN::ESCAPED} + #{ESCAPED} | #{REGEX_VARIABLE} )* /x - segment = / - #{param} - (?:;#{param})* + SEGMENT = / + #{PARAM} + (?:;#{PARAM})* /x REGEX_PATH = %r{ - /#{segment}(?:/#{segment})* # normal URI path segments like /foo/bar + /#{SEGMENT}(?:/#{SEGMENT})* # normal URI path segments like /foo/bar }x - query = / + QUERY = / (?: - [#{URI::REGEXP::PATTERN::UNRESERVED}#{URI::REGEXP::PATTERN::RESERVED}] + [#{UNRESERVED}#{RESERVED}] | - #{URI::REGEXP::PATTERN::ESCAPED} + #{ESCAPED} | #{REGEX_LITERAL}=#{REGEX_VARIABLE} )* @@ -64,7 +67,7 @@ class PatternParser ABSOLUTE_PATH = /\A #{REGEX_PATH} # absolute path [$]? # optionally forcing to match the end of path - (?:\?(?:#{query}))? # optionally followed by a query string + (?:\?(?:#{QUERY}))? # optionally followed by a query string \Z/x def initialize @@ -89,7 +92,7 @@ def call(_) validates :http_method, inclusion: { in: ALLOWED_HTTP_METHODS } validate :non_repeated_parameters validate :no_vars_in_keys - validates :redirect_url, format: URI::DEFAULT_PARSER.make_regexp(%w[http https]), allow_blank: true, length: { maximum: 10000 } + validates :redirect_url, uri: { path: true, query: true, fragment: true }, allow_blank: true, length: { maximum: 10000 } def parameters Addressable::Template.new(path_pattern).variables diff --git a/app/models/web_hook.rb b/app/models/web_hook.rb index 1a975939e4..aec6e10585 100644 --- a/app/models/web_hook.rb +++ b/app/models/web_hook.rb @@ -4,7 +4,8 @@ class WebHook < ApplicationRecord alias provider account validates :account_id, presence: true - validates :url, format: { :with => URI::DEFAULT_PARSER.make_regexp(%w[http https]), :if => :active }, length: { maximum: 255 } + validates :url, uri: { path: true, query: true, fragment: true }, if: :active + validates :url, length: { maximum: 255 } #TODO: limit association only to providers? #TODO validate url as url? diff --git a/app/validators/uri_validator.rb b/app/validators/uri_validator.rb index 97d9a19dd8..a5a2ee9659 100644 --- a/app/validators/uri_validator.rb +++ b/app/validators/uri_validator.rb @@ -5,9 +5,7 @@ class UriValidator < ActiveModel::EachValidator DEFAULT_PERMISSIONS_OF_PARTS = { port: true, userinfo: false, - registry: false, path: false, - opaque: false, query: false, fragment: false }.freeze @@ -50,11 +48,12 @@ def initialize(uri:, permissions_parts:, accepted_scheme:) @accepted_scheme = accepted_scheme end - attr_reader :uri, :permissions_parts, :accepted_scheme, :generic_error_message + attr_reader :uri, :permissions_parts, :accepted_scheme + delegate :host, :scheme, to: :uri def errors - return [generic_error_message] if uri.blank? + return [:invalid_url] if uri.blank? errors_scheme | errors_host | errors_forbidden_parts end @@ -69,11 +68,11 @@ def errors_scheme [*accepted_scheme].include?(scheme) end - valid ? [] : [:invalid] + valid ? [] : [:invalid_url] end def errors_host - return [:invalid] if host.blank? + return [:invalid_url] if host.blank? errors = [] errors << I18n.t('errors.messages.too_long', count: MAX_HOST_SIZE) unless valid_host_size? @@ -90,7 +89,7 @@ def valid_host_labels? end def errors_forbidden_parts - contains_forbidden_parts? ? [:invalid] : [] + contains_forbidden_parts? ? [:invalid_url] : [] end def contains_forbidden_parts? diff --git a/config/locales/en.yml b/config/locales/en.yml index 837c58b7b5..84a4c0e3e8 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -780,6 +780,10 @@ en: success: Referrer filter has been deleted service_discovery: + auth: + show: + success: Service discovery authentication was successful + error: Service discovery authentication failed services: create: error: Cannot create product @@ -1898,6 +1902,7 @@ en: errors: messages: + invalid_url: "Invalid URL format" host_label_too_long: is too long for one or more labels of the host (maximum is %{count} characters) duplicated_user_provider_side: "Duplicate user registration. Delete one of the duplicates in order to continue." duplicated_user_buyer_side: "For activating your account please contact support." @@ -2027,17 +2032,17 @@ en: base: cannot_be_destroyed_with_products: cannot be deleted because it is used by at least one Product private_endpoint: - invalid: "the accepted format is 'scheme://address(:port)(/path)'. Accepted schemes are http, https, ws and wss" + invalid_url: "the accepted format is 'scheme://address(:port)(/path)'. Accepted schemes are http, https, ws and wss" proxy: attributes: api_backend: - invalid: "the accepted format is 'scheme://address(:port)(/path)'. Accepted schemes are http, https, ws and wss" + invalid_url: "the accepted format is 'scheme://address(:port)(/path)'. Accepted schemes are http, https, ws and wss" api_test_path: invalid: "only URI characters allowed" endpoint: - invalid: "the accepted format is 'protocol://address(:port)'" + invalid_url: "the accepted format is 'protocol://address(:port)'" sandbox_endpoint: - invalid: "the accepted format is 'protocol://address(:port)'" + invalid_url: "the accepted format is 'protocol://address(:port)'" oauth_login_url: invalid: "invalid auth login url. (hint: make sure it uses https scheme)" proxy_config: @@ -2105,7 +2110,7 @@ en: web_hook: attributes: url: - invalid: Must be a valid URL such as http://example.com + invalid_url: Must be a valid URL such as http://example.com cms/partial: attributes: diff --git a/features/provider/admin/webhooks/edit.feature b/features/provider/admin/webhooks/edit.feature index 486fa9de7a..faafc59efa 100644 --- a/features/provider/admin/webhooks/edit.feature +++ b/features/provider/admin/webhooks/edit.feature @@ -28,10 +28,10 @@ Feature: Provider webhooks Then they should see a toast alert with text "http://3scale-test.org responded with 200" Scenario: Webhook endpoint is wrong - Given the provider has a webhook with endpoint "http:://banana" When they go to the edit webhooks page - And press "Ping!" - Then they should see a toast alert with text "Hostname not supplied: 'http:://banana'" + And the form is submitted with: + | URL | http:://banana | + Then they should see "Must be a valid URL such as http://example.com" Scenario: Webhooks switch is denied Given the provider has "web_hooks" switch denied diff --git a/test/integration/provider/admin/redhat/auth_controller_test.rb b/test/integration/provider/admin/redhat/auth_controller_test.rb index 00f42feba8..6f2980b1b5 100644 --- a/test/integration/provider/admin/redhat/auth_controller_test.rb +++ b/test/integration/provider/admin/redhat/auth_controller_test.rb @@ -38,6 +38,22 @@ def setup end end + test 'callback decodes referrer url' do + login_provider @provider + host! @provider.external_admin_domain + user_data = ThreeScale::OAuth2::UserData.new(username: 'redhat_user') + ThreeScale::OAuth2::KeycloakClient.any_instance.stubs(:authenticate!).returns(user_data) + + get @callback_url, params: { referrer: '/p/admin/dashboard%3Ffoo%3Dbar' } + assert_redirected_to '/p/admin/dashboard?foo=bar' + + get @callback_url, params: { referrer: '/p/admin/search?q=hello+world' } + assert_redirected_to '/p/admin/search?q=hello+world' + + get @callback_url, params: { referrer: '/p/admin/hello+world' } + assert_redirected_to '/p/admin/hello+world' + end + test 'Red Hat Customer Portal disabled' do ThreeScale.config.redhat_customer_portal.stubs(enabled: false) diff --git a/test/integration/provider/admin/service_discovery/auth_controller_test.rb b/test/integration/provider/admin/service_discovery/auth_controller_test.rb new file mode 100644 index 0000000000..f2462a94f9 --- /dev/null +++ b/test/integration/provider/admin/service_discovery/auth_controller_test.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'test_helper' + +class Provider::Admin::ServiceDiscovery::AuthControllerTest < ActionDispatch::IntegrationTest + setup do + @provider = FactoryBot.create(:provider_account) + @callback_url = "/p/admin/auth/#{ServiceDiscovery::AuthenticationProviderSupport::SERVICE_DISCOVERY_SYSTEM_NAME}/callback" + + ThreeScale.config.service_discovery.stubs(enabled: true, authentication_method: 'oauth') + Rails.application.reload_routes! + + login_provider @provider + host! @provider.external_admin_domain + end + + test 'callback decodes referrer url' do + user_data = ThreeScale::OAuth2::UserData.new(username: 'discovery_user') + ServiceDiscovery::OAuthConfiguration.instance.stubs( + token_endpoint: 'https://oauth.example.com/token', + authorization_endpoint: 'https://oauth.example.com/authorize', + userinfo_endpoint: 'https://oauth.example.com/userinfo' + ) + ThreeScale::OAuth2::ServiceDiscoveryClient.any_instance.stubs(:authenticate!).returns(user_data) + ThreeScale::OAuth2::ServiceDiscoveryClient.any_instance.stubs(:access_token).returns(stub(token: 'tok', expires_at: 1.hour.from_now.to_i)) + + get @callback_url, params: { referrer: '/p/admin/dashboard%3Ffoo%3Dbar' } + assert_redirected_to '/p/admin/dashboard?foo=bar' + + get @callback_url, params: { referrer: '/p/admin/search?q=hello+world' } + assert_redirected_to '/p/admin/search?q=hello+world' + + get @callback_url, params: { referrer: '/p/admin/hello+world' } + assert_redirected_to '/p/admin/hello+world' + end +end diff --git a/test/models/backend_api_test.rb b/test/models/backend_api_test.rb index 4f5b9439f9..c8a7b678f9 100644 --- a/test/models/backend_api_test.rb +++ b/test/models/backend_api_test.rb @@ -22,7 +22,7 @@ def test_default_api_backend @account.expects(:provider_can_use?).with(:proxy_private_base_path).at_least_once.returns(false) @backend_api.private_endpoint = 'https://example.org:3/path' @backend_api.valid? - assert_equal [@backend_api.errors.generate_message(:private_endpoint, :invalid)], @backend_api.errors.messages[:private_endpoint] + assert_equal [@backend_api.errors.generate_message(:private_endpoint, :invalid_url)], @backend_api.errors.messages[:private_endpoint] @account.expects(:provider_can_use?).with(:proxy_private_base_path).at_least_once.returns(true) @backend_api.private_endpoint = 'https://example.org:3/path' diff --git a/test/unit/proxy_test.rb b/test/unit/proxy_test.rb index 50cc6cbb21..18f14bc563 100644 --- a/test/unit/proxy_test.rb +++ b/test/unit/proxy_test.rb @@ -269,6 +269,9 @@ def test_deployable @proxy.hostname_rewrite = 'my-own.api.example.net::80' refute @proxy.valid? + + @proxy.hostname_rewrite = 'my_proxy.internal' + refute @proxy.valid? end test 'backend' do @@ -301,7 +304,7 @@ def test_deployable backend_api.stubs(account: @account) @proxy.api_backend = 'https://example.org:3/path' refute @proxy.valid? - assert_equal [@proxy.errors.generate_message(:api_backend, :invalid)], @proxy.errors.messages[:api_backend] + assert_equal [@proxy.errors.generate_message(:api_backend, :invalid_url)], @proxy.errors.messages[:api_backend] @account.expects(:provider_can_use?).with(:proxy_private_base_path).at_least_once.returns(true) @proxy.api_backend = 'https://example.org:3/path' @@ -342,7 +345,7 @@ def test_deployable end test 'api_test_path formats valid' do - [ '/', '/i/m/a/lumberjack/42', '/~stuff', '/!not_-here']. each do |path| + ['/', '/i/m/a/lumberjack/42', '/~stuff', '/!not_-here', '/path?key=value&other=123'].each do |path| @proxy.api_test_path = path @proxy.valid? assert_empty @proxy.errors[:api_test_path], "errors found on - #{path}" @@ -361,7 +364,7 @@ def test_deployable %w[example.org:9 fdsfas ssh://example.org:39 http://example.org:32/fdsa?a=1].each do |endpoint| @proxy.api_backend = endpoint refute @proxy.valid? - assert_equal [@proxy.errors.generate_message(:api_backend, :invalid)], @proxy.errors.messages[:api_backend] + assert_equal [@proxy.errors.generate_message(:api_backend, :invalid_url)], @proxy.errors.messages[:api_backend] end %w[http://localhost/ https://127.0.0.1 http://127.10.0.50].each do |endpoint| diff --git a/test/unit/validators/non_localhost_validator_test.rb b/test/unit/validators/non_localhost_validator_test.rb index 8f760ee020..0ccbe021f7 100644 --- a/test/unit/validators/non_localhost_validator_test.rb +++ b/test/unit/validators/non_localhost_validator_test.rb @@ -25,7 +25,7 @@ def test_validate_each error = validator.validate_each(record, :api_backend, ' http://34.210.51.155:8181') assert_equal ActiveModel::Error, error.class - assert_equal ["Invalid URL format"], record.errors.messages_for(:api_backend) + assert record.errors.added?(:api_backend, :invalid_url) error = validator.validate_each(record, :api_backend, 'hrdt://smth') assert_nil error @@ -40,7 +40,7 @@ def test_validate_each assert record.errors.present? error = validator.validate_each(record, :api_backend, 'https://') - assert_equal "Invalid URL format", error.message + assert_equal :invalid_url, error.type assert record.errors.present? end end diff --git a/test/unit/validators/uri_validator_test.rb b/test/unit/validators/uri_validator_test.rb index f81fb05360..02cbe0041f 100644 --- a/test/unit/validators/uri_validator_test.rb +++ b/test/unit/validators/uri_validator_test.rb @@ -40,7 +40,7 @@ def self.model_name record = ModelWithURIValidation.new record.uri = "http://domain.test/path" refute record.valid? - assert_equal 'is invalid', record.errors[:uri].to_sentence + assert_equal 'Invalid URL format', record.errors[:uri].to_sentence with_clean_validators ModelWithURIValidation do [true, false].each do |valid_path| @@ -81,6 +81,18 @@ def self.model_name end end + test 'sub-delimiter characters are valid' do + with_clean_validators ModelWithURIValidation do + klass = Class.new(ModelWithURIValidation) { validates :uri, uri: { path: true } } + record = klass.new + + %w[! ' ( ) *].each do |char| + record.uri = "http://domain.test/path#{char}" + assert record.valid?, "Expected URI with '#{char}' to be valid" + end + end + end + test 'hostname with label longer than 63 chars' do record = ModelWithURIValidation.new record.uri = "http://#{long_hostname_label}.#{short_hostname_label}.test" diff --git a/test/unit/web_hook_test.rb b/test/unit/web_hook_test.rb index 54c6b4589a..339a97e08c 100644 --- a/test/unit/web_hook_test.rb +++ b/test/unit/web_hook_test.rb @@ -71,6 +71,32 @@ class PushingBehaviourTest < ActiveSupport::TestCase end end + test 'validate url' do + hook = FactoryBot.build_stubbed(:web_hook, active: true) + + hook.url = 'http://example.com' + assert_valid hook + + hook.url = 'https://example.com/webhook?token=abc' + assert_valid hook + + hook.url = 'foo' + refute_valid hook + assert hook.errors[:url].present? + + hook.url = 'ftp://example.com' + refute_valid hook + assert hook.errors[:url].present? + + hook.url = '' + refute_valid hook + assert hook.errors[:url].present? + + hook.active = false + hook.url = 'not-a-url' + assert_valid hook + end + test '#ping' do hook = WebHook.new(url: "http://foo")