diff options
| -rw-r--r-- | activejob/test/cases/test_helper_test.rb | 2 | ||||
| -rw-r--r-- | activerecord/CHANGELOG.md | 9 | ||||
| -rw-r--r-- | activerecord/lib/active_record/core.rb | 40 | ||||
| -rw-r--r-- | activerecord/lib/active_record/railtie.rb | 4 | ||||
| -rw-r--r-- | activerecord/test/cases/filter_attributes_test.rb | 32 | ||||
| -rw-r--r-- | guides/source/configuring.md | 2 | ||||
| -rw-r--r-- | railties/test/application/configuration_test.rb | 3 | ||||
| -rw-r--r-- | railties/test/application/loading_test.rb | 33 |
8 files changed, 90 insertions, 35 deletions
diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb index 805dd80ad1..160b876e85 100644 --- a/activejob/test/cases/test_helper_test.rb +++ b/activejob/test/cases/test_helper_test.rb @@ -1566,7 +1566,7 @@ class PerformedJobsTest < ActiveJob::TestCase end end - def test_assert_performed_wiht_with_global_id_args + def test_assert_performed_with_with_global_id_args ricardo = Person.new(9) assert_performed_with(job: HelloJob, args: [ricardo]) do HelloJob.perform_later(ricardo) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 336946b756..0bb5dfe313 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -9,11 +9,16 @@ *Darwin Wu* -* Configuration item `config.filter_parameters` could also filter out sensitive value of database column when call `#inspect`. +* Configuration item `config.filter_parameters` could also filter out + sensitive values of database columns when call `#inspect`. + We also added `ActiveRecord::Base::filter_attributes`/`=` in order to + specify sensitive attributes to specific model. ``` Rails.application.config.filter_parameters += [:credit_card_number] - Account.last.inspect # => #<Account id: 123, credit_card_number: [FILTERED] ...> + Account.last.inspect # => #<Account id: 123, name: "DHH", credit_card_number: [FILTERED] ...> + SecureAccount.filter_attributes += [:name] + SecureAccount.last.inspect # => #<SecureAccount id: 42, name: [FILTERED], credit_card_number: [FILTERED] ...> ``` *Zhang Kang* diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 82cf7563a2..392602bc0f 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -3,6 +3,7 @@ require "active_support/core_ext/hash/indifferent_access" require "active_support/core_ext/string/filters" require "concurrent/map" +require "set" module ActiveRecord module Core @@ -125,9 +126,7 @@ module ActiveRecord class_attribute :default_connection_handler, instance_writer: false - ## - # Specifies columns which don't want to be exposed while calling #inspect - class_attribute :filter_attributes, instance_writer: false, default: [] + self.filter_attributes = [] def self.connection_handler ActiveRecord::RuntimeRegistry.connection_handler || default_connection_handler @@ -140,7 +139,7 @@ module ActiveRecord self.default_connection_handler = ConnectionAdapters::ConnectionHandler.new end - module ClassMethods # :nodoc: + module ClassMethods def initialize_find_by_cache # :nodoc: @find_by_statement_cache = { true => Concurrent::Map.new, false => Concurrent::Map.new } end @@ -217,7 +216,7 @@ module ActiveRecord generated_association_methods end - def generated_association_methods + def generated_association_methods # :nodoc: @generated_association_methods ||= begin mod = const_set(:GeneratedAssociationMethods, Module.new) private_constant :GeneratedAssociationMethods @@ -227,8 +226,22 @@ module ActiveRecord end end + # Returns columns which shouldn't be exposed while calling +#inspect+. + def filter_attributes + if defined?(@filter_attributes) + @filter_attributes + else + superclass.filter_attributes + end + end + + # Specifies columns which shouldn't be exposed while calling +#inspect+. + def filter_attributes=(attributes_names) + @filter_attributes = attributes_names.map(&:to_s).to_set + end + # Returns a string like 'Post(id:integer, title:string, body:text)' - def inspect + def inspect # :nodoc: if self == Base super elsif abstract_class? @@ -244,7 +257,7 @@ module ActiveRecord end # Overwrite the default class equality method to provide support for decorated models. - def ===(object) + def ===(object) # :nodoc: object.is_a?(self) end @@ -493,13 +506,12 @@ module ActiveRecord # Returns the contents of the record as a nicely formatted string. def inspect - filter_attributes = self.filter_attributes.map(&:to_s).to_set # We check defined?(@attributes) not to issue warnings if the object is # allocated but not initialized. inspection = if defined?(@attributes) && @attributes self.class.attribute_names.collect do |name| if has_attribute?(name) - if filter_attributes.include?(name) && !read_attribute(name).nil? + if filter_attribute?(name) "#{name}: #{ActiveRecord::Core::FILTERED}" else "#{name}: #{attribute_for_inspect(name)}" @@ -517,21 +529,19 @@ module ActiveRecord # when pp is required. def pretty_print(pp) return super if custom_inspect_method_defined? - filter_attributes = self.filter_attributes.map(&:to_s).to_set pp.object_address_group(self) do if defined?(@attributes) && @attributes column_names = self.class.column_names.select { |name| has_attribute?(name) || new_record? } pp.seplist(column_names, proc { pp.text "," }) do |column_name| - column_value = read_attribute(column_name) pp.breakable " " pp.group(1) do pp.text column_name pp.text ":" pp.breakable - if filter_attributes.include?(column_name) && !column_value.nil? + if filter_attribute?(column_name) pp.text ActiveRecord::Core::FILTERED else - pp.pp column_value + pp.pp read_attribute(column_name) end end end @@ -583,5 +593,9 @@ module ActiveRecord def custom_inspect_method_defined? self.class.instance_method(:inspect).owner != ActiveRecord::Base.instance_method(:inspect).owner end + + def filter_attribute?(attribute_name) + self.class.filter_attributes.include?(attribute_name) && !read_attribute(attribute_name).nil? + end end end diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 47351588d3..b213754641 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -180,9 +180,7 @@ end_warning end initializer "active_record.set_executor_hooks" do - ActiveSupport.on_load(:active_record) do - ActiveRecord::QueryCache.install_executor_hooks - end + ActiveRecord::QueryCache.install_executor_hooks end initializer "active_record.add_watchable_files" do |app| diff --git a/activerecord/test/cases/filter_attributes_test.rb b/activerecord/test/cases/filter_attributes_test.rb index f88cecfe2b..af5badd87d 100644 --- a/activerecord/test/cases/filter_attributes_test.rb +++ b/activerecord/test/cases/filter_attributes_test.rb @@ -10,11 +10,12 @@ class FilterAttributesTest < ActiveRecord::TestCase fixtures :"admin/users", :"admin/accounts" setup do + @previous_filter_attributes = ActiveRecord::Base.filter_attributes ActiveRecord::Base.filter_attributes = [:name] end teardown do - ActiveRecord::Base.filter_attributes = [] + ActiveRecord::Base.filter_attributes = @previous_filter_attributes end test "filter_attributes" do @@ -35,20 +36,23 @@ class FilterAttributesTest < ActiveRecord::TestCase assert_equal 1, account.inspect.scan("[FILTERED]").length end - Admin::Account.filter_attributes = [] - - # Above changes should not impact other models - Admin::User.all.each do |user| - assert_includes user.inspect, "name: [FILTERED]" - assert_equal 1, user.inspect.scan("[FILTERED]").length + begin + previous_account_filter_attributes = Admin::Account.filter_attributes + Admin::Account.filter_attributes = [] + + # Above changes should not impact other models + Admin::User.all.each do |user| + assert_includes user.inspect, "name: [FILTERED]" + assert_equal 1, user.inspect.scan("[FILTERED]").length + end + + Admin::Account.all.each do |account| + assert_not_includes account.inspect, "name: [FILTERED]" + assert_equal 0, account.inspect.scan("[FILTERED]").length + end + ensure + Admin::Account.filter_attributes = previous_account_filter_attributes end - - Admin::Account.all.each do |account| - assert_not_includes account.inspect, "name: [FILTERED]" - assert_equal 0, account.inspect.scan("[FILTERED]").length - end - - Admin::Account.filter_attributes = [:name] end test "filter_attributes should not filter nil value" do diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 892634c261..4c508f77f1 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -104,7 +104,7 @@ application. Accepts a valid week day symbol (e.g. `:monday`). * `config.filter_parameters` used for filtering out the parameters that you don't want shown in the logs, such as passwords or credit card -numbers. By default, Rails filters out passwords by adding `Rails.application.config.filter_parameters += [:password]` in `config/initializers/filter_parameter_logging.rb`. Parameters filter works by partial matching regular expression. +numbers. It also filters out sensitive values of database columns when call `#inspect` on an Active Record object. By default, Rails filters out passwords by adding `Rails.application.config.filter_parameters += [:password]` in `config/initializers/filter_parameter_logging.rb`. Parameters filter works by partial matching regular expression. * `config.force_ssl` forces all requests to be served over HTTPS by using the `ActionDispatch::SSL` middleware, and sets `config.action_mailer.default_url_options` to be `{ protocol: 'https' }`. This can be configured by setting `config.ssl_options` - see the [ActionDispatch::SSL documentation](http://api.rubyonrails.org/classes/ActionDispatch/SSL.html) for details. diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 83192edb39..44f6af5d4a 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -3,6 +3,7 @@ require "isolation/abstract_unit" require "rack/test" require "env_helpers" +require "set" class ::MyMailInterceptor def self.delivering_email(email); email; end @@ -2049,7 +2050,7 @@ module ApplicationTests RUBY app "development" assert_equal [ :password, :credit_card_number ], Rails.application.config.filter_parameters - assert_equal [ :password, :credit_card_number ], ActiveRecord::Base.filter_attributes + assert_equal [ "password", "credit_card_number" ].to_set, ActiveRecord::Base.filter_attributes end private diff --git a/railties/test/application/loading_test.rb b/railties/test/application/loading_test.rb index 889ad16fb8..d7f4f09665 100644 --- a/railties/test/application/loading_test.rb +++ b/railties/test/application/loading_test.rb @@ -371,6 +371,39 @@ class LoadingTest < ActiveSupport::TestCase end end + test "active record query cache hooks are installed before first request" do + app_file "app/controllers/omg_controller.rb", <<-RUBY + begin + class OmgController < ActionController::Metal + ActiveSupport.run_load_hooks(:action_controller, self) + def show + if ActiveRecord::Base.connection.query_cache_enabled + self.response_body = ["Query cache is enabled."] + else + self.response_body = ["Expected ActiveRecord::Base.connection.query_cache_enabled to be true"] + end + end + end + rescue => e + puts "Error loading metal: \#{e.class} \#{e.message}" + end + RUBY + + app_file "config/routes.rb", <<-RUBY + Rails.application.routes.draw do + get "/:controller(/:action)" + end + RUBY + + require "#{rails_root}/config/environment" + + require "rack/test" + extend Rack::Test::Methods + + get "/omg/show" + assert_equal "Query cache is enabled.", last_response.body + end + private def setup_ar! |