[Docs]Add missing entry for encryption.add_to_filter_paramters

[hachi8833]

Motivation / Background

This Pull Request has been created because the new config entry for #46453 is missing in configuring.md guide.

Detail

This Pull Request adds config.active_record.encryption.add_to_filter_parameters to configuring.md.

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]

[hachi8833]

The lint system detected the missing entries in railties/lib/rails/application/configuration.rb and railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_1.rb.tt.
I added the entries to them and removed ci-skip from the PR title.

[skipkayhil]

I'm a bit confused about this one. Is the goal just to document how it is already or to change what the value will be with load_defaults 7.1?

[skipkayhil]

Suggested change

	| (original) | `true` |
	| 7.1 | `false` |
	| (original) | `false` |
	| 7.1 | `true` |

[hachi8833]

My bad. Fixed the inverted values.

[skipkayhil]

Is the intention to make this the default in new apps? This isn't really a doc change anymore is it?

[hachi8833]

Fixed the doc and I think we can now leave it as the new 7.1 default value.

[hachi8833]

@skipkayhil Sorry, I found that I unintentionally closed the PR...

Unfortunately, after that the situation has been changed that new_framework_defaults_7_1.rb.tt has been renamed to new_framework_defaults_7_2.rb.tt in main branch, thus I guess the PR should not be merged into the current main branch.

I'm afraid I should recreate the PR and send to 7-1-stable branch, as well as send another PR to main branch just for updating configuring.md.
Should I do that? Or should I leave this as it is?

[skipkayhil]

I'm afraid I should recreate the PR and send to 7-1-stable branch, as well as send another PR to main branch just for updating configuring.md.

Hmm, the documentation for main will kind of end up depending on whether the frame default ends up accepted. I'd start with a PR to 7-1-stable I think? I'll admit this is a weird edge case of the usual workflow 😅

cc @rafaelfranca do you think we should add this new framework default in 7.1?

[rafaelfranca]

I personally didn't understand the need. Isn't active_record.encryption.add_to_filter_parameters already true? Which value are we trying to make the new framework default?

[rafaelfranca]

I actually don't think this should be a config at all. We should never to add encrypted value to the logs. Even on inspect. We don't do for password hashes.

[hachi8833]

I got it and leave this closed as it is.

[skipkayhil]

We don't do for password hashes.

If I understand correctly, I think this is because of the configuration we put in new apps:

rails/railties/lib/rails/generators/rails/app/templates/config/initializers/filter_parameter_logging.rb.tt

Line 7 in deb0869

:passw, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn

When add_to_filer_parameters is set to true, all encrypted model attributes are added to this list automatically.

[skipkayhil]

Oh, yes I understand now. The configuration is true by default as you say. Apps can set it to false to opt out if they want.

@hachi8833 so what we can do is a new documentation PR that just adds the
#### config.active_record.encryption.add_to_filter_parameters section if you are still interested.

[hachi8833]

Thank you for the explanation! I'll try the new PR perhaps today evening.