FEATURE: perform security checks earlier in the pipeline

This avoids needing to discard results and ensures no areas of mini profiler
can execute prior to a request being authorized

authorization token lasts for 30 minutes and is cycled after that

Author: SamSaffron

Gemfile

@@ -2,4 +2,4 @@ source 'http://rubygems.org'
 
 gemspec
 
-gem 'codecov', :require => false, :group => :test
+gem 'codecov', :require => false, :group => :test

Guardfile

@@ -0,0 +1,30 @@
+# A sample Guardfile
+# More info at https://github.com/guard/guard#readme
+
+directories %w(lib spec) \
+ .select{|d| Dir.exists?(d) ? d : UI.warning("Directory #{d} does not exist")}
+
+## Note: if you are using the `directories` clause above and you are not
+## watching the project directory ('.'), then you will want to move
+## the Guardfile to a watched dir and symlink it back, e.g.
+#
+#  $ mkdir config
+#  $ mv Guardfile config/
+#  $ ln -s config/Guardfile .
+#
+# and, you'll have to watch "config/Guardfile" instead of "Guardfile"
+
+# Note: The cmd option is now required due to the increasing number of ways
+#       rspec may be run, below are examples of the most common uses.
+#  * bundler: 'bundle exec rspec'
+#  * bundler binstubs: 'bin/rspec'
+#  * spring: 'bin/rsspec' (This will use spring if running and you have
+#                          installed the spring binstubs per the docs)
+#  * zeus: 'zeus rspec' (requires the server to be started separetly)
+#  * 'just' rspec: 'rspec'
+guard :rspec, cmd: 'bundle exec rspec', failed_mode: :focus do
+  watch(%r{^spec/.+_spec\.rb$})
+  watch(%r{^lib/mini_profiler/(.+)\.rb$})     { |m| "spec/lib/#{m[1]}_spec.rb" }
+  watch('spec/spec_helper.rb')  { "spec" }
+end
+

autotest/discover.rb

@@ -12,39 +12,90 @@ class ClientSettings
       attr_accessor :backtrace_level
 
 
-      def initialize(env)
+      def initialize(env, store, start)
         request = ::Rack::Request.new(env)
         @cookie = request.cookies[COOKIE_NAME]
+        @store = store
+        @start = start
+
+        @allowed_tokens, @orig_auth_tokens = nil
+
         if @cookie
           @cookie.split(",").map{|pair| pair.split("=")}.each do |k,v|
             @orig_disable_profiling = @disable_profiling = (v=='t') if k == "dp"
             @backtrace_level = v.to_i if k == "bt"
+            @orig_auth_tokens = v.to_s.split("|") if k == "a"
           end
         end
 
-        @backtrace_level = nil if !@backtrace_level.nil? && (@backtrace_level == 0 || @backtrace_level > BACKTRACE_NONE)
+        if !@backtrace_level.nil? && (@backtrace_level == 0 || @backtrace_level > BACKTRACE_NONE)
+          @backtrace_level = nil
+        end
+
         @orig_backtrace_level = @backtrace_level
 
       end
 
+      def handle_cookie(result)
+        status,headers,_body = result
+
+        if (MiniProfiler.config.authorization_mode == :whitelist && !MiniProfiler.request_authorized?)
+          # this is non-obvious, don't kill the profiling cookie on errors or short requests
+          # this ensures that stuff that never reaches the rails stack does not kill profiling
+          if status.to_i >= 200 && status.to_i < 300 && ((Time.now - @start) > 0.1)
+            discard_cookie!(headers)
+          end
+        else
+          write!(headers)
+        end
+
+        result
+      end
+
       def write!(headers)
-        if @orig_disable_profiling != @disable_profiling || @orig_backtrace_level != @backtrace_level || @cookie.nil?
+
+        tokens_changed = false
+
+        if MiniProfiler.request_authorized? && MiniProfiler.config.authorization_mode == :whitelist
+          @allowed_tokens ||= @store.allowed_tokens
+          tokens_changed = !@orig_auth_tokens || ((@allowed_tokens - @orig_auth_tokens).length > 0)
+        end
+
+        if  @orig_disable_profiling != @disable_profiling ||
+            @orig_backtrace_level != @backtrace_level ||
+            @cookie.nil? ||
+            tokens_changed
+
           settings = {"p" =>  "t" }
-          settings["dp"] = "t"              if @disable_profiling
-          settings["bt"] = @backtrace_level if @backtrace_level
+          settings["dp"] = "t"                  if @disable_profiling
+          settings["bt"] = @backtrace_level     if @backtrace_level
+          settings["a"] = @allowed_tokens.join("|") if @allowed_tokens && MiniProfiler.request_authorized?
+
           settings_string = settings.map{|k,v| "#{k}=#{v}"}.join(",")
           Rack::Utils.set_cookie_header!(headers, COOKIE_NAME, :value => settings_string, :path => '/')
         end
       end
 
       def discard_cookie!(headers)
-        Rack::Utils.delete_cookie_header!(headers, COOKIE_NAME, :path => '/')
+        if @cookie
+          Rack::Utils.delete_cookie_header!(headers, COOKIE_NAME, :path => '/')
+        end
       end
 
-      def has_cookie?
-        !@cookie.nil?
+      def has_valid_cookie?
+        valid_cookie = !@cookie.nil?
+
+        if (MiniProfiler.config.authorization_mode == :whitelist)
+          @allowed_tokens ||= @store.allowed_tokens
+
+          valid_cookie = (Array === @orig_auth_tokens) &&
+            ((@allowed_tokens & @orig_auth_tokens).length > 0)
+        end
+
+        valid_cookie
       end
 
+
       def disable_profiling?
         @disable_profiling
       end

lib/mini_profiler/client_settings.rb

@@ -1,5 +1,6 @@
 class Rack::MiniProfiler::Context
-  attr_accessor :inject_js,:current_timer,:page_struct,:skip_backtrace,:full_backtrace,:discard, :mpt_init, :measure
+  attr_accessor :inject_js,:current_timer,:page_struct,:skip_backtrace,
+                :full_backtrace,:discard, :mpt_init, :measure
 
   def initialize(opts = {})
     opts["measure"] = true unless opts.key? "measure"

lib/mini_profiler/context.rb

@@ -149,7 +149,9 @@ def config
 
     def call(env)
 
-      client_settings = ClientSettings.new(env)
+      start = Time.now
+      client_settings = ClientSettings.new(env, @storage, start)
+      MiniProfiler.deauthorize_request if @config.authorization_mode == :whitelist
 
       status = headers = body = nil
       query_string = env['QUERY_STRING']
@@ -162,43 +164,39 @@ def call(env)
                 (@config.skip_paths && @config.skip_paths.any?{ |p| path.start_with?(p) }) ||
                 query_string =~ /pp=skip/
 
-      has_profiling_cookie = client_settings.has_cookie?
-
-      if skip_it || (@config.authorization_mode == :whitelist && !has_profiling_cookie)
-        status,headers,body = @app.call(env)
-        if !skip_it && @config.authorization_mode == :whitelist && !has_profiling_cookie && MiniProfiler.request_authorized?
-          client_settings.write!(headers)
-        end
-        return [status,headers,body]
+      if skip_it || (
+        @config.authorization_mode == :whitelist &&
+        !client_settings.has_valid_cookie?
+      )
+        return client_settings.handle_cookie(@app.call(env))
       end
 
       # handle all /mini-profiler requests here
-      return serve_html(env) if path.start_with? @config.base_url_path
+      return client_settings.handle_cookie(serve_html(env)) if path.start_with? @config.base_url_path
 
       has_disable_cookie = client_settings.disable_profiling?
       # manual session disable / enable
       if query_string =~ /pp=disable/ || has_disable_cookie
         skip_it = true
       end
 
-      if query_string =~ /pp=enable/ && (@config.authorization_mode != :whitelist || MiniProfiler.request_authorized?)
+      if query_string =~ /pp=enable/
         skip_it = false
         config.enabled = true
       end
 
       if skip_it || !config.enabled
         status,headers,body = @app.call(env)
         client_settings.disable_profiling = true
-        client_settings.write!(headers)
-        return [status,headers,body]
+        return client_settings.handle_cookie([status,headers,body])
       else
         client_settings.disable_profiling = false
       end
 
       # profile gc
       if query_string =~ /pp=profile-gc/
         current.measure = false if current
-        return Rack::MiniProfiler::GCProfiler.new.profile_gc(@app, env)
+        return client_settings.handle_cookie(Rack::MiniProfiler::GCProfiler.new.profile_gc(@app, env))
       end
 
       # profile memory
@@ -215,11 +213,10 @@ def call(env)
           body.close if body.respond_to? :close
         end
         report.pretty_print(result)
-        return text_result(result.string)
+        return client_settings.handle_cookie(text_result(result.string))
       end
 
       MiniProfiler.create_current(env, @config)
-      MiniProfiler.deauthorize_request if @config.authorization_mode == :whitelist
 
       if query_string =~ /pp=normal-backtrace/
         client_settings.backtrace_level = ClientSettings::BACKTRACE_DEFAULT
@@ -238,7 +235,6 @@ def call(env)
       trace_exceptions = query_string =~ /pp=trace-exceptions/ && defined? TracePoint
       status, headers, body, exceptions,trace = nil
 
-      start = Time.now
 
       if trace_exceptions
         exceptions = []
@@ -281,43 +277,37 @@ def call(env)
         else
           status,headers,body = @app.call(env)
         end
-        client_settings.write!(headers)
       ensure
         trace.disable if trace
       end
 
       skip_it = current.discard
 
       if (config.authorization_mode == :whitelist && !MiniProfiler.request_authorized?)
-        # this is non-obvious, don't kill the profiling cookie on errors or short requests
-        # this ensures that stuff that never reaches the rails stack does not kill profiling
-        if status.to_i >= 200 && status.to_i < 300 && ((Time.now - start) > 0.1)
-          client_settings.discard_cookie!(headers)
-        end
         skip_it = true
       end
 
-      return [status,headers,body] if skip_it
+      return client_settings.handle_cookie([status,headers,body]) if skip_it
 
       # we must do this here, otherwise current[:discard] is not being properly treated
       if trace_exceptions
         body.close if body.respond_to? :close
-        return dump_exceptions exceptions
+        return client_settings.handle_cookie(dump_exceptions exceptions)
       end
 
       if query_string =~ /pp=env/ && !config.disable_env_dump
         body.close if body.respond_to? :close
-        return dump_env env
+        return client_settings.handle_cookie(dump_env env)
       end
 
       if query_string =~ /pp=analyze-memory/
         body.close if body.respond_to? :close
-        return analyze_memory
+        return client_settings.handle_cookie(analyze_memory)
       end
 
       if query_string =~ /pp=help/
         body.close if body.respond_to? :close
-        return help(client_settings, env)
+        return client_settings.handle_cookie(help(client_settings, env))
       end
 
       page_struct = current.page_struct
@@ -326,7 +316,7 @@ def call(env)
 
       if flamegraph
         body.close if body.respond_to? :close
-        return self.flamegraph(flamegraph)
+        return client_settings.handle_cookie(self.flamegraph(flamegraph))
       end
 
 
@@ -337,18 +327,16 @@ def call(env)
 
         # inject headers, script
         if status >= 200 && status < 300
-          client_settings.write!(headers)
           result = inject_profiler(env,status,headers,body)
-          return result if result
+          return client_settings.handle_cookie(result) if result
         end
       rescue Exception => e
         if @config.storage_failure != nil
           @config.storage_failure.call(e)
         end
       end
 
-      client_settings.write!(headers)
-      [status, headers, body]
+      client_settings.handle_cookie([status, headers, body])
 
     ensure
       # Make sure this always happens
@@ -543,7 +531,6 @@ def help(client_settings, env)
 </html>
 "
 
-      client_settings.write!(headers)
       [200, headers, [body]]
     end
 

lib/mini_profiler/profiler.rb

@@ -2,6 +2,9 @@ module Rack
   class MiniProfiler
     class AbstractStore
 
+      # maximum age of allowed tokens before cycling in seconds
+      MAX_TOKEN_AGE = 1800
+
       def save(page_struct)
         raise NotImplementedError.new("save is not implemented")
       end
@@ -31,6 +34,11 @@ def diagnostics(user)
         ""
       end
 
+      # a list of tokens that are permitted to access profiler in whitelist mode
+      def allowed_tokens
+        raise NotImplementedError.new("allowed_tokens is not implemented")
+      end
+
     end
   end
 end

lib/mini_profiler/storage/abstract_store.rb

@@ -51,6 +51,9 @@ def initialize(args = nil)
         @user_view_cache    = FileCache.new(@path, "mp_views")
         @user_view_lock     = Mutex.new
 
+        @auth_token_cache    = FileCache.new(@path, "tokens")
+        @auth_token_lock     = Mutex.new
+
         me = self
         t = CacheCleanupThread.new do
           interval = 10
@@ -126,6 +129,28 @@ def get_unviewed_ids(user)
         }
       end
 
+      def flush_tokens
+        @auth_token_lock.synchronize {
+          @auth_token_cache[""] = nil
+        }
+      end
+
+      def allowed_tokens
+        @auth_token_lock.synchronize {
+          token1, token2, cycle_at = @auth_token_cache[""]
+
+          unless cycle_at && (Time === cycle_at) && (cycle_at > Time.now)
+            token2 = token1
+            token1 = SecureRandom.hex
+            cycle_at = Time.now + Rack::MiniProfiler::AbstractStore::MAX_TOKEN_AGE
+          end
+
+          @auth_token_cache[""] = [token1, token2, cycle_at]
+
+          [token1, token2].compact
+        }
+      end
+
       def cleanup_cache
         files = Dir.entries(@path)
         @timer_struct_lock.synchronize {