From 568dad796dc21bbb4826a58eab258bcf3e1fc26d Mon Sep 17 00:00:00 2001
From: Prem Sichanugrist <s@sikachu.com>
Date: Sat, 1 Jan 2011 23:51:05 +0700
Subject: [PATCH] Filter sensitive `QUERY_STRING` parameters in the log

This provides more safety to applications that appends secret information, such as API key or SSO token, as a `QUERY_STRING` in the request.

[#6244 state:committed]
---
 actionpack/CHANGELOG                               |    2 +
 .../lib/action_dispatch/http/filter_parameters.rb  |   19 +++++++++++---
 actionpack/test/dispatch/request_test.rb           |   27 ++++++++++++++++++++
 railties/lib/rails/rack/logger.rb                  |    2 +-
 4 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index d9fe45d..68ef4f3 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *Rails 3.1.0 (unreleased)*
 
+* Sensitive query string parameters (specified in config.filter_parameters) will now be filtered out from the request paths in the log file. [Prem Sichanugrist]
+
 * URL parameters which return false for to_param now appear in the query string (previously they were removed) [Andrew White]
 
 * URL parameters which return nil for to_param are now removed from the query string [Andrew White]
diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb
index 1ab48ae..1b095c2 100644
--- a/actionpack/lib/action_dispatch/http/filter_parameters.rb
+++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb
@@ -5,10 +5,10 @@ require 'active_support/core_ext/object/duplicable'
 module ActionDispatch
   module Http
     # Allows you to specify sensitive parameters which will be replaced from
-    # the request log by looking in all subhashes of the param hash for keys
-    # to filter. If a block is given, each key and value of the parameter
-    # hash and all subhashes is passed to it, the value or key can be replaced
-    # using String#replace or similar method.
+    # the request log by looking in the query string of the request and all
+    # subhashes of the param hash to filter. If a block is given, each key and
+    # value of the parameter hash and all subhashes is passed to it, the value
+    # or key can be replaced using String#replace or similar method.
     #
     # Examples:
     #
@@ -38,6 +38,11 @@ module ActionDispatch
         @filtered_env ||= env_filter.filter(@env)
       end
 
+      # Reconstructed a path with all sensitive GET parameters replaced.
+      def filtered_path
+        @filtered_path ||= query_string.empty? ? path : "#{path}?#{filtered_query_string}"
+      end
+
     protected
 
       def parameter_filter
@@ -52,6 +57,12 @@ module ActionDispatch
         @@parameter_filter_for[filters] ||= ParameterFilter.new(filters)
       end
 
+      def filtered_query_string
+        query_string.gsub(/([^&;]+)/) do |param|
+          parameter_filter.filter(Hash[[param.split('=', 2)]]).first.join("=")
+        end
+      end
+
     end
   end
 end
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index dd5bf5e..c271027 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -518,6 +518,33 @@ class RequestTest < ActiveSupport::TestCase
     assert_equal "1", request.params["step"]
   end
 
+  test "filtered_path returns path with filtered query string" do
+    request = stub_request('QUERY_STRING' => "username=sikachu&secret=bd4f21f&api_key=b1bc3b3cd352f68d79d7",
+      'PATH_INFO' => '/authenticate',
+      'action_dispatch.parameter_filter' => [:secret, :api_key])
+
+    path = request.filtered_path
+    assert_equal "/authenticate?username=sikachu&secret=[FILTERED]&api_key=[FILTERED]", path
+  end
+
+  test "filtered_path should not filter genuine string '[FILTERED]'" do
+    request = stub_request('QUERY_STRING' => "secret=bd4f21f&genuine=%5BFILTERED%5D",
+      'PATH_INFO' => '/authenticate',
+      'action_dispatch.parameter_filter' => [:secret])
+
+    path = request.filtered_path
+    assert_equal "/authenticate?secret=[FILTERED]&genuine=%5BFILTERED%5D", path
+  end
+
+  test "filtered_path should preserve duplication of keys in query string" do
+    request = stub_request('QUERY_STRING' => "username=sikachu&secret=bd4f21f&username=fxn",
+      'PATH_INFO' => '/authenticate',
+      'action_dispatch.parameter_filter' => [:secret])
+
+    path = request.filtered_path
+    assert_equal "/authenticate?username=sikachu&secret=[FILTERED]&username=fxn", path
+  end
+
 protected
 
   def stub_request(env = {})
diff --git a/railties/lib/rails/rack/logger.rb b/railties/lib/rails/rack/logger.rb
index 32acc66..3be262d 100644
--- a/railties/lib/rails/rack/logger.rb
+++ b/railties/lib/rails/rack/logger.rb
@@ -19,7 +19,7 @@ module Rails
 
       def before_dispatch(env)
         request = ActionDispatch::Request.new(env)
-        path = request.fullpath
+        path = request.filtered_path
 
         info "\n\nStarted #{request.request_method} \"#{path}\" " \
              "for #{request.ip} at #{Time.now.to_default_s}"
-- 
1.7.4.1