Tidy up new filter_parameters implementation.

This commit is contained in:
José Valim 2010-01-21 11:39:57 +01:00
parent b1bc3b3cd3
commit 31fddf2ace
13 changed files with 150 additions and 125 deletions

@ -16,7 +16,6 @@ module ActionController
autoload :ConditionalGet
autoload :Configuration
autoload :Cookies
autoload :FilterParameterLogging
autoload :Flash
autoload :Head
autoload :Helpers

@ -27,7 +27,6 @@ class Base < Metal
include ActionController::Compatibility
include ActionController::Cookies
include ActionController::FilterParameterLogging
include ActionController::Flash
include ActionController::Verification
include ActionController::RequestForgeryProtection
@ -74,6 +73,11 @@ def self.subclasses
@subclasses ||= []
end
# This method has been moved to ActionDispatch::Request.filter_parameters
def self.filter_parameter_logging(*)
ActiveSupport::Deprecation.warn("Setting filter_parameter_logging in ActionController is deprecated and has no longer effect, please set 'config.filter_parameters' in config/application.rb instead", caller)
end
def _normalize_options(action=nil, options={}, &blk)
case action
when NilClass

@ -1,13 +0,0 @@
module ActionController
module FilterParameterLogging
extend ActiveSupport::Concern
module ClassMethods
# This method has been moved to ActionDispatch::Http::ParametersFilter.filter_parameters
def filter_parameter_logging(*filter_words, &block)
ActiveSupport::Deprecation.warn("Setting filter_parameter_logging in ActionController is deprecated, please set 'config.filter_parameters' in application.rb or environments/[environment_name].rb instead.", caller)
ActionDispatch::Http::ParametersFilter.filter_parameters(*filter_words, &block)
end
end
end
end

@ -10,7 +10,6 @@ module Instrumentation
extend ActiveSupport::Concern
include AbstractController::Logger
include ActionController::FilterParameterLogging
attr_internal :view_runtime

@ -63,7 +63,7 @@ module Http
autoload :Headers
autoload :MimeNegotiation
autoload :Parameters
autoload :ParametersFilter
autoload :FilterParameters
autoload :Upload
autoload :UploadedFile, 'action_dispatch/http/upload'
autoload :URL

@ -0,0 +1,94 @@
require 'active_support/core_ext/hash/keys'
module ActionDispatch
module Http
module FilterParameters
extend ActiveSupport::Concern
INTERNAL_PARAMS = %w(controller action format _method only_path)
module ClassMethods
# Specify sensitive parameters which will be replaced from the request log.
# Filters parameters that have any of the arguments as a substring.
# Looks 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.
#
# Examples:
#
# ActionDispatch::Request.filter_parameters :password
# => replaces the value to all keys matching /password/i with "[FILTERED]"
#
# ActionDispatch::Request.filter_parameters :foo, "bar"
# => replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
#
# ActionDispatch::Request.filter_parameters do |k,v|
# v.reverse! if k =~ /secret/i
# end
# => reverses the value to all keys matching /secret/i
#
# ActionDispatch::Request.filter_parameters(:foo, "bar") do |k,v|
# v.reverse! if k =~ /secret/i
# end
# => reverses the value to all keys matching /secret/i, and
# replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
def filter_parameters(*filter_words, &block)
raise "You must filter at least one word" if filter_words.empty?
parameter_filter = Regexp.new(filter_words.join('|'), true)
define_method(:process_parameter_filter) do |original_params|
filtered_params = {}
original_params.each do |key, value|
if key =~ parameter_filter
value = '[FILTERED]'
elsif value.is_a?(Hash)
value = process_parameter_filter(value)
elsif value.is_a?(Array)
value = value.map { |i| process_parameter_filter(i) }
elsif block_given?
key = key.dup
value = value.dup if value.duplicable?
yield key, value
end
filtered_params[key] = value
end
filtered_params.except!(*INTERNAL_PARAMS)
end
protected :process_parameter_filter
end
end
# Return a hash of parameters with all sensitive data replaced.
def filtered_parameters
@filtered_parameters ||= process_parameter_filter(parameters)
end
alias :fitered_params :filtered_parameters
# Return a hash of request.env with all sensitive data replaced.
# TODO Josh should white list env to remove stuff like rack.input and rack.errors
def filtered_env
filtered_env = @env.dup
filtered_env.each do |key, value|
if (key =~ /RAW_POST_DATA/i)
filtered_env[key] = '[FILTERED]'
elsif value.is_a?(Hash)
filtered_env[key] = process_parameter_filter(value)
end
end
filtered_env
end
protected
def process_parameter_filter(original_parameters)
original_parameters.except(*INTERNAL_PARAMS)
end
end
end
end

@ -1,88 +0,0 @@
require 'active_support/core_ext/hash/keys'
module ActionDispatch
module Http
module ParametersFilter
INTERNAL_PARAMS = %w(controller action format _method only_path)
@@filter_parameters = nil
@@filter_parameters_block = nil
# Specify sensitive parameters which will be replaced from the request log.
# Filters parameters that have any of the arguments as a substring.
# Looks 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.
#
# Examples:
#
# ActionDispatch::Http::ParametersFilter.filter_parameters :password
# => replaces the value to all keys matching /password/i with "[FILTERED]"
#
# ActionDispatch::Http::ParametersFilter.filter_parameters :foo, "bar"
# => replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
#
# ActionDispatch::Http::ParametersFilter.filter_parameters do |k,v|
# v.reverse! if k =~ /secret/i
# end
# => reverses the value to all keys matching /secret/i
#
# ActionDispatch::Http::ParametersFilter.filter_parameters(:foo, "bar") do |k,v|
# v.reverse! if k =~ /secret/i
# end
# => reverses the value to all keys matching /secret/i, and
# replaces the value to all keys matching /foo|bar/i with "[FILTERED]"
def self.filter_parameters(*filter_words, &block)
raise "You must filter at least one word" if filter_words.empty? and !block_given?
@@filter_parameters = filter_words.empty? ? nil : Regexp.new(filter_words.join('|'), true)
@@filter_parameters_block = block
end
# Return a hash of parameters with all sensitive data replaced.
def filtered_parameters
@filtered_parameters ||= process_parameter_filter(parameters)
end
alias_method :fitered_params, :filtered_parameters
# Return a hash of request.env with all sensitive data replaced.
def filtered_env
@env.merge(@env) do |key, value|
if (key =~ /RAW_POST_DATA/i)
'[FILTERED]'
else
process_parameter_filter({key => value}, false).values[0]
end
end
end
protected
def process_parameter_filter(original_parameters, validate_block = true)
if @@filter_parameters or @@filter_parameters_block
filtered_params = {}
original_parameters.each do |key, value|
if key =~ @@filter_parameters
value = '[FILTERED]'
elsif value.is_a?(Hash)
value = process_parameter_filter(value)
elsif value.is_a?(Array)
value = value.map { |item| process_parameter_filter({key => item}, validate_block).values[0] }
elsif validate_block and @@filter_parameters_block
key = key.dup
value = value.dup if value.duplicable?
value = @@filter_parameters_block.call(key, value) || value
end
filtered_params[key] = value
end
filtered_params.except!(*INTERNAL_PARAMS)
else
original_parameters.except(*INTERNAL_PARAMS)
end
end
end
end
end

@ -11,7 +11,7 @@ class Request < Rack::Request
include ActionDispatch::Http::Cache::Request
include ActionDispatch::Http::MimeNegotiation
include ActionDispatch::Http::Parameters
include ActionDispatch::Http::ParametersFilter
include ActionDispatch::Http::FilterParameters
include ActionDispatch::Http::Upload
include ActionDispatch::Http::URL

@ -99,7 +99,8 @@ def test_process_action_with_view_runtime
end
def test_process_action_with_filter_parameters
Another::SubscribersController.filter_parameter_logging(:lifo, :amount)
ActionDispatch::Request.class_eval "alias :safe_process_parameter_filter :process_parameter_filter"
ActionDispatch::Request.filter_parameters(:lifo, :amount)
get :show, :lifo => 'Pratik', :amount => '420', :step => '1'
wait
@ -108,6 +109,8 @@ def test_process_action_with_filter_parameters
assert_match /"amount"=>"\[FILTERED\]"/, params
assert_match /"lifo"=>"\[FILTERED\]"/, params
assert_match /"step"=>"1"/, params
ensure
ActionDispatch::Request.class_eval "alias :process_parameter_filter :safe_process_parameter_filter"
end
def test_redirect_to

@ -454,15 +454,18 @@ def teardown
assert_equal Mime::XML, request.negotiate_mime([Mime::XML, Mime::CSV])
end
test "filter_parameters" do
request = stub_request
request.stubs(:request_parameters).returns({ "foo" => 1 })
request.stubs(:query_parameters).returns({ "bar" => 2 })
assert_raises RuntimeError do
ActionDispatch::Http::ParametersFilter.filter_parameters
class FilterRequest < ActionDispatch::Request
end
test "filter_parameters raises error without arguments" do
assert_raises RuntimeError do
FilterRequest.filter_parameters
end
end
test "process parameter filter" do
request = FilterRequest.new({})
test_hashes = [
[{'foo'=>'bar'},{'foo'=>'bar'},%w'food'],
[{'foo'=>'bar'},{'foo'=>'[FILTERED]'},%w'foo'],
@ -473,21 +476,47 @@ def teardown
[{'baz'=>[{'foo'=>'baz'}]}, {'baz'=>[{'foo'=>'[FILTERED]'}]}, %w(foo)]]
test_hashes.each do |before_filter, after_filter, filter_words|
ActionDispatch::Http::ParametersFilter.filter_parameters(*filter_words)
assert_equal after_filter, request.__send__(:process_parameter_filter, before_filter)
FilterRequest.filter_parameters(*filter_words)
assert_equal after_filter, request.send(:process_parameter_filter, before_filter)
filter_words.push('blah')
ActionDispatch::Http::ParametersFilter.filter_parameters(*filter_words) do |key, value|
FilterRequest.filter_parameters(*filter_words) do |key, value|
value.reverse! if key =~ /bargain/
end
before_filter['barg'] = {'bargain'=>'gain', 'blah'=>'bar', 'bar'=>{'bargain'=>{'blah'=>'foo'}}}
after_filter['barg'] = {'bargain'=>'niag', 'blah'=>'[FILTERED]', 'bar'=>{'bargain'=>{'blah'=>'[FILTERED]'}}}
assert_equal after_filter, request.__send__(:process_parameter_filter, before_filter)
assert_equal after_filter, request.send(:process_parameter_filter, before_filter)
end
end
test "filtered_parameters returns params filtered" do
FilterRequest.filter_parameters(:lifo, :amount)
request = FilterRequest.new('action_dispatch.request.parameters' =>
{ 'lifo' => 'Pratik', 'amount' => '420', 'step' => '1' })
params = request.filtered_parameters
assert_equal "[FILTERED]", params["lifo"]
assert_equal "[FILTERED]", params["amount"]
assert_equal "1", params["step"]
end
test "filtered_env filters env as a whole" do
FilterRequest.filter_parameters(:lifo, :amount)
request = FilterRequest.new('action_dispatch.request.parameters' =>
{ 'amount' => '420', 'step' => '1' }, "RAW_POST_DATA" => "yada yada")
request = FilterRequest.new(request.filtered_env)
assert_equal "[FILTERED]", request.raw_post
assert_equal "[FILTERED]", request.params["amount"]
assert_equal "1", request.params["step"]
end
protected
def stub_request(env={})

@ -1,4 +1,3 @@
class ApplicationController < ActionController::Base
protect_from_forgery
filter_parameter_logging :password
end

@ -32,7 +32,6 @@ class Application < Rails::Application
# end
# Configure sensitive parameters which will be filtered from the log file.
# Check the documentation for ActionDispatch::Http::ParametersFilter for more information.
# config.filter_parameters :password
config.filter_parameters :password
end
end

@ -254,7 +254,7 @@ def i18n
end
def filter_parameters(*filter_words, &block)
ActionDispatch::Http::ParametersFilter.filter_parameters(*filter_words, &block)
ActionDispatch::Request.filter_parameters(*filter_words, &block)
end
def environment_path