Disallow invalid or private URIs

Author: oneiros

fasp_base/app/models/fasp_base/registration.rb

@@ -15,9 +15,9 @@ class Registration
     validates :email,
       presence: true,
       format: { with: /@/ }
-    # TODO: Proper URL validation
     validates :base_url,
-      presence: true
+      presence: true,
+      "fasp_base/uri": { if: -> { Rails.env.production? } }
     validates :password,
       presence: true,
       confirmation: true,

fasp_base/app/validators/fasp_base/uri_validator.rb

@@ -0,0 +1,52 @@
+module FaspBase
+  class UriValidator < ActiveModel::EachValidator
+    def validate_each(record, attribute, value)
+      record.errors.add(attribute, :invalid) unless uri_valid?(value)
+    end
+
+    private
+
+    def uri_valid?(string)
+      uri = URI.parse(string)
+
+      return false unless scheme_valid?(uri)
+      return false unless host_valid?(uri.host)
+
+      true
+    rescue URI::Error
+      false
+    end
+
+    def scheme_valid?(uri)
+      return false unless uri.is_a?(URI::HTTP)
+      return false unless options[:allow_http] || uri.is_a?(URI::HTTPS)
+
+      true
+    end
+
+    def host_valid?(host)
+      return false unless host.present?
+
+      addresses = resolve_addresses(host)
+      return false if addresses.empty?
+
+      non_private?(addresses)
+    end
+
+    def resolve_addresses(host)
+      Socket.getaddrinfo(host, nil)
+        .map { |info| info[3] }
+        .uniq
+    rescue Socket::ResolutionError
+      []
+    end
+
+    def non_private?(addresses)
+      addresses.none? do |address|
+        ipaddr = IPAddr.new(address)
+
+        ipaddr.loopback? || ipaddr.link_local? || ipaddr.private?
+      end
+    end
+  end
+end

fasp_base/test/validators/fasp_base/uri_validator_test.rb

@@ -0,0 +1,54 @@
+require "test_helper"
+
+module FaspBase
+  class UriValidatorTest < ::ActiveSupport::TestCase
+    class Website
+      include ActiveModel::Model
+      include ActiveModel::Attributes
+
+      attribute :uri, :string
+
+      validates :uri, 'fasp_base/uri': true
+    end
+
+    setup do
+      @website = Website.new
+    end
+
+    test "non-https URIs are invalid" do
+      invalid_uris = %w[ ftp://example.com file:///root/ ]
+
+      invalid_uris.each do |uri|
+        @website.uri = uri
+
+        assert @website.invalid?
+      end
+    end
+
+    test "missing, incomplete or syntactically wrong URIs are invalid" do
+      invalid_uris = [ nil, "", "abc", "www.test.com", "https///test" ]
+
+      invalid_uris.each do |uri|
+        @website.uri = uri
+
+        assert @website.invalid?
+      end
+    end
+
+    test "URI with hostnames resolving to private IPs are invalid" do
+      local_uris = %w[ http://127.0.0.1/test https://127.0.0.123/other http://localhost:3000/base ]
+
+      local_uris.each do |uri|
+        @website.uri = uri
+
+        assert @website.invalid?
+      end
+    end
+
+    test "external, existing URI is valid" do
+      @website.uri = "https://github.com/mastodon"
+
+      assert @website.valid?
+    end
+  end
+end