diff --git a/.github/workflows/buildlight.yml b/.github/workflows/buildlight.yml new file mode 100644 index 00000000..af2ac1f0 --- /dev/null +++ b/.github/workflows/buildlight.yml @@ -0,0 +1,15 @@ +name: Buildlight + +on: + workflow_run: + workflows: + - CI + branches: + - main + +jobs: + webhook: + runs-on: ubuntu-latest + steps: + - name: Webhook + uses: collectiveidea/buildlight@main diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..6e9cdf84 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,145 @@ +name: CI + +on: + - pull_request + - push + +jobs: + build: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + ruby: [2.3, 2.4, 2.5, 2.6, 2.7, 3.0, 3.1, 3.2] + appraisal: + - rails50 + - rails51 + - rails52 + - rails60 + - rails61 + - rails70 + - rails71 + db: [POSTGRES, MYSQL, SQLITE] + exclude: + # MySQL has issues on Ruby 2.3 + # https://github.com/ruby/setup-ruby/issues/150 + - ruby: 2.3 + db: MYSQL + + # PostgreSQL is segfaulting on 2.3 + # Doesn't seem worth solving. + - ruby: 2.3 + db: POSTGRES + + # Rails 5.0 supports Ruby 2.2-2.4 + - appraisal: rails50 + ruby: 2.5 + - appraisal: rails50 + ruby: 2.6 + - appraisal: rails50 + ruby: 2.7 + - appraisal: rails50 + ruby: 3.0 + - appraisal: rails50 + ruby: 3.1 + - appraisal: rails50 + ruby: 3.2 + + # Rails 5.1 supports Ruby 2.2-2.5 + - appraisal: rails51 + ruby: 2.6 + - appraisal: rails51 + ruby: 2.7 + - appraisal: rails51 + ruby: 3.0 + - appraisal: rails51 + ruby: 3.1 + - appraisal: rails51 + ruby: 3.2 + + # Rails 5.2 supports Ruby 2.2-2.5 + - appraisal: rails52 + ruby: 2.6 + - appraisal: rails52 + ruby: 2.7 + - appraisal: rails52 + ruby: 3.0 + - appraisal: rails52 + ruby: 3.1 + - appraisal: rails52 + ruby: 3.2 + + # Rails 6.0 supports Ruby 2.5-2.7 + - appraisal: rails60 + ruby: 2.3 + - appraisal: rails60 + ruby: 2.4 + - appraisal: rails60 + ruby: 3.0 + - appraisal: rails60 + ruby: 3.1 + - appraisal: rails60 + ruby: 3.2 + + # Rails 6.1 supports Ruby 2.5+ + - appraisal: rails61 + ruby: 2.3 + - appraisal: rails61 + ruby: 2.4 + + # Rails 7 supports Ruby 2.7+ + - appraisal: rails70 + ruby: 2.3 + - appraisal: rails70 + ruby: 2.4 + - appraisal: rails70 + ruby: 2.5 + - appraisal: rails70 + ruby: 2.6 + + # Rails 7.1 supports Ruby 2.7+ + - appraisal: rails71 + ruby: 2.3 + - appraisal: rails71 + ruby: 2.4 + - appraisal: rails71 + ruby: 2.5 + - appraisal: rails71 + ruby: 2.6 + + services: + postgres: + image: postgres + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + POSTGRES_DB: audited_test + ports: + - 5432:5432 + # needed because the postgres container does not provide a healthcheck + options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5 + + env: + DB_DATABASE: audited_test + DB_USER: root + DB_PASSWORD: 'root' + DB_HOST: localhost + + steps: + - name: Setup MySQL + run: | + sudo /etc/init.d/mysql start + mysql -e 'CREATE DATABASE audited_test;' -uroot -proot + mysql -e 'SHOW DATABASES;' -uroot -proot + - uses: actions/checkout@v3 + - name: Copy Gemfile + run: sed 's/\.\././' gemfiles/${{ matrix.appraisal }}.gemfile > Gemfile + - name: Set up Ruby ${{ matrix.ruby }} + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby }} + bundler-cache: true + - name: Run tests + env: + DB: ${{ matrix.db }} + run: bundle exec rake diff --git a/Appraisals b/Appraisals index c06a96f9..e724b7b4 100644 --- a/Appraisals +++ b/Appraisals @@ -6,6 +6,8 @@ appraise "rails50" do gem "mysql2", ">= 0.3.18", "< 0.6.0" gem "pg", ">= 0.18", "< 2.0" gem "sqlite3", "~> 1.3.6" + gem "psych", "~> 3.1" + gem "loofah", "2.20.0" end appraise "rails51" do @@ -13,13 +15,17 @@ appraise "rails51" do gem "mysql2", ">= 0.3.18", "< 0.6.0" gem "pg", ">= 0.18", "< 2.0" gem "sqlite3", "~> 1.3.6" + gem "psych", "~> 3.1" + gem "loofah", "2.20.0" end appraise "rails52" do - gem "rails", ">= 5.2.0", "< 5.3" + gem "rails", ">= 5.2.8.1", "< 5.3" gem "mysql2", ">= 0.4.4", "< 0.6.0" gem "pg", ">= 0.18", "< 2.0" gem "sqlite3", "~> 1.3.6" + gem "psych", "~> 3.1" + gem "loofah", "2.20.0" end appraise "rails60" do @@ -37,7 +43,14 @@ appraise "rails61" do end appraise "rails70" do - gem "rails", ">= 7.0.0.alpha2", "< 7.1" + gem "rails", ">= 7.0.0", "< 7.1" + gem "mysql2", ">= 0.4.4" + gem "pg", ">= 1.1" + gem "sqlite3", ">= 1.4" +end + +appraise "rails71" do + gem "rails", ">= 7.1.0.beta1", "< 7.2" gem "mysql2", ">= 0.4.4" gem "pg", ">= 1.1" gem "sqlite3", ">= 1.4" diff --git a/CHANGELOG.md b/CHANGELOG.md index ef9300e6..aa9ba411 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,90 @@ # Audited ChangeLog +## 5.4.2 (2023-11-30) + +- Revert replacing RequetStore with ActiveSupport::CurrentAttributes until it is fully tested. + +## 5.4.1 (2023-11-30) + +- Replace RequestStore with ActiveSupport::CurrentAttributes - @the-spectator + [#673](https://github.com/collectiveidea/audited/pull/673/) +- Don't require railtie when used outside of Rails - @nicduke38degrees + [#665](https://github.com/collectiveidea/audited/pull/665) + +## 5.4.0 (2023-09-30) + +- Add Rails 7.1 support - @yuki24 + [#686](https://github.com/collectiveidea/audited/pull/686) + +## 5.3.3 (2023-03-24) + +- Use RequestStore instead of Thread.current for thread-safe requests - @tiagocassio + [#669](https://github.com/collectiveidea/audited/pull/669) +- Clean up Touch audits - @mcyoung, @akostadinov + [#668](https://github.com/collectiveidea/audited/pull/668) + +## 5.3.2 (2023-02-22) + +- Touch audit bug fixes - @mcyoung + [#662](https://github.com/collectiveidea/audited/pull/662) + +## 5.3.1 (2023-02-21) + +- Ensure touch support doesn't cause double audits - @mcyoung + [#660](https://github.com/collectiveidea/audited/pull/660) +- Testing Improvements - @vlad-psh + [#628](https://github.com/collectiveidea/audited/pull/628) +- Testing Improvements - @mcyoung + [#658](https://github.com/collectiveidea/audited/pull/658) + +## 5.3.0 (2023-02-14) + +- Audit touch calls - @mcyoung + [#657](https://github.com/collectiveidea/audited/pull/657) +- Allow using with Padrino and other non-Rails projects - @nicduke38degrees + [#655](https://github.com/collectiveidea/audited/pull/655) +- Testing updates - @jdufresne + [#652](https://github.com/collectiveidea/audited/pull/652) + [#653](https://github.com/collectiveidea/audited/pull/653) + +## 5.2.0 (2023-01-23) + +Improved + +- config.audit_class can take a string or constant - @rocket-turtle + Fixes overzealous change in 5.1.0 where it only took a string. + [#648](https://github.com/collectiveidea/audited/pull/648) +- README link fix - @jeremiahlukus + [#646](https://github.com/collectiveidea/audited/pull/646) +- Typo fix in GitHub Actions - @jdufresne + [#644](https://github.com/collectiveidea/audited/pull/644) + +## 5.1.0 (2022-12-23) + +Changed + +- config.audit_class takes a string - @simmerz + [#609](https://github.com/collectiveidea/audited/pull/609) +- Filter encrypted attributes automatically - @vlad-psh + [#630](https://github.com/collectiveidea/audited/pull/630) + +Improved + +- README improvements - @jess, @mstroming + [#605](https://github.com/collectiveidea/audited/pull/605) + [#640](https://github.com/collectiveidea/audited/issues/640) +- Ignore deadlocks in concurrent audit combinations - @Crammaman + [#621](https://github.com/collectiveidea/audited/pull/621) +- Fix timestamped_migrations deprecation warning - @shouichi + [#624](https://github.com/collectiveidea/audited/pull/624) +- Ensure audits are re-enabled after blocks - @dcorlett + [#632](https://github.com/collectiveidea/audited/pull/632) +- Replace raw string where clause with query methods - @macowie + [#642](https://github.com/collectiveidea/audited/pull/642) +- Test against more Ruby/Rails Versions - @enomotodev, @danielmorrison + [#610](https://github.com/collectiveidea/audited/pull/610) + [#643](https://github.com/collectiveidea/audited/pull/643) + ## 5.0.2 (2021-09-16) Added @@ -11,7 +96,7 @@ Improved - Improve loading - @mvastola [#592](https://github.com/collectiveidea/audited/pull/592) -- Update README - @danirod, clement1234 +- Update README - @danirod, @clement1234 [#596](https://github.com/collectiveidea/audited/pull/596) [#594](https://github.com/collectiveidea/audited/pull/594) diff --git a/README.md b/README.md index 1ec927c0..f12257be 100644 --- a/README.md +++ b/README.md @@ -2,14 +2,13 @@ Audited [![Gem Version](https://img.shields.io/gem/v/audited.svg)](http://rubygems.org/gems/audited) ![Build Status](https://github.com/collectiveidea/audited/actions/workflows/ci.yml/badge.svg) [![Code Climate](https://codeclimate.com/github/collectiveidea/audited.svg)](https://codeclimate.com/github/collectiveidea/audited) -[![Security](https://hakiri.io/github/collectiveidea/audited/master.svg)](https://hakiri.io/github/collectiveidea/audited/master) [![Ruby Style Guide](https://img.shields.io/badge/code_style-standard-brightgreen.svg)](https://github.com/testdouble/standard) ======= **Audited** (previously acts_as_audited) is an ORM extension that logs all changes to your models. Audited can also record who made those changes, save comments and associate models related to the changes. -Audited currently (5.x) works with Rails 7.0, 6.1, 6.0, 5.2, 5.1, and 5.0. +Audited currently (5.x) works with Rails 7.1, 7.0, 6.1, 6.0, 5.2, 5.1, and 5.0. For Rails 4, use gem version 4.x For Rails 3, use gem version 3.0 or see the [3.0-stable branch](https://github.com/collectiveidea/audited/tree/3.0-stable). @@ -18,12 +17,14 @@ For Rails 3, use gem version 3.0 or see the [3.0-stable branch](https://github.c Audited supports and is [tested against](https://github.com/collectiveidea/audited/actions/workflows/ci.yml) the following Ruby versions: -* 2.3 +* 2.3 (only tested on Sqlite due to testing issues with other DBs) * 2.4 * 2.5 * 2.6 * 2.7 * 3.0 +* 3.1 +* 3.2 Audited may work just fine with a Ruby version not listed above, but we can't guarantee that it will. If you'd like to maintain a Ruby that isn't listed, please let us know with a [pull request](https://github.com/collectiveidea/audited/pulls). @@ -36,7 +37,7 @@ Audited is currently ActiveRecord-only. In a previous life, Audited worked with Add the gem to your Gemfile: ```ruby -gem "audited", "~> 5.0" +gem "audited" ``` And if you're using ```require: false``` you must add initializers like this: @@ -237,7 +238,7 @@ class ApplicationController < ActionController::Base if current_user current_user else - 'Elon Musk' + 'Alexander Fleming' end end end @@ -286,6 +287,7 @@ class User < ActiveRecord::Base end class Company < ActiveRecord::Base + audited has_many :users has_associated_audits end @@ -314,8 +316,6 @@ If you want to audit only under specific conditions, you can provide conditional class User < ActiveRecord::Base audited if: :active? - private - def active? last_login > 6.months.ago end @@ -392,6 +392,18 @@ To enable sql column write, default value is false. # Change the setting in the initializer for each application Audited.sql_log_enabled = true ``` + +### Encrypted attributes + +If you're using ActiveRecord's encryption (available from Rails 7) to encrypt some attributes, Audited will automatically filter values of these attributes. No additional configuration is required. Changes to encrypted attributes will be logged as `[FILTERED]`. + +```ruby +class User < ActiveRecord::Base + audited + encrypts :password +end +``` + ### Custom `Audit` model If you want to extend or modify the audit model, create a new class that @@ -408,7 +420,7 @@ Then set it in an initializer: # config/initializers/audited.rb Audited.config do |config| - config.audit_class = CustomAudit + config.audit_class = "CustomAudit" end ``` @@ -424,7 +436,7 @@ Audited.store_synthesized_enums = true ## Support -You can find documentation at: http://rdoc.info/github/collectiveidea/audited +You can find documentation at: https://www.rubydoc.info/gems/audited Or join the [mailing list](http://groups.google.com/group/audited) to get help or offer suggestions. diff --git a/audited.gemspec b/audited.gemspec index 556033b0..b6b31fca 100644 --- a/audited.gemspec +++ b/audited.gemspec @@ -16,10 +16,11 @@ Gem::Specification.new do |gem| gem.required_ruby_version = ">= 2.3.0" - gem.add_dependency "activerecord", ">= 5.0", "< 7.1" + gem.add_dependency "activerecord", ">= 5.0", "< 7.2" + gem.add_dependency "request_store", "~> 1.2" gem.add_development_dependency "appraisal" - gem.add_development_dependency "rails", ">= 5.0", "< 7.1" + gem.add_development_dependency "rails", ">= 5.0", "< 7.2" gem.add_development_dependency "rspec-rails" gem.add_development_dependency "standard" gem.add_development_dependency "single_cov" @@ -30,7 +31,7 @@ Gem::Specification.new do |gem| gem.add_development_dependency "activerecord-jdbcpostgresql-adapter", "~> 1.3" gem.add_development_dependency "activerecord-jdbcmysql-adapter", "~> 1.3" else - gem.add_development_dependency "sqlite3", "~> 1.3" + gem.add_development_dependency "sqlite3", ">= 1.3.6" gem.add_development_dependency "mysql2", ">= 0.3.20" gem.add_development_dependency "pg", ">= 0.18", "< 2.0" end diff --git a/gemfiles/rails50.gemfile b/gemfiles/rails50.gemfile index 9a14d0c2..92d5f8c4 100644 --- a/gemfiles/rails50.gemfile +++ b/gemfiles/rails50.gemfile @@ -6,5 +6,7 @@ gem "rails", "~> 5.0.0" gem "mysql2", ">= 0.3.18", "< 0.6.0" gem "pg", ">= 0.18", "< 2.0" gem "sqlite3", "~> 1.3.6" +gem "psych", "~> 3.1" +gem "loofah", "2.20.0" gemspec name: "audited", path: "../" diff --git a/gemfiles/rails51.gemfile b/gemfiles/rails51.gemfile index 21ceb0a4..fa43c66f 100644 --- a/gemfiles/rails51.gemfile +++ b/gemfiles/rails51.gemfile @@ -6,5 +6,7 @@ gem "rails", "~> 5.1.4" gem "mysql2", ">= 0.3.18", "< 0.6.0" gem "pg", ">= 0.18", "< 2.0" gem "sqlite3", "~> 1.3.6" +gem "psych", "~> 3.1" +gem "loofah", "2.20.0" gemspec name: "audited", path: "../" diff --git a/gemfiles/rails52.gemfile b/gemfiles/rails52.gemfile index 3fcdabaa..b8d7a12b 100644 --- a/gemfiles/rails52.gemfile +++ b/gemfiles/rails52.gemfile @@ -2,9 +2,11 @@ source "https://rubygems.org" -gem "rails", ">= 5.2.0", "< 5.3" +gem "rails", ">= 5.2.8.1", "< 5.3" gem "mysql2", ">= 0.4.4", "< 0.6.0" gem "pg", ">= 0.18", "< 2.0" gem "sqlite3", "~> 1.3.6" +gem "psych", "~> 3.1" +gem "loofah", "2.20.0" gemspec name: "audited", path: "../" diff --git a/gemfiles/rails70.gemfile b/gemfiles/rails70.gemfile index 74c93c73..396fda4d 100644 --- a/gemfiles/rails70.gemfile +++ b/gemfiles/rails70.gemfile @@ -2,7 +2,7 @@ source "https://rubygems.org" -gem "rails", ">= 7.0.0.alpha2", "< 7.1" +gem "rails", ">= 7.0.0", "< 7.1" gem "mysql2", ">= 0.4.4" gem "pg", ">= 1.1" gem "sqlite3", ">= 1.4" diff --git a/gemfiles/rails71.gemfile b/gemfiles/rails71.gemfile new file mode 100644 index 00000000..e34fc967 --- /dev/null +++ b/gemfiles/rails71.gemfile @@ -0,0 +1,10 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "rails", ">= 7.1.0.beta1", "< 7.2" +gem "mysql2", ">= 0.4.4" +gem "pg", ">= 1.1" +gem "sqlite3", ">= 1.4" + +gemspec name: "audited", path: "../" diff --git a/lib/audited.rb b/lib/audited.rb index 299299f5..3c586360 100644 --- a/lib/audited.rb +++ b/lib/audited.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "active_record" +require "request_store" module Audited class << self @@ -14,17 +15,19 @@ class << self attr_writer :audit_class def audit_class - @audit_class ||= Audit + # The audit_class is set as String in the initializer. It can not be constantized during initialization and must + # be constantized at runtime. See https://github.com/collectiveidea/audited/issues/608 + @audit_class = @audit_class.safe_constantize if @audit_class.is_a?(String) + @audit_class ||= Audited::Audit end - def store - current_store_value = Thread.current.thread_variable_get(:audited_store) + # remove audit_model in next major version it was only shortly present in 5.1.0 + alias_method :audit_model, :audit_class + deprecate audit_model: "use Audited.audit_class instead of Audited.audit_model. This method will be removed.", + deprecator: ActiveSupport::Deprecation.new('6.0.0', 'Audited') - if current_store_value.nil? - Thread.current.thread_variable_set(:audited_store, {}) - else - current_store_value - end + def store + RequestStore.store[:audited_store] ||= {} end def config @@ -59,4 +62,4 @@ def config end require "audited/sweeper" -require "audited/railtie" +require "audited/railtie" if Audited.const_defined?(:Rails) diff --git a/lib/audited/audit.rb b/lib/audited/audit.rb index 1bd9153e..997de7ef 100644 --- a/lib/audited/audit.rb +++ b/lib/audited/audit.rb @@ -51,7 +51,11 @@ class Audit < ::ActiveRecord::Base cattr_accessor :audited_class_names self.audited_class_names = Set.new - serialize :audited_changes, YAMLIfTextColumnType + if Rails.version >= "7.1" + serialize :audited_changes, coder: YAMLIfTextColumnType + else + serialize :audited_changes, YAMLIfTextColumnType + end scope :ascending, -> { reorder(version: :asc) } scope :descending, -> { reorder(version: :desc) } @@ -80,14 +84,14 @@ def revision # Returns a hash of the changed attributes with the new values def new_attributes (audited_changes || {}).each_with_object({}.with_indifferent_access) do |(attr, values), attrs| - attrs[attr] = (action == "update" ? values.last : values) + attrs[attr] = (action == "update") ? values.last : values end end # Returns a hash of the changed attributes with the old values def old_attributes (audited_changes || {}).each_with_object({}.with_indifferent_access) do |(attr, values), attrs| - attrs[attr] = (action == "update" ? values.first : values) + attrs[attr] = (action == "update") ? values.first : values end end @@ -181,7 +185,7 @@ def set_version_number if action == "create" self.version = 1 else - collection = Rails::VERSION::MAJOR >= 6 ? self.class.unscoped : self.class + collection = (ActiveRecord::VERSION::MAJOR >= 6) ? self.class.unscoped : self.class max = collection.auditable_finder(auditable_id, auditable_type).maximum(:version) || 0 self.version = max + 1 end diff --git a/lib/audited/auditor.rb b/lib/audited/auditor.rb index d7d674fe..48da81bd 100644 --- a/lib/audited/auditor.rb +++ b/lib/audited/auditor.rb @@ -13,7 +13,7 @@ module Audited # # See Audited::Auditor::ClassMethods#audited # for configuration options - module Auditor #:nodoc: + module Auditor # :nodoc: extend ActiveSupport::Concern CALLBACKS = [:audit_create, :audit_update, :audit_destroy] @@ -84,6 +84,7 @@ def audited(options = {}) after_create :audit_create if audited_options[:on].include?(:create) before_update :audit_update if audited_options[:on].include?(:update) + after_touch :audit_touch if audited_options[:on].include?(:touch) && ::ActiveRecord::VERSION::MAJOR >= 6 before_destroy :audit_destroy if audited_options[:on].include?(:destroy) # Define and set after_audit and around_audit callbacks. This might be useful if you want @@ -172,14 +173,15 @@ def revision_at(date_or_time) # List of attributes that are audited. def audited_attributes audited_attributes = attributes.except(*self.class.non_audited_columns) + audited_attributes = redact_values(audited_attributes) + audited_attributes = filter_encrypted_attrs(audited_attributes) normalize_enum_changes(audited_attributes) end # Returns a list combined of record audits and associated audits. def own_and_associated_audits - Audited.audit_class.unscoped - .where("(auditable_type = :type AND auditable_id = :id) OR (associated_type = :type AND associated_id = :id)", - type: self.class.base_class.name, id: id) + Audited.audit_class.unscoped.where(auditable: self) + .or(Audited.audit_class.unscoped.where(associated: self)) .order(created_at: :desc) end @@ -190,8 +192,13 @@ def combine_audits(audits_to_combine) combine_target.comment = "#{combine_target.comment}\nThis audit is the result of multiple audits being combined." transaction do - combine_target.save! - audits_to_combine.unscope(:limit).where("version < ?", combine_target.version).delete_all + begin + combine_target.save! + audits_to_combine.unscope(:limit).where("version < ?", combine_target.version).delete_all + rescue ActiveRecord::Deadlocked + # Ignore Deadlocks, if the same record is getting its old audits combined more than once at the same time then + # both combining operations will be the same. Ignoring this error allows one of the combines to go through successfully. + end end end @@ -224,8 +231,15 @@ def revision_with(attributes) private - def audited_changes - all_changes = respond_to?(:changes_to_save) ? changes_to_save : changes + def audited_changes(for_touch: false) + all_changes = if for_touch + previous_changes + elsif respond_to?(:changes_to_save) + changes_to_save + else + changes + end + filtered_changes = \ if audited_options[:only].present? all_changes.slice(*self.class.audited_columns) @@ -233,7 +247,15 @@ def audited_changes all_changes.except(*self.class.non_audited_columns) end + if for_touch && (last_audit = audits.last&.audited_changes) + filtered_changes.reject! do |k, v| + last_audit[k].to_json == v.to_json || + last_audit[k].to_json == v[1].to_json + end + end + filtered_changes = redact_values(filtered_changes) + filtered_changes = filter_encrypted_attrs(filtered_changes) filtered_changes = normalize_enum_changes(filtered_changes) filtered_changes.to_hash end @@ -257,19 +279,36 @@ def normalize_enum_changes(changes) end def redact_values(filtered_changes) - [audited_options[:redacted]].flatten.compact.each do |option| - changes = filtered_changes[option.to_s] - new_value = audited_options[:redaction_value] || REDACTED - values = if changes.is_a? Array - changes.map { new_value } - else - new_value - end - hash = {option.to_s => values} - filtered_changes.merge!(hash) + filter_attr_values( + audited_changes: filtered_changes, + attrs: Array(audited_options[:redacted]).map(&:to_s), + placeholder: audited_options[:redaction_value] || REDACTED + ) + end + + def filter_encrypted_attrs(filtered_changes) + filter_attr_values( + audited_changes: filtered_changes, + attrs: respond_to?(:encrypted_attributes) ? Array(encrypted_attributes).map(&:to_s) : [] + ) + end + + # Replace values for given attrs to a placeholder and return modified hash + # + # @param audited_changes [Hash] Hash of changes to be saved to audited version record + # @param attrs [Array] Array of attrs, values of which will be replaced to placeholder value + # @param placeholder [String] Placeholder to replace original attr values + def filter_attr_values(audited_changes: {}, attrs: [], placeholder: "[FILTERED]") + attrs.each do |attr| + next unless audited_changes.key?(attr) + + changes = audited_changes[attr] + values = changes.is_a?(Array) ? changes.map { placeholder } : placeholder + + audited_changes[attr] = values end - filtered_changes + audited_changes end def rails_below?(rails_version) @@ -290,20 +329,27 @@ def audits_to(version = nil) def audit_create write_audit(action: "create", audited_changes: audited_attributes, - comment: audit_comment) + comment: audit_comment) end def audit_update unless (changes = audited_changes).empty? && (audit_comment.blank? || audited_options[:update_with_comment_only] == false) write_audit(action: "update", audited_changes: changes, - comment: audit_comment) + comment: audit_comment) + end + end + + def audit_touch + unless (changes = audited_changes(for_touch: true)).empty? + write_audit(action: "update", audited_changes: changes, + comment: audit_comment) end end def audit_destroy unless new_record? write_audit(action: "destroy", audited_changes: audited_attributes, - comment: audit_comment) + comment: audit_comment) end end @@ -397,7 +443,7 @@ def non_audited_columns=(columns) # end # def without_auditing - auditing_was_enabled = auditing_enabled + auditing_was_enabled = class_auditing_enabled disable_auditing yield ensure @@ -411,7 +457,7 @@ def without_auditing # end # def with_auditing - auditing_was_enabled = auditing_enabled + auditing_was_enabled = class_auditing_enabled enable_auditing yield ensure @@ -435,7 +481,7 @@ def audit_as(user, &block) end def auditing_enabled - Audited.store.fetch("#{table_name}_auditing_enabled", true) && Audited.auditing_enabled + class_auditing_enabled && Audited.auditing_enabled end def auditing_enabled=(val) @@ -450,7 +496,7 @@ def default_ignored_attributes def normalize_audited_options audited_options[:on] = Array.wrap(audited_options[:on]) - audited_options[:on] = [:create, :update, :destroy] if audited_options[:on].empty? + audited_options[:on] = [:create, :update, :touch, :destroy] if audited_options[:on].empty? audited_options[:only] = Array.wrap(audited_options[:only]).map(&:to_s) audited_options[:except] = Array.wrap(audited_options[:except]).map(&:to_s) max_audits = audited_options[:max_audits] || Audited.max_audits @@ -466,6 +512,10 @@ def calculate_non_audited_columns default_ignored_attributes end end + + def class_auditing_enabled + Audited.store.fetch("#{table_name}_auditing_enabled", true) + end end end end diff --git a/lib/audited/version.rb b/lib/audited/version.rb index 6b5c890d..f646fe51 100644 --- a/lib/audited/version.rb +++ b/lib/audited/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Audited - VERSION = "5.0.2" + VERSION = "5.4.2" end diff --git a/lib/generators/audited/migration.rb b/lib/generators/audited/migration.rb index e1390844..1536ed85 100644 --- a/lib/generators/audited/migration.rb +++ b/lib/generators/audited/migration.rb @@ -4,14 +4,22 @@ module Audited module Generators module Migration # Implement the required interface for Rails::Generators::Migration. - def next_migration_number(dirname) #:nodoc: + def next_migration_number(dirname) # :nodoc: next_migration_number = current_migration_number(dirname) + 1 - if ::ActiveRecord::Base.timestamped_migrations + if timestamped_migrations? [Time.now.utc.strftime("%Y%m%d%H%M%S"), "%.14d" % next_migration_number].max else "%.3d" % next_migration_number end end + + private + + def timestamped_migrations? + (Rails.version >= "7.0") ? + ::ActiveRecord.timestamped_migrations : + ::ActiveRecord::Base.timestamped_migrations + end end end end diff --git a/spec/audited/audit_spec.rb b/spec/audited/audit_spec.rb index ae47b7d0..c18abdb5 100644 --- a/spec/audited/audit_spec.rb +++ b/spec/audited/audit_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -SingleCov.covered! uncovered: 1 # Rails version check +SingleCov.covered! uncovered: 2 # Rails version check class CustomAudit < Audited::Audit def custom_method @@ -37,7 +37,7 @@ class Models::ActiveRecord::CustomUserSubclass < Models::ActiveRecord::CustomUse context "when a custom audit class is configured" do it "should be used in place of #{described_class}" do - Audited.config { |config| config.audit_class = CustomAudit } + Audited.config { |config| config.audit_class = "CustomAudit" } TempModel1.audited record = TempModel1.create diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index 08407d1b..265dfba3 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -1,6 +1,9 @@ require "spec_helper" -SingleCov.covered! uncovered: 9 # not testing proxy_respond_to? hack / 2 methods / deprecation of `version` +# 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 +SingleCov.covered! uncovered: uncovered class ConditionalPrivateCompany < ::ActiveRecord::Base self.table_name = "companies" @@ -143,7 +146,7 @@ class Secret2 < ::ActiveRecord::Base end it "should be configurable which attributes are not audited via ignored_attributes" do - Audited.ignored_attributes = ["delta", "top_secret", "created_at"] + Audited.ignored_attributes = ["delta", "top_secret", "created_at", "updated_at"] expect(Secret.non_audited_columns).to include("delta", "top_secret", "created_at") end @@ -215,17 +218,25 @@ def non_column_attr=(val) redacted = Audited::Auditor::AuditedInstanceMethods::REDACTED user = Models::ActiveRecord::UserMultipleRedactedAttributes.create( - password: "password", - ssn: 123456789 + password: "password" ) user.save! expect(user.audits.last.audited_changes["password"]).to eq(redacted) + # Saving '[REDACTED]' value for 'ssn' even if value wasn't set explicitly when record was created expect(user.audits.last.audited_changes["ssn"]).to eq(redacted) + user.password = "new_password" user.ssn = 987654321 user.save! expect(user.audits.last.audited_changes["password"]).to eq([redacted, redacted]) expect(user.audits.last.audited_changes["ssn"]).to eq([redacted, redacted]) + + # If we haven't changed any attrs from 'redacted' list, audit should not contain these keys + user.name = "new name" + user.save! + expect(user.audits.last.audited_changes).to have_key("name") + expect(user.audits.last.audited_changes).not_to have_key("password") + expect(user.audits.last.audited_changes).not_to have_key("ssn") end it "should redact columns in 'redacted' column with custom option" do @@ -234,6 +245,14 @@ def non_column_attr=(val) expect(user.audits.last.audited_changes["password"]).to eq(["My", "Custom", "Value", 7]) end + if ::ActiveRecord::VERSION::MAJOR >= 7 + it "should filter encrypted attributes" do + user = Models::ActiveRecord::UserWithEncryptedPassword.create(password: "password") + user.save + expect(user.audits.last.audited_changes["password"]).to eq("[FILTERED]") + end + end + if ActiveRecord::Base.connection.adapter_name == "PostgreSQL" describe "'json' and 'jsonb' audited_changes column type" do let(:migrations_path) { SPEC_ROOT.join("support/active_record/postgres") } @@ -272,11 +291,11 @@ def non_column_attr=(val) yesterday = 1.day.ago u = Models::ActiveRecord::NoAttributeProtectionUser.new(name: "name", - username: "username", - password: "password", - activated: true, - suspended_at: yesterday, - logins: 2) + username: "username", + password: "password", + activated: true, + suspended_at: yesterday, + logins: 2) expect(u.name).to eq("name") expect(u.username).to eq("username") @@ -406,6 +425,99 @@ def non_column_attr=(val) end end + if ::ActiveRecord::VERSION::MAJOR >= 6 + describe "on touch" do + before do + @user = create_user(name: "Brandon", status: :active) + end + + it "should save an audit" do + expect { @user.touch(:suspended_at) }.to change(Audited::Audit, :count).by(1) + end + + it "should set the action to 'update'" do + @user.touch(:suspended_at) + expect(@user.audits.last.action).to eq("update") + expect(Audited::Audit.updates.order(:id).last).to eq(@user.audits.last) + expect(@user.audits.updates.last).to eq(@user.audits.last) + end + + it "should store the changed attributes" do + @user.touch(:suspended_at) + expect(@user.audits.last.audited_changes["suspended_at"][0]).to be_nil + expect(Time.parse(@user.audits.last.audited_changes["suspended_at"][1].to_s)).to be_within(2.seconds).of(Time.current) + end + + it "should store audit comment" do + @user.audit_comment = "Here exists a touch comment" + @user.touch(:suspended_at) + expect(@user.audits.last.action).to eq("update") + expect(@user.audits.last.comment).to eq("Here exists a touch comment") + end + + it "should not save an audit if only specified on create/destroy" do + on_create_destroy = Models::ActiveRecord::OnCreateDestroyUser.create(name: "Bart") + expect { + on_create_destroy.touch(:suspended_at) + }.to_not change(Audited::Audit, :count) + end + + it "should store an audit if touch is the only audit" do + on_touch = Models::ActiveRecord::OnTouchOnly.create(name: "Bart") + expect { + on_touch.update(name: "NotBart") + }.to_not change(Audited::Audit, :count) + expect { + on_touch.touch(:suspended_at) + }.to change(on_touch.audits, :count).from(0).to(1) + + @user.audits.destroy_all + expect(@user.audits).to be_empty + expect { + @user.touch(:suspended_at) + }.to change(@user.audits, :count).from(0).to(1) + end + + context "don't double audit" do + let(:user) { Models::ActiveRecord::Owner.create(name: "OwnerUser", suspended_at: 1.month.ago, companies_attributes: [{name: "OwnedCompany"}]) } + let(:company) { user.companies.first } + + it "should only create 1 (create) audit for object" do + expect(user.audits.count).to eq(1) + expect(user.audits.first.action).to eq("create") + end + + it "should only create 1 (create) audit for nested resource" do + expect(company.audits.count).to eq(1) + expect(company.audits.first.action).to eq("create") + end + + context "after creating" do + it "updating / touching nested resource shouldn't save touch audit on parent object" do + expect { company.touch(:type) }.not_to change(user.audits, :count) + expect { company.update(type: "test") }.not_to change(user.audits, :count) + end + + it "updating / touching parent object shouldn't save previous data" do + expect { user.touch(:suspended_at) }.to change(user.audits, :count).from(1).to(2) + expect(user.audits.last.action).to eq("update") + expect(user.audits.last.audited_changes.keys).to eq(%w[suspended_at]) + end + end + + context "after updating" do + it "changing nested resource shouldn't audit owner" do + expect { user.update(username: "test") }.to change(user.audits, :count).from(1).to(2) + expect { company.update(type: "test") }.not_to change(user.audits, :count) + + expect { user.touch(:suspended_at) }.to change(user.audits, :count).from(2).to(3) + expect { company.update(type: "another_test") }.not_to change(user.audits, :count) + end + end + end + end + end + describe "on destroy" do before do @user = create_user(status: :active) @@ -814,6 +926,15 @@ def stub_global_max_audits(max_audits) }.to_not change(Audited::Audit, :count) end + context "when global audits are disabled" do + it "should re-enable class audits after #without_auditing block" do + Audited.auditing_enabled = false + Models::ActiveRecord::User.without_auditing {} + Audited.auditing_enabled = true + expect(Models::ActiveRecord::User.auditing_enabled).to eql(true) + end + end + it "should reset auditing status even it raises an exception" do begin Models::ActiveRecord::User.without_auditing { raise } @@ -884,6 +1005,15 @@ def stub_global_max_audits(max_audits) }.to change(Audited::Audit, :count).by(1) end + context "when global audits are disabled" do + it "should re-enable class audits after #with_auditing block" do + Audited.auditing_enabled = false + Models::ActiveRecord::User.with_auditing {} + Audited.auditing_enabled = true + expect(Models::ActiveRecord::User.auditing_enabled).to eql(true) + end + end + it "should reset auditing status even it raises an exception" do Models::ActiveRecord::User.disable_auditing begin diff --git a/spec/audited_spec.rb b/spec/audited_spec.rb index db5e6379..aa142199 100644 --- a/spec/audited_spec.rb +++ b/spec/audited_spec.rb @@ -3,16 +3,12 @@ describe Audited do describe "#store" do describe "maintains state of store" do - let(:current_user) { "current_user" } + let(:current_user) { RequestStore.store[:audited_store] } before { Audited.store[:current_user] = current_user } - it "when executed without fibers" do + it "checks store is not nil" do expect(Audited.store[:current_user]).to eq(current_user) end - - it "when executed with Fibers" do - Fiber.new { expect(Audited.store[:current_user]).to eq(current_user) }.resume - end end end end diff --git a/spec/rails_app/config/application.rb b/spec/rails_app/config/application.rb index ba52037d..dca65709 100644 --- a/spec/rails_app/config/application.rb +++ b/spec/rails_app/config/application.rb @@ -4,6 +4,35 @@ module RailsApp class Application < Rails::Application config.root = File.expand_path("../../", __FILE__) config.i18n.enforce_available_locales = true + + if Rails.version.start_with?("7.1") && config.active_record.respond_to?(:yaml_column_permitted_classes=) + config.active_record.yaml_column_permitted_classes = [ + String, + Symbol, + Integer, + NilClass, + Float, + Time, + Date, + FalseClass, + Hash, + Array, + DateTime, + TrueClass, + BigDecimal, + ActiveSupport::TimeWithZone, + ActiveSupport::TimeZone, + ActiveSupport::HashWithIndifferentAccess + ] + elsif !Rails.version.start_with?("5.0") && !Rails.version.start_with?("5.1") && config.active_record.respond_to?(:yaml_column_permitted_classes=) + config.active_record.yaml_column_permitted_classes = + %w[String Symbol Integer NilClass Float Time Date FalseClass Hash Array DateTime TrueClass BigDecimal + ActiveSupport::TimeWithZone ActiveSupport::TimeZone ActiveSupport::HashWithIndifferentAccess] + end + + if Rails.version >= "7.1" + config.active_support.cache_format_version = 7.1 + end end end diff --git a/spec/rails_app/config/environments/test.rb b/spec/rails_app/config/environments/test.rb index 7ceef3a3..78d3403c 100644 --- a/spec/rails_app/config/environments/test.rb +++ b/spec/rails_app/config/environments/test.rb @@ -44,4 +44,9 @@ # Raises error for missing translations # config.action_view.raise_on_missing_translations = true + + if ::ActiveRecord::VERSION::MAJOR >= 7 + config.active_record.encryption.key_derivation_salt = SecureRandom.hex + config.active_record.encryption.primary_key = SecureRandom.hex + end end diff --git a/spec/support/active_record/models.rb b/spec/support/active_record/models.rb index a770ca9f..9acceb43 100644 --- a/spec/support/active_record/models.rb +++ b/spec/support/active_record/models.rb @@ -8,7 +8,12 @@ class User < ::ActiveRecord::Base attribute :non_column_attr if Rails.version >= "5.1" attr_protected :logins if respond_to?(:attr_protected) enum status: {active: 0, reliable: 1, banned: 2} - serialize :phone_numbers, Array + + if Rails.version >= "7.1" + serialize :phone_numbers, type: Array + else + serialize :phone_numbers, Array + end def name=(val) write_attribute(:name, CGI.escapeHTML(val)) @@ -41,6 +46,14 @@ class UserRedactedPasswordCustomRedaction < ::ActiveRecord::Base audited redacted: :password, redaction_value: ["My", "Custom", "Value", 7] end + if ::ActiveRecord::VERSION::MAJOR >= 7 + class UserWithEncryptedPassword < ::ActiveRecord::Base + self.table_name = :users + audited + encrypts :password + end + end + class CommentRequiredUser < ::ActiveRecord::Base self.table_name = :users audited except: :password, comment_required: true @@ -116,11 +129,12 @@ class Owner < ::ActiveRecord::Base audited has_associated_audits has_many :companies, class_name: "OwnedCompany", dependent: :destroy + accepts_nested_attributes_for :companies end class OwnedCompany < ::ActiveRecord::Base self.table_name = "companies" - belongs_to :owner, class_name: "Owner" + belongs_to :owner, class_name: "Owner", touch: true attr_accessible :name, :owner if respond_to?(:attr_accessible) # declare attr_accessible before calling aaa audited associated_with: :owner end @@ -138,6 +152,11 @@ class OnCreateDestroy < ::ActiveRecord::Base audited on: [:create, :destroy] end + class OnCreateDestroyUser < ::ActiveRecord::Base + self.table_name = "users" + audited on: [:create, :destroy] + end + class OnCreateDestroyExceptName < ::ActiveRecord::Base self.table_name = "companies" audited except: :name, on: [:create, :destroy] @@ -147,5 +166,10 @@ class OnCreateUpdate < ::ActiveRecord::Base self.table_name = "companies" audited on: [:create, :update] end + + class OnTouchOnly < ::ActiveRecord::Base + self.table_name = "users" + audited on: [:touch] + end end end