Merge pull request #104 from gsamokovarov/whitelisted-ips

Always whitelist localhost and inform users why no console is displayed

lib/web_console.rb

@@ -1,6 +1,7 @@
 require 'binding_of_caller'
 
 require 'active_support/lazy_load_hooks'
+require 'active_support/logger'
 
 require 'web_console/core_ext/exception'
 require 'web_console/engine'
@@ -11,13 +12,13 @@
 require 'web_console/template'
 require 'web_console/unsupported_platforms'
 require 'web_console/middleware'
+require 'web_console/whitelist'
 require 'web_console/request'
+require 'web_console/whiny_request'
 
 module WebConsole
-  # Shortcut the +WebConsole::Engine.config.web_console+.
-  def self.config
-    Engine.config.web_console
-  end
+  mattr_accessor :logger
+  @@logger = ActiveSupport::Logger.new($stderr)
 
   ActiveSupport.run_load_hooks(:web_console, self)
 end

lib/web_console/engine.rb

@@ -38,31 +38,19 @@ def render_exception_with_web_console(env, exception)
 
     initializer 'web_console.templates_path' do
       if template_paths = config.web_console.template_paths
-        WebConsole::Template.template_paths.unshift(*Array(template_paths))
+        Template.template_paths.unshift(*Array(template_paths))
       end
     end
 
-    initializer 'web_console.process_whitelisted_ips' do
-      config.web_console.tap do |c|
-        # Ensure that it is an array of IPAddr instances and it is defaulted to
-        # 127.0.0.1 if not precent. Only unique entries are left in the end.
-        c.whitelisted_ips = Array(c.whitelisted_ips).map { |ip|
-          if ip.is_a?(IPAddr)
-            ip
-          else
-            IPAddr.new(ip.presence || '127.0.0.1')
-          end
-        }.uniq
+    initializer 'web_console.whitelisted_ips' do
+      if whitelisted_ips = config.web_console.whitelisted_ips
+        Request.whitelisted_ips = Whitelist.new(whitelisted_ips)
+      end
+    end
 
-        # IPAddr instances can cover whole networks, so simplify the #include?
-        # check for the most common case.
-        def (c.whitelisted_ips).include?(ip)
-          if ip.is_a?(IPAddr)
-            super
-          else
-            any? { |net| net.include?(ip.to_s) }
-          end
-        end
+    initializer 'web_console.whiny_requests' do
+      if config.web_console.key?(:whiny_requests)
+        Middleware.whiny_requests = config.web_console.whiny_requests
       end
     end
   end

lib/web_console/middleware.rb

@@ -7,13 +7,16 @@ class Middleware
       binding_change_re: %r{/repl_sessions/(?<id>.+?)/trace\z}
     }
 
+    cattr_accessor :whiny_requests
+    @@whiny_requests = true
+
     def initialize(app, options = {})
       @app     = app
       @options = DEFAULT_OPTIONS.merge(options)
     end
 
     def call(env)
-      request = Request.new(env)
+      request = create_regular_or_whiny_request(env)
       return @app.call(env) unless request.from_whitelited_ip?
 
       if id = id_for_repl_session_update(request)
@@ -43,6 +46,11 @@ def call(env)
 
     private
 
+      def create_regular_or_whiny_request(env)
+        request = Request.new(env)
+        whiny_requests ? WhinyRequest.new(request) : request
+      end
+
       def update_re
         @options[:update_re]
       end

lib/web_console/request.rb

@@ -3,22 +3,28 @@ module WebConsole
   class Request < ActionDispatch::Request
     # While most of the servers will return blank content type if none given,
     # Puma will return text/plain.
-    ACCEPTABLE_CONTENT_TYPE = [Mime::HTML, Mime::TEXT]
+    cattr_accessor :acceptable_content_types
+    @@acceptable_content_types = [Mime::HTML, Mime::TEXT]
+
+    # Configurable set of whitelisted networks.
+    cattr_accessor :whitelisted_ips
+    @@whitelisted_ips = Whitelist.new
 
     # Returns whether a request came from a whitelisted IP.
     #
     # For a request to hit Web Console features, it needs to come from a white
     # listed IP.
     def from_whitelited_ip?
-      WebConsole.config.whitelisted_ips.include?(remote_ip)
+      whitelisted_ips.include?(remote_ip)
     end
 
     # Returns whether the request is from an acceptable content type.
     #
-    # We can render a console for HTML and TEXT. If a client didn't
-    # specified any content type, we'll render it as well.
+    # We can render a console for HTML and TEXT by default. If a client didn't
+    # specified any content type and the server returned it as blank, we'll
+    # render it as well.
     def acceptable_content_type?
-      content_type.blank? || content_type.in?(ACCEPTABLE_CONTENT_TYPE)
+      content_type.blank? || content_type.in?(acceptable_content_types)
     end
   end
 end

lib/web_console/whiny_request.rb

@@ -0,0 +1,38 @@
+module WebConsole
+  # Noisy wrapper around +Request+.
+  #
+  # If any calls to +from_whitelisted_ip?+ and +acceptable_content_type?+
+  # return false, an info log message will be displayed in users' logs.
+  class WhinyRequest < SimpleDelegator
+    def from_whitelited_ip?
+      whine_unless request.from_whitelited_ip? do
+        "Cannot render console from #{request.remote_ip}! " \
+          "Allowed networks: #{request.whitelisted_ips}"
+      end
+    end
+
+    def acceptable_content_type?
+      whine_unless request.acceptable_content_type? do
+        "Cannot render console with content type #{request.content_type}" \
+          "Allowed content types: #{request.acceptable_content_types}"
+      end
+    end
+
+    private
+
+      def whine_unless(condition)
+        unless condition
+          logger.info { yield }
+        end
+        condition
+      end
+
+      def logger
+        env['action_dispatch.logger'] || WebConsole.logger
+      end
+
+      def request
+        __getobj__
+      end
+  end
+end

lib/web_console/whitelist.rb

@@ -0,0 +1,40 @@
+module WebConsole
+  # Whitelist of allowed networks that can access Web Console.
+  #
+  # Networks are represented by standard IPAddr and can be either IPv4 or IPv6
+  # networks.
+  class Whitelist
+    # IPv4 and IPv6 localhost should be always whitelisted.
+    ALWAYS_WHITELISTED_NETWORKS = %w( 127.0.0.0/8 ::1 )
+
+    def initialize(networks = nil)
+      @networks = normalize_networks(networks).map(&method(:coerce_network_to_ipaddr)).uniq
+    end
+
+    def include?(network)
+      @networks.any? { |whitelist| whitelist.include?(network.to_s) }
+    end
+
+    def to_s
+      @networks.map(&method(:human_readable_ipaddr)).join(', ')
+    end
+
+    private
+
+      def normalize_networks(networks)
+        Array(networks).concat(ALWAYS_WHITELISTED_NETWORKS)
+      end
+
+      def coerce_network_to_ipaddr(network)
+        if network.is_a?(IPAddr)
+          network
+        else
+          IPAddr.new(network)
+        end
+      end
+
+      def human_readable_ipaddr(ipaddr)
+        ipaddr.to_range.to_s.split('..').uniq.join('/')
+      end
+  end
+end

test/test_helper.rb

@@ -23,6 +23,28 @@ def assert_select(*)
   include SilenceRailsDomTesting
 end
 
+# A copy of Kernel#capture in active_support/core_ext/kernel/reporting.rb as
+# its getting deprecated past 4.2. Its not thread safe, but I don't need it to
+# be in the tests
+def capture(stream)
+  stream = stream.to_s
+  captured_stream = Tempfile.new(stream)
+  stream_io = eval("$#{stream}")
+  origin_stream = stream_io.dup
+  stream_io.reopen(captured_stream)
+
+  yield
+
+  stream_io.rewind
+  return captured_stream.read
+ensure
+  captured_stream.close
+  captured_stream.unlink
+  stream_io.reopen(origin_stream)
+end
+
+alias silence capture
+
 # Load fixtures from the engine
 if ActiveSupport::TestCase.method_defined?(:fixture_path=)
   ActiveSupport::TestCase.fixture_path = File.expand_path("../fixtures", __FILE__)

test/web_console/engine_test.rb

@@ -2,90 +2,52 @@
 
 module WebConsole
   class EngineTest < ActiveSupport::TestCase
-    test 'whitelisted ips are courced to IPAddr' do
+    test 'config.whitelisted_ips sets whitelisted networks' do
       new_uninitialized_app do |app|
-        app.config.web_console.whitelisted_ips = '127.0.0.1'
-        app.initialize!
-
-        assert_equal [ IPAddr.new('127.0.0.1') ], app.config.web_console.whitelisted_ips
-      end
-    end
-
-    test 'whitelisted ips with IPv6 format as default' do
-      new_uninitialized_app do |app|
-        app.config.web_console.whitelisted_ips = [ '127.0.0.1', '::1' ]
-        app.initialize!
-
-        assert_equal [ IPAddr.new('127.0.0.1'), IPAddr.new('::1') ], app.config.web_console.whitelisted_ips
-      end
-    end
-
-    test 'whitelisted ips are normalized and unique IPAddr' do
-      new_uninitialized_app do |app|
-        app.config.web_console.whitelisted_ips = [ '127.0.0.1', '127.0.0.1', nil, '', ' ' ]
-        app.initialize!
-
-        assert_equal [ IPAddr.new('127.0.0.1') ], app.config.web_console.whitelisted_ips
-      end
-    end
-
-    test 'whitelisted_ips.include? coerces to IPAddr' do
-      new_uninitialized_app do |app|
-        app.config.web_console.whitelisted_ips = '127.0.0.1'
+        app.config.web_console.whitelisted_ips = %w( 172.16.0.0/12 192.168.0.0/16 )
         app.initialize!
 
-        assert app.config.web_console.whitelisted_ips.include?('127.0.0.1')
+        1.upto(255).each do |n|
+          assert_includes Request.whitelisted_ips, "172.16.0.#{n}"
+          assert_includes Request.whitelisted_ips, "192.168.0.#{n}"
+        end
       end
     end
 
-    test 'whitelisted_ips.include? works with IPAddr' do
+    test 'config.whitelisted_ips always includes localhost' do
       new_uninitialized_app do |app|
-        app.config.web_console.whitelisted_ips = '127.0.0.1'
+        app.config.web_console.whitelisted_ips = '8.8.8.8'
         app.initialize!
 
-        assert app.config.web_console.whitelisted_ips.include?(IPAddr.new('127.0.0.1'))
+        assert_includes Request.whitelisted_ips, '127.0.0.1'
+        assert_includes Request.whitelisted_ips, '::1'
+        assert_includes Request.whitelisted_ips, '8.8.8.8'
       end
     end
 
-    test 'whitelist whole networks' do
+    test 'config.template_paths prepend paths if it exists' do
       new_uninitialized_app do |app|
-        app.config.web_console.whitelisted_ips = '172.16.0.0/12'
-        app.initialize!
-
-        1.upto(255).each do |n|
-          assert_includes app.config.web_console.whitelisted_ips, "172.16.0.#{n}"
-        end
-      end
-    end
+        dirname = File.expand_path('..', __FILE__)
 
-    test 'whitelist multiple networks' do
-      new_uninitialized_app do |app|
-        app.config.web_console.whitelisted_ips = %w( 172.16.0.0/12 192.168.0.0/16 )
+        app.config.web_console.template_paths = dirname
         app.initialize!
 
-        1.upto(255).each do |n|
-          assert_includes app.config.web_console.whitelisted_ips, "172.16.0.#{n}"
-          assert_includes app.config.web_console.whitelisted_ips, "192.168.0.#{n}"
-        end
+        assert_equal dirname, Template.template_paths.first
       end
     end
 
-    test 'config.template_paths prepend paths if it exists' do
+    test 'config.whiny_request removes extra logging' do
       new_uninitialized_app do |app|
-        dirname = File.expand_path('..', __FILE__)
-
-        app.config.web_console.template_paths = dirname
+        app.config.web_console.whiny_requests = false
         app.initialize!
 
-        assert_equal dirname, WebConsole::Template.template_paths.first
+        assert_not Middleware.whiny_requests
       end
     end
 
     private
 
       def new_uninitialized_app(root = File.expand_path('../../dummy', __FILE__))
-        skip if Rails::VERSION::MAJOR == 3
-
         old_app = Rails.application
 
         FileUtils.mkdir_p(root)

test/web_console/helper_test.rb

@@ -47,7 +47,7 @@ def call(env)
     end
 
     setup do
-      WebConsole.config.stubs(:whitelisted_ips).returns(IPAddr.new('0.0.0.0/0'))
+      Request.stubs(:whitelisted_ips).returns(IPAddr.new('0.0.0.0/0'))
 
       @app = Middleware.new(SingleConsoleApplication.new)
     end

test/web_console/middleware_test.rb

@@ -18,7 +18,7 @@ def call(env)
     end
 
     setup do
-      WebConsole.config.stubs(:whitelisted_ips).returns(IPAddr.new('0.0.0.0/0'))
+      Request.stubs(:whitelisted_ips).returns(IPAddr.new('0.0.0.0/0'))
 
       @app = Middleware.new(Application.new)
     end
@@ -56,9 +56,11 @@ def call(env)
     end
 
     test "doesn't render console from non whitelisted IP" do
-      WebConsole.config.stubs(:whitelisted_ips).returns(IPAddr.new('127.0.0.1'))
+      Request.stubs(:whitelisted_ips).returns(IPAddr.new('127.0.0.1'))
 
-      get '/', nil, 'CONTENT_TYPE' => 'text/html', 'REMOTE_ADDR' => '1.1.1.1', 'web-console.binding' => binding
+      silence(:stderr) do
+        get '/', nil, 'CONTENT_TYPE' => 'text/html', 'REMOTE_ADDR' => '1.1.1.1', 'web-console.binding' => binding
+      end
 
       assert_select '#console', 0
     end

test/web_console/request_test.rb

@@ -3,7 +3,7 @@
 module WebConsole
   class RequestTest < ActiveSupport::TestCase
     setup do
-      WebConsole.config.stubs(:whitelisted_ips).returns(IPAddr.new('127.0.0.1'))
+      Request.stubs(:whitelisted_ips).returns(IPAddr.new('127.0.0.1'))
     end
 
     test '#from_whitelited_ip? is falsy for blacklisted IPs' do