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

Audited changes included into audits to STDOUT #878

Merged
merged 1 commit into from Jun 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/models/account.rb
Expand Up @@ -72,7 +72,7 @@ def self.columns

scope :not_master, -> { where(master: [false, nil]) }

audited allow_mass_assignment: true
audited allow_mass_assignment: true, sensitive_attributes: %i[site_access_code credit_card_partial_number credit_card_expires_on credit_card_auth_code payment_gateway_options credit_card_authorize_net_payment_profile_token]

# this is done in a callback because we want to do this AFTER the account is deleted
# otherwise the before_destroy admin check in the user will stop the deletion
Expand Down
2 changes: 1 addition & 1 deletion app/models/contract.rb
Expand Up @@ -4,7 +4,7 @@ class Contract < ApplicationRecord
# https://github.com/collectiveidea/audited/blob/f03c5b5d1717f2ebec64032d269316dc74476056/lib/audited/auditor.rb#L305-L311
self.table_name = 'cinstances'

audited :allow_mass_assignment => true
audited allow_mass_assignment: true, sensitive_attributes: %i[user_key]
include ::ThreeScale::MethodTracing

# FIXME: This class should be an abstract class I think, but doing so makes plenty of tests fail
Expand Down
2 changes: 1 addition & 1 deletion app/models/payment_detail.rb
Expand Up @@ -10,7 +10,7 @@ class PaymentDetail < ApplicationRecord

belongs_to :account

audited :allow_mass_assignment => true
audited allow_mass_assignment: true, sensitive_attributes: %i[payment_service_reference credit_card_partial_number credit_card_expires_on]

attr_protected :account_id, :audit_ids

Expand Down
2 changes: 1 addition & 1 deletion app/models/settings.rb
Expand Up @@ -3,7 +3,7 @@
class Settings < ApplicationRecord
belongs_to :account, inverse_of: :settings

audited allow_mass_assignment: true
audited allow_mass_assignment: true, sensitive_attributes: %i[janrain_api_key cms_token sso_key]

attr_protected :account_id, :tenant_id, :product, :audit_ids, :sso_key, :heroku_id, :heroku_name

Expand Down
2 changes: 1 addition & 1 deletion app/models/user.rb
Expand Up @@ -22,7 +22,7 @@ def self.columns
include Permissions
include ProvidedAccessTokens

audited
audited sensitive_attributes: %i[crypted_password salt activation_code lost_password_token password_digest]

before_validation :trim_white_space_from_username
before_destroy :can_be_destroyed?
Expand Down
25 changes: 20 additions & 5 deletions config/initializers/audited_hacks.rb
Expand Up @@ -35,6 +35,18 @@ def log_to_stdout
logger.tagged('audit', kind, action) { logger.info log_trail }
end

FILTERED = '[FILTERED]'.to_sym

def obfuscated
sentitive_attributes = kind.constantize.sensitive_attributes.map(&:to_s) & audited_changes.keys
filtered_hash = sentitive_attributes.map { |attr_name| [attr_name, FILTERED] }.to_h
copy = dup
copy.send(:write_attribute, :id, id)
copy.send(:write_attribute, :created_at, created_at)
copy.audited_changes.merge! filtered_hash
copy
end

protected

def log_trail
Expand All @@ -44,10 +56,10 @@ def log_trail
alias_method :to_s, :log_trail

def to_h_safe
hash = attributes.slice(*%w[auditable_type auditable_id action version provider_id user_id user_type request_uuid remote_address created_at])
attrs = %w[auditable_type auditable_id action audited_changes version provider_id user_id user_type request_uuid remote_address created_at]
hash = obfuscated.attributes.slice(*attrs)
hash['user_role'] = user&.role
hash['audit_id'] = id
hash['changed_attributes'] = audited_changes.keys if action != 'create'
hash
end
end
Expand Down Expand Up @@ -88,14 +100,17 @@ def enqueue_job
end

module AuditedHacks
extend ActiveSupport::Concern

def self.included klass
klass.extend ClassMethods
included do
class_attribute :sensitive_attributes
extend ClassMethods
end

module ClassMethods

def audited(options = {})
self.sensitive_attributes = options.delete(:sensitive_attributes) || []

super

self.disable_auditing if Rails.env.test?
Expand Down
2 changes: 1 addition & 1 deletion test/factories/audit.rb
Expand Up @@ -12,7 +12,7 @@
kind { auditable_type }

action { 'create' }
audited_changes { { 'org_name' => ['Previous', 'Current'] } }
audited_changes { { 'org_name' => 'Some Org Name' } }
version { 1 }
request_uuid { SecureRandom.uuid }

Expand Down
52 changes: 25 additions & 27 deletions test/unit/audited_hacks_test.rb
Expand Up @@ -31,7 +31,7 @@ class LoggingTest < ActiveSupport::TestCase
setup do
@provider = FactoryBot.create(:simple_provider)
User.current = FactoryBot.create(:admin, account: provider)

@audit_class = Audited.audit_class
@audit = FactoryBot.build(:audit, provider_id: provider.id)

Expand Down Expand Up @@ -78,23 +78,11 @@ class LoggingTest < ActiveSupport::TestCase
audit.log_to_stdout
end

test 'safe hash for create action' do
assert_equal expected_hash_new_record, audit.send(:to_h_safe)

test 'safe hash' do
audit = FactoryBot.build(:audit, provider_id: provider.id)
assert_equal expected_hash(audit), audit.send(:to_h_safe)
audit.save!

assert_equal expected_hash_persisted, audit.send(:to_h_safe)
end

test 'sahe hash for update action' do
audit = FactoryBot.build(:audit, action: 'update', provider_id: provider.id)
changed_attributes = { action: 'update', changed_attributes: ['org_name'] }.stringify_keys

assert_equal expected_hash_new_record(audit).merge(changed_attributes), audit.send(:to_h_safe)

audit.save!

assert_equal expected_hash_persisted(audit).merge(changed_attributes), audit.send(:to_h_safe)
assert_equal expected_hash_persisted(audit), audit.send(:to_h_safe)
end

test 'log trail' do
Expand All @@ -108,31 +96,41 @@ class LoggingTest < ActiveSupport::TestCase
assert_equal expected_hash_persisted(audit).to_json, audit.send(:log_trail)
end

test 'obfuscated audit' do
provider_id = provider.id
Settings.stubs(sensitive_attributes: %i[sso_key])
settings = Settings.new(account_id: provider_id)
audit = FactoryBot.build(:audit, auditable_type: settings.class.name, auditable_id: 123, provider_id: provider_id, audited_changes: { 'welcome_text' => 'hello', 'sso_key' => 'sensitive' })
audit_obfuscated = audit.obfuscated
assert_not_equal audit.object_id, audit_obfuscated.object_id
assert_equal({ 'welcome_text' => 'hello', 'sso_key' => 'sensitive' }, audit.audited_changes)
assert_equal({ 'welcome_text' => 'hello', 'sso_key' => '[FILTERED]'.to_sym }, audit_obfuscated.audited_changes)
end

protected

def expected_hash_new_record(audit = @audit)
def expected_hash(audit = @audit)
provider_id = audit.provider_id
expected_hash = {
user = User.current
{
auditable_type: 'Account',
auditable_id: provider_id,
action: 'create',
audited_changes: { 'org_name' => 'Some Org Name' },
version: 1,
provider_id: provider_id,
user_id: nil,
user_type: nil,
user_id: user&.id,
user_type: user&.class.to_s.presence,
request_uuid: audit.request_uuid,
remote_address: nil,
created_at: nil,
user_role: nil,
user_role: user&.role,
audit_id: nil
}
user = User.current
expected_hash.merge!(user_id: user.id, user_type: 'User', user_role: user.role) if user
expected_hash.stringify_keys
}.stringify_keys
end

def expected_hash_persisted(audit = @audit)
expected_hash_new_record(audit).merge({ created_at: audit.created_at, audit_id: audit.id }.stringify_keys)
expected_hash(audit).merge({ created_at: audit.created_at, audit_id: audit.id }.stringify_keys)
end
end
end