Merge pull request #33832 from bogdanvlviv/follow-up-33756

Follow up #33756

5 files changed, 55 insertions, 31 deletions

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/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