From 301f9cdd47bf754660de9fdafe323b544e46aef1 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Thu, 21 Jan 2010 11:48:27 +0700 Subject: [PATCH 2/2] Add deprecation warning for calling filter_parameter_logging ActionController::Base, and allow it to be configured in config.filter_parameters --- .../metal/filter_parameter_logging.rb | 29 +---------- actionpack/test/controller/filter_params_test.rb | 51 -------------------- .../rails/app/templates/config/application.rb | 4 ++ railties/lib/rails/configuration.rb | 4 ++ railties/test/application/configuration_test.rb | 12 +++++ 5 files changed, 22 insertions(+), 78 deletions(-) delete mode 100644 actionpack/test/controller/filter_params_test.rb diff --git a/actionpack/lib/action_controller/metal/filter_parameter_logging.rb b/actionpack/lib/action_controller/metal/filter_parameter_logging.rb index befb4a5..b59f6df 100644 --- a/actionpack/lib/action_controller/metal/filter_parameter_logging.rb +++ b/actionpack/lib/action_controller/metal/filter_parameter_logging.rb @@ -3,36 +3,11 @@ module ActionController extend ActiveSupport::Concern module ClassMethods - # Replace sensitive parameter data 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: - # - # filter_parameter_logging :password - # => replaces the value to all keys matching /password/i with "[FILTERED]" - # - # filter_parameter_logging :foo, "bar" - # => replaces the value to all keys matching /foo|bar/i with "[FILTERED]" - # - # filter_parameter_logging { |k,v| v.reverse! if k =~ /secret/i } - # => reverses the value to all keys matching /secret/i - # - # filter_parameter_logging(:foo, "bar") { |k,v| v.reverse! if k =~ /secret/i } - # => reverses the value to all keys matching /secret/i, and - # replaces the value to all keys matching /foo|bar/i with "[FILTERED]" + # 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 - - protected - - def filter_parameters(params) - request.send(:process_parameter_filter, params) - end end end diff --git a/actionpack/test/controller/filter_params_test.rb b/actionpack/test/controller/filter_params_test.rb deleted file mode 100644 index 4594963..0000000 --- a/actionpack/test/controller/filter_params_test.rb +++ /dev/null @@ -1,51 +0,0 @@ -require 'abstract_unit' - -class FilterParamController < ActionController::Base - def payment - head :ok - end -end - -class FilterParamTest < ActionController::TestCase - tests FilterParamController - - def test_filter_parameters_must_have_one_word - assert_raises RuntimeError do - FilterParamController.filter_parameter_logging - end - end - - def test_filter_parameters - assert FilterParamController.respond_to?(:filter_parameter_logging) - - test_hashes = [ - [{'foo'=>'bar'},{'foo'=>'bar'},%w'food'], - [{'foo'=>'bar'},{'foo'=>'[FILTERED]'},%w'foo'], - [{'foo'=>'bar', 'bar'=>'foo'},{'foo'=>'[FILTERED]', 'bar'=>'foo'},%w'foo baz'], - [{'foo'=>'bar', 'baz'=>'foo'},{'foo'=>'[FILTERED]', 'baz'=>'[FILTERED]'},%w'foo baz'], - [{'bar'=>{'foo'=>'bar','bar'=>'foo'}},{'bar'=>{'foo'=>'[FILTERED]','bar'=>'foo'}},%w'fo'], - [{'foo'=>{'foo'=>'bar','bar'=>'foo'}},{'foo'=>'[FILTERED]'},%w'f banana'], - [{'baz'=>[{'foo'=>'baz'}]}, {'baz'=>[{'foo'=>'[FILTERED]'}]}, %w(foo)]] - - test_hashes.each do |before_filter, after_filter, filter_words| - FilterParamController.filter_parameter_logging(*filter_words) - assert_equal after_filter, @controller.__send__(:filter_parameters, before_filter) - - filter_words.push('blah') - FilterParamController.filter_parameter_logging(*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, @controller.__send__(:filter_parameters, before_filter) - end - end - - def test_filter_parameters_is_protected - FilterParamController.filter_parameter_logging(:foo) - assert !FilterParamController.action_methods.include?('filter_parameters') - assert_raise(NoMethodError) { @controller.filter_parameters([{'password' => '[FILTERED]'}]) } - end -end diff --git a/railties/lib/generators/rails/app/templates/config/application.rb b/railties/lib/generators/rails/app/templates/config/application.rb index 3348208..8a7f024 100644 --- a/railties/lib/generators/rails/app/templates/config/application.rb +++ b/railties/lib/generators/rails/app/templates/config/application.rb @@ -30,5 +30,9 @@ module <%= app_const_base %> # g.template_engine :erb # g.test_framework :test_unit, :fixture => true # 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 end end diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index b76a7ac..ae4f400 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -252,6 +252,10 @@ module Rails i18n end end + + def filter_parameters(*filter_words, &block) + ActionDispatch::Http::ParametersFilter.filter_parameters(*filter_words, &block) + end def environment_path "#{root}/config/environments/#{Rails.env}.rb" diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 79dfacd..8c08fe5 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -122,5 +122,17 @@ module ApplicationTests require "#{app_path}/config/environment" end end + + test "filter_parameters should be able to set via config.filter_parameters" do + add_to_config <<-RUBY + config.filter_parameters :foo, 'bar' do |key, value| + value = value.reverse if key =~ /baz/ + end + RUBY + + assert_nothing_raised do + require "#{app_path}/config/application" + end + end end end -- 1.6.4.2