New configurable settings: encryption for audited_changes & filtering encrypted attrs#694
Conversation
|
@danielmorrison please review and let me if the proposed config in the PR is good, also I wonder why some of the specs are failing 🤔 |
| Audited.filter_encrypted_attributes = false | ||
| ``` | ||
|
|
||
| If you want to encrypt the changes that are audited, you can simply add this line to your config |
There was a problem hiding this comment.
Maybe another header here to show that these are separate features?
There was a problem hiding this comment.
hmm, because we use encrypts, this config is also available only from Rails 7 ... so I grouped it under the same header
should I add a sub-header?
danielmorrison
left a comment
There was a problem hiding this comment.
Looking good to me. Test errors are something strange with coverage? Doesn't make sense offhand.
I had the same errors with for this it just runs fine |
|
@danielmorrison if this PR looks good, please merge? |
| serialize :audited_changes, YAMLIfTextColumnType | ||
| end | ||
|
|
||
| if Rails.gem_version >= Gem::Version.new("7.0") && Audited.encrypt_audited_changes |
There was a problem hiding this comment.
This doesn't seem to work as expected, it appears that this is evaluated before the Audited.encrypt_audited_changes = true in the initializer runs so it evaluates to false initially and is never re-evaluated. The encrypts call below never runs as a result.
|
How are you doing with PR? |
| end | ||
|
|
||
| if Rails.gem_version >= Gem::Version.new("7.0") && Audited.encrypt_audited_changes | ||
| encrypts :audited_changes |
There was a problem hiding this comment.
Is it necessary to take into account the deterministic encryption?
|
Just stumbled upon this PR. I'd love to have this functionality. Is anything blocking this currently @danielmorrison ? |
Closes #690
encrypt_audited_changesdefault tofalsefilter_encrypted_attributesdefault totrue