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

New configurable settings: encryption for audited_changes & filtering encrypted attrs #694

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,17 @@ class User < ActiveRecord::Base
end
```

You can disable the filtering by adding a config

```ruby
Audited.filter_encrypted_attributes = false
```

If you want to encrypt the changes that are audited, you can simply add this line to your config
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe another header here to show that these are separate features?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

```ruby
Audited.encrypt_audited_changes = true
```

### Custom `Audit` model

If you want to extend or modify the audit model, create a new class that
Expand Down
4 changes: 4 additions & 0 deletions lib/audited.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ class << self
attr_accessor \
:auditing_enabled,
:current_user_method,
:encrypt_audited_changes,
:filter_encrypted_attributes,
:ignored_attributes,
:ignored_default_callbacks,
:max_audits,
Expand Down Expand Up @@ -40,6 +42,8 @@ def config

@current_user_method = :current_user
@auditing_enabled = true
@encrypt_audited_changes = false
@filter_encrypted_attributes = true
@store_synthesized_enums = false
end

Expand Down
4 changes: 4 additions & 0 deletions lib/audited/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ class Audit < ::ActiveRecord::Base
serialize :audited_changes, YAMLIfTextColumnType
end

if Rails.gem_version >= Gem::Version.new("7.0") && Audited.encrypt_audited_changes
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

encrypts :audited_changes

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to take into account the deterministic encryption?

end

scope :ascending, -> { reorder(version: :asc) }
scope :descending, -> { reorder(version: :desc) }
scope :creates, -> { where(action: "create") }
Expand Down
6 changes: 6 additions & 0 deletions lib/audited/auditor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,8 @@ def redact_values(filtered_changes)
end

def filter_encrypted_attrs(filtered_changes)
return filtered_changes unless filter_encrypted_attributes?

filter_attr_values(
audited_changes: filtered_changes,
attrs: respond_to?(:encrypted_attributes) ? Array(encrypted_attributes).map(&:to_s) : []
Expand Down Expand Up @@ -420,6 +422,10 @@ def reconstruct_attributes(audits)
audits.each { |audit| attributes.merge!(audit.new_attributes) }
attributes
end

def filter_encrypted_attributes?
Audited.filter_encrypted_attributes
end
end

module AuditedClassMethods
Expand Down
20 changes: 19 additions & 1 deletion spec/audited/audit_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "spec_helper"

SingleCov.covered! uncovered: 2 # Rails version check
SingleCov.covered! uncovered: 5 # Rails version check

class CustomAudit < Audited::Audit
def custom_method
Expand Down Expand Up @@ -74,6 +74,24 @@ class Models::ActiveRecord::CustomUserSubclass < Models::ActiveRecord::CustomUse
audit.audited_changes = {foo: "bar"}
expect(audit.audited_changes).to eq "{:foo=>\"bar\"}"
end

if ::ActiveRecord::VERSION::MAJOR >= 7
context "when encryption is enabled" do
before do
Audited.encrypt_audited_changes = true
end

it "encrypts the whole column" do
company = Models::ActiveRecord::EncryptCompanyAuditedChanges.create!(name: "CollectiveIdea")

audit = company.audits.last
audited_changes = audit.audited_changes

expect({"name"=>"CollectiveIdea", "owner_id"=>nil}).not_to eq(audit.ciphertext_for(:audited_changes))
expect({"name"=>"CollectiveIdea", "owner_id"=>nil}).to eq(audited_changes)
end
end
end
end

describe "#undo" do
Expand Down
16 changes: 14 additions & 2 deletions spec/audited/auditor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# not testing proxy_respond_to? hack / 2 methods / deprecation of `version`
# also, an additional 6 around `after_touch` for Versions before 6.
uncovered = (ActiveRecord::VERSION::MAJOR < 6) ? 15 : 9
uncovered = (ActiveRecord::VERSION::MAJOR < 6) ? 16 : 9
SingleCov.covered! uncovered: uncovered

class ConditionalPrivateCompany < ::ActiveRecord::Base
Expand Down Expand Up @@ -267,11 +267,23 @@ class CallbacksSpecified < ::ActiveRecord::Base
end

if ::ActiveRecord::VERSION::MAJOR >= 7
it "should filter encrypted attributes" do
it "should filter encrypted attributes by default" do
user = Models::ActiveRecord::UserWithEncryptedPassword.create(password: "password")
user.save
expect(user.audits.last.audited_changes["password"]).to eq("[FILTERED]")
end

context "when filtering is disabled" do
before do
Audited.filter_encrypted_attributes = false
end

it "should not filter encrypted attributes" do
user = Models::ActiveRecord::UserWithEncryptedPassword.create(password: "password")
user.save
expect(user.audits.last.audited_changes["password"]).not_to eq("[FILTERED]")
end
end
end

if ActiveRecord::Base.connection.adapter_name == "PostgreSQL"
Expand Down
3 changes: 3 additions & 0 deletions spec/support/active_record/models.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ class Company < ::ActiveRecord::Base
audited
end

class EncryptCompanyAuditedChanges < Company
end

class Company::STICompany < Company
end

Expand Down
Loading