collectiveidea / audited

Audited (formerly acts_as_audited) is an ORM extension that logs all changes to your Rails models.
MIT License
3.34k stars 645 forks source link

Encryption for audited_changes / disabling the FILTERED feature #690

Open 20goto10 opened 7 months ago

20goto10 commented 7 months ago

We recently set up Activerecord 7 encryption in our app, which was already using Audited, and then realized we needed to disable the automatic filtering of AR-encrypted fields for the sake of our record keeping policy. But the data still needed to be stored encrypted (also a matter of policy). So, we encrypted the entire audited_changes column for the Audit model. This required a local patch.

Setting up AR to encrypt the whole field was pretty straightforward. This module override goes in a library loaded in our initializers:

module Audited
  class Audit < ::ActiveRecord::Base
    encrypts :audited_changes
  end
end

Note that you would also need to ensure you have config.active_record.encryption.support_unencrypted_data = true in your application config so AR doesn't choke on older records. (You can avoid needing this by deleting or retroactively encrypting the existing records.)

We then overrode the specific classes that had encrypted fields, with another module:

module UnfilteredAudit
  def filter_encrypted_attrs(input)
    input # i.e. actually DON'T filter the encrypted attrs since we are encrypting the column altogether
  end
end

and then adding include UnfilteredAudit after the audited line in the specific models.

Both these changes are a bit fragile and slightly hacky. So my issue is really a request to make the filtering optional/configurable and to add an option to use encryption on the whole column, both of which could be handled with a couple simple if statements in audited. Note, of course, once you encrypt the audited_changes column you will not be able to directly query the table data in SQL, but it can still be searched programmatically post-decryption.

I would add a PR for this if anyone is interested, but it would likely be a long wait (and maybe these suggestions bear further discussion).

danielmorrison commented 5 months ago

I think this should be built-in. PRs very welcome.

sriddbs commented 5 months ago

I'll be happy to work on this and submit a PR

20goto10 commented 4 months ago

Original poster here... That PR looks like a good implementation to cover our needs. Thanks @sriddbs .