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.

Sign up for GitHub

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

Jump to bottom

Encryption for audited_changes / disabling the FILTERED feature #690

Open
20goto10 opened this issue Nov 9, 2023 · 4 comments · May be fixed by #694
Open

Encryption for audited_changes / disabling the FILTERED feature #690

20goto10 opened this issue Nov 9, 2023 · 4 comments · May be fixed by #694

Comments

@20goto10
Copy link

20goto10 commented Nov 9, 2023

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
Copy link
Member

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

@sriddbs
Copy link
Contributor

sriddbs commented Jan 13, 2024

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

@sriddbs sriddbs linked a pull request Jan 15, 2024 that will close this issue
Open
@20goto10
Copy link
Author

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

tvararu added a commit to nhsuk/manage-vaccinations-in-schools that referenced this issue May 29, 2024
@tvararu
Use audited fork that allows reading encrypted attrs
3f09c7e 
Audited automatically replaces entries in the `audited_changes` field on
audits with `[FILTERED]` for attributes that are encrypted. This is
designed to prevent leaking of sensitive information in
`audited_changes` which is an unencrypted field.

The downside to this is that the `audited_changes` field now provides
less information about what the audit actually changed.

To solve this, collectiveidea/audited#694 adds
additional configuration attributes:

- `Audited.filter_encrypted_attributes = false` disables the automatic
  replacement with `[FILTERED]`
- `Audited.encrypt_audited_changes = true` encrypts the actual entire
  `audited_changes` field, ensuring that sensitive information isn't
  leaked

See:

- collectiveidea/audited#690
- collectiveidea/audited#694
@tvararu tvararu mentioned this issue May 29, 2024
Merged
tvararu added a commit to nhsuk/manage-vaccinations-in-schools that referenced this issue May 29, 2024
@tvararu
Use audited fork that allows reading encrypted attrs
f97eb27 
Audited automatically replaces entries in the `audited_changes` field on
audits with `[FILTERED]` for attributes that are encrypted. This is
designed to prevent leaking of sensitive information in
`audited_changes` which is an unencrypted field.

The downside to this is that the `audited_changes` field now provides
less information about what the audit actually changed.

To solve this, collectiveidea/audited#694 adds
additional configuration attributes:

- `Audited.filter_encrypted_attributes = false` disables the automatic
  replacement with `[FILTERED]`
- `Audited.encrypt_audited_changes = true` encrypts the actual entire
  `audited_changes` field, ensuring that sensitive information isn't
  leaked

See:

- collectiveidea/audited#690
- collectiveidea/audited#694
tvararu added a commit to nhsuk/manage-vaccinations-in-schools that referenced this issue May 29, 2024
@tvararu
Use audited fork that allows reading encrypted attrs
5a8bd35 
Audited automatically replaces entries in the `audited_changes` field on
audits with `[FILTERED]` for attributes that are encrypted. This is
designed to prevent leaking of sensitive information in
`audited_changes` which is an unencrypted field.

The downside to this is that the `audited_changes` field now provides
less information about what the audit actually changed.

To solve this, collectiveidea/audited#694 adds
additional configuration attributes:

- `Audited.filter_encrypted_attributes = false` disables the automatic
  replacement with `[FILTERED]`
- `Audited.encrypt_audited_changes = true` encrypts the actual entire
  `audited_changes` field, ensuring that sensitive information isn't
  leaked

See:

- collectiveidea/audited#690
- collectiveidea/audited#694
tvararu added a commit to nhsuk/manage-vaccinations-in-schools that referenced this issue May 29, 2024
@tvararu
Use audited fork that allows reading encrypted attrs
22457fa 
Audited automatically replaces entries in the `audited_changes` field on
audits with `[FILTERED]` for attributes that are encrypted. This is
designed to prevent leaking of sensitive information in
`audited_changes` which is an unencrypted field.

The downside to this is that the `audited_changes` field now provides
less information about what the audit actually changed.

To solve this, collectiveidea/audited#694 adds
additional configuration attributes:

- `Audited.filter_encrypted_attributes = false` disables the automatic
  replacement with `[FILTERED]`
- `Audited.encrypt_audited_changes = true` encrypts the actual entire
  `audited_changes` field, ensuring that sensitive information isn't
  leaked

See:

- collectiveidea/audited#690
- collectiveidea/audited#694
@sb-truefalse
Copy link

Due to the inability to get the original value from audited_changes, I support the proposal to encrypt the audit completely

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@danielmorrison @20goto10 @sriddbs @sb-truefalse