Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add `inspection_masks` to make values of sensitive database columns w… #33756

Merged
merged 1 commit into from Sep 7, 2018
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -1,3 +1,12 @@
* Configuration item `config.filter_parameters` could also filter out sensitive value of database column when call `#inspect`.

```
Rails.application.config.filter_parameters += [:credit_card_number]
Account.last.insepct # => #<Account id: 123, credit_card_number: [FILTERED] ...>
```

*Zhang Kang*

* Deprecate `column_name_length`, `table_name_length`, `columns_per_table`,
`indexes_per_table`, `columns_per_multicolumn_index`, `sql_query_length`,
and `joins_per_query` methods in `DatabaseLimits`.
@@ -8,6 +8,8 @@ module ActiveRecord
module Core
extend ActiveSupport::Concern

FILTERED = "[FILTERED]" # :nodoc:

included do
##
# :singleton-method:
@@ -123,6 +125,10 @@ def self.configurations

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: []

def self.connection_handler
ActiveRecord::RuntimeRegistry.connection_handler || default_connection_handler
end
@@ -487,12 +493,17 @@ def connection_handler

# 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)
"#{name}: #{attribute_for_inspect(name)}"
if filter_attributes.include?(name) && !read_attribute(name).nil?
"#{name}: #{ActiveRecord::Core::FILTERED}"
else
"#{name}: #{attribute_for_inspect(name)}"
end
end
end.compact.join(", ")
else
@@ -506,6 +517,7 @@ def inspect
# 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? }
@@ -516,7 +528,11 @@ def pretty_print(pp)
pp.text column_name
pp.text ":"
pp.breakable
pp.pp column_value
if filter_attributes.include?(column_name) && !column_value.nil?
pp.text ActiveRecord::Core::FILTERED
else
pp.pp column_value
end
end
end
else
@@ -235,5 +235,11 @@ class Railtie < Rails::Railtie # :nodoc:
end
end
end

initializer "active_record.set_filter_attributes" do
ActiveSupport.on_load(:active_record) do
self.filter_attributes += Rails.application.config.filter_parameters
end
end
end
end
@@ -0,0 +1,80 @@
# frozen_string_literal: true

require "cases/helper"
require "models/admin"
require "models/admin/user"
require "models/admin/account"
require "pp"

class FilterAttributesTest < ActiveRecord::TestCase
fixtures :"admin/users", :"admin/accounts"

setup do
ActiveRecord::Base.filter_attributes = [:name]
end

teardown do
ActiveRecord::Base.filter_attributes = []
end

test "filter_attributes" do
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_includes account.inspect, "name: [FILTERED]"
assert_equal 1, account.inspect.scan("[FILTERED]").length
end
end

test "filter_attributes could be overwritten by models" do
Admin::Account.all.each do |account|
assert_includes account.inspect, "name: [FILTERED]"
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
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
account = Admin::Account.new

assert_includes account.inspect, "name: nil"
assert_not_includes account.inspect, "name: [FILTERED]"
assert_equal 0, account.inspect.scan("[FILTERED]").length
end

test "filter_attributes on pretty_print" do
user = admin_users(:david)
actual = "".dup
PP.pp(user, StringIO.new(actual))

assert_includes actual, "name: [FILTERED]"
assert_equal 1, actual.scan("[FILTERED]").length
end

test "filter_attributes on pretty_print should not filter nil value" do
user = Admin::User.new
actual = "".dup
PP.pp(user, StringIO.new(actual))

assert_includes actual, "name: nil"
assert_not_includes actual, "name: [FILTERED]"
assert_equal 0, actual.scan("[FILTERED]").length
end
end
@@ -1996,6 +1996,15 @@ class ::DummySerializer < ActiveJob::Serializers::ObjectSerializer; end
assert_equal false, ActionView::Template.finalize_compiled_template_methods
end

test "ActiveRecord::Base.filter_attributes should equal to filter_parameters" do
app_file "config/initializers/filter_parameters_logging.rb", <<-RUBY
Rails.application.config.filter_parameters += [ :password, :credit_card_number ]
RUBY
app "development"
assert_equal [ :password, :credit_card_number ], Rails.application.config.filter_parameters
assert_equal [ :password, :credit_card_number ], ActiveRecord::Base.filter_attributes
end

private
def force_lazy_load_hooks
yield # Tasty clarifying sugar, homie! We only need to reference a constant to load it.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.