diff --git a/lib/web_console.rb b/lib/web_console.rb index 6157deda..ef50089e 100644 --- a/lib/web_console.rb +++ b/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 diff --git a/lib/web_console/engine.rb b/lib/web_console/engine.rb index 4c2edd5f..7d2c0189 100644 --- a/lib/web_console/engine.rb +++ b/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 diff --git a/lib/web_console/middleware.rb b/lib/web_console/middleware.rb index 9052542c..3fc539fd 100644 --- a/lib/web_console/middleware.rb +++ b/lib/web_console/middleware.rb @@ -7,13 +7,16 @@ class Middleware binding_change_re: %r{/repl_sessions/(?.+?)/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 diff --git a/lib/web_console/request.rb b/lib/web_console/request.rb index 656e5178..7917855a 100644 --- a/lib/web_console/request.rb +++ b/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 diff --git a/lib/web_console/whiny_request.rb b/lib/web_console/whiny_request.rb new file mode 100644 index 00000000..87a6b5ba --- /dev/null +++ b/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 diff --git a/lib/web_console/whitelist.rb b/lib/web_console/whitelist.rb new file mode 100644 index 00000000..346f5622 --- /dev/null +++ b/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 diff --git a/test/test_helper.rb b/test/test_helper.rb index 43c2793a..822f306c 100644 --- a/test/test_helper.rb +++ b/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__) diff --git a/test/web_console/engine_test.rb b/test/web_console/engine_test.rb index 66dfed54..7c721eeb 100644 --- a/test/web_console/engine_test.rb +++ b/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) diff --git a/test/web_console/helper_test.rb b/test/web_console/helper_test.rb index 7766977e..a65902d2 100644 --- a/test/web_console/helper_test.rb +++ b/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 diff --git a/test/web_console/middleware_test.rb b/test/web_console/middleware_test.rb index e6ac1473..422099b3 100644 --- a/test/web_console/middleware_test.rb +++ b/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 diff --git a/test/web_console/request_test.rb b/test/web_console/request_test.rb index db958955..adae6868 100644 --- a/test/web_console/request_test.rb +++ b/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 diff --git a/test/web_console/whiny_request_test.rb b/test/web_console/whiny_request_test.rb new file mode 100644 index 00000000..f2436ec3 --- /dev/null +++ b/test/web_console/whiny_request_test.rb @@ -0,0 +1,33 @@ +require 'test_helper' + +module WebConsole + class WhinyRequestTest < ActiveSupport::TestCase + test '#from_whitelited_ip? logs out to stderr' do + Request.stubs(:whitelisted_ips).returns(IPAddr.new('127.0.0.1')) + assert_output_to_stderr do + req = request('http://example.com', 'REMOTE_ADDR' => '0.0.0.0') + assert_not req.from_whitelited_ip? + end + end + + test '#acceptable_content_type? logs out to stderr' do + Request.stubs(:acceptable_content_types).returns([]) + assert_output_to_stderr do + req = request('http://example.com', 'CONTENT_TYPE' => 'application/json') + assert_not req.acceptable_content_type? + end + end + + private + + def assert_output_to_stderr + output = capture(:stderr) { yield } + assert_not output.blank? + end + + def request(*args) + request = Request.new(Rack::MockRequest.env_for(*args)) + WhinyRequest.new(request) + end + end +end diff --git a/test/web_console/whitelist_test.rb b/test/web_console/whitelist_test.rb new file mode 100644 index 00000000..8a84db5b --- /dev/null +++ b/test/web_console/whitelist_test.rb @@ -0,0 +1,43 @@ +module WebConsole + class WhitelistTest < ActiveSupport::TestCase + test 'localhost is always whitelisted' do + whitelisted_ips = whitelist('8.8.8.8') + + assert_includes whitelisted_ips, '127.0.0.1' + assert_includes whitelisted_ips, '::1' + end + + test 'can whitelist single IPs' do + whitelisted_ips = whitelist('8.8.8.8') + + assert_includes whitelisted_ips, '8.8.8.8' + end + + test 'can whitelist whole networks' do + whitelisted_ips = whitelist('172.16.0.0/12') + + 1.upto(255).each do |n| + assert_includes whitelisted_ips, "172.16.0.#{n}" + end + end + + test 'can whitelist multiple networks' do + whitelisted_ips = whitelist %w(172.16.0.0/12 192.168.0.0/16) + + 1.upto(255).each do |n| + assert_includes whitelisted_ips, "172.16.0.#{n}" + assert_includes whitelisted_ips, "192.168.0.#{n}" + end + end + + test 'can be represented in a human readable form' do + assert_includes whitelist.to_s, '127.0.0.0/127.255.255.255, ::1' + end + + private + + def whitelist(*args) + Whitelist.new(*args) + end + end +end