Use ActiveSupport::Notifications for auditing

Using Active Record to inject audit records into a database table is
cumbersome because we have mix them in as part of the model code. By
using ActiveSupport::Notifications we can gain flexibility by making
the auditing declarable and having multiple listeners, e.g. logging
all the events to something like LogStash for full auditing whilst
also listening on specific events for things like posting Slack
notifications when a planning application status changes.

.github/workflows/build.yml

@@ -62,6 +62,7 @@ jobs:
           - { group: "bops_admin", pattern: "*_spec.rb", module: "engines" }
           - { group: "bops_api", pattern: "*_spec.rb", module: "engines" }
           - { group: "bops_config", pattern: "*_spec.rb", module: "engines" }
+          - { group: "bops_core", pattern: "*_spec.rb", module: "engines" }
       fail-fast: false
     with:
       name: "${{matrix.specs.group}}: ${{matrix.specs.pattern }}"

.rspec

@@ -1,3 +1,4 @@
+-I engines/bops_core/spec
 -I engines/bops_admin/spec
 -I engines/bops_api/spec
 -I engines/bops_config/spec

.rubocop.yml

@@ -78,6 +78,11 @@ Rails/ThreeStateBooleanColumn:
     - db/migrate/20230*
     - db/migrate/202310*
 
+Rails/ApplicationJob:
+  Exclude:
+    # specs ignored so we can test concerns in isolation
+    - engines/*/spec/**/*
+
 # False positive: if being used in migrations, behaviour is different and intentional
 Rails/ApplicationRecord:
   Exclude:

Makefile

@@ -45,6 +45,9 @@ admin-specs:
 config-specs:
 	$(DOCKER-RUN) console rspec engines/bops_config/spec
 
+core-specs:
+	$(DOCKER-RUN) console rspec engines/bops_core/spec
+
 rspec:
 	$(DOCKER-RUN) console rspec
 

app/models/application_type.rb

@@ -1,8 +1,11 @@
 # frozen_string_literal: true
 
 class ApplicationType < ApplicationRecord
+  include BopsCore::AuditableModel
   include StoreModel::NestedAttributes
 
+  self.audit_attributes = %w[code]
+
   NAME_ORDER = %w[prior_approval planning_permission lawfulness_certificate other].freeze
   ODP_APPLICATION_TYPES = I18n.t(:"odp.application_types").to_h.freeze
 

app/models/user.rb

@@ -1,8 +1,11 @@
 # frozen_string_literal: true
 
 class User < ApplicationRecord
-  enum role: {assessor: 0, reviewer: 1, administrator: 2, global_administrator: 3}
+  include BopsCore::AuditableModel
+
+  self.audit_attributes = %w[id name role]
 
+  enum role: {assessor: 0, reviewer: 1, administrator: 2, global_administrator: 3}
   enum otp_delivery_method: {sms: 0, email: 1}
 
   devise :recoverable, :two_factor_authenticatable, :recoverable, :timeoutable,

engines/bops_config/app/controllers/bops_config/application_controller.rb

@@ -2,6 +2,16 @@
 
 module BopsConfig
   class ApplicationController < ActionController::Base
+    include BopsCore::AuditableController
+
+    self.audit_payload = -> {
+      {
+        engine: "bops_config",
+        params: request.path_parameters,
+        user: current_user.audit_attributes
+      }
+    }
+
     default_form_builder GOVUKDesignSystemFormBuilder::FormBuilder
 
     before_action :authenticate_user!

engines/bops_config/app/controllers/bops_config/application_types_controller.rb

@@ -6,6 +6,16 @@ class ApplicationTypesController < ApplicationController
     before_action :set_application_types, only: %i[index]
     before_action :set_application_type, only: %i[show edit update]
 
+    audit :create, :update,
+      unless: -> { @application_type.changed? },
+      payload: -> {
+        {
+          application_type: @application_type.audit_attributes,
+          changes: @application_type.audit_changes,
+          automated: false
+        }
+      }
+
     def index
       respond_to do |format|
         format.html

engines/bops_config/spec/bops_config_helper.rb

@@ -2,6 +2,8 @@
 
 require "rails_helper"
 
+Dir[BopsCore::Engine.root.join("spec/support/**/*.rb")].each { |f| require f }
+
 RSpec.configure do |config|
   config.before type: :system do |example|
     Capybara.app_host = "http://config.bops.services"

engines/bops_config/spec/controllers/bops_config/application_types_controller_spec.rb

@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+require "bops_config_helper"
+
+RSpec.describe BopsConfig::ApplicationTypesController, type: :controller do
+  let(:user) { create(:user, :global_administrator) }
+
+  before do
+    sign_in(user)
+  end
+
+  routes { BopsConfig::Engine.routes }
+
+  describe "#create" do
+    context "with invalid params" do
+      it "doesn't send an audit event" do
+        expect {
+          post :create, params: {application_type: {code: "", suffix: ""}}
+        }.not_to have_audit("created.application_type")
+      end
+    end
+
+    context "with valid params" do
+      it "sends an audit event" do
+        expect {
+          post :create, params: {application_type: {code: "advertConsent", suffix: "ADVT"}}
+        }.to have_audit("created.application_type").with_payload(a_hash_including(
+          engine: "bops_config",
+          params: {action: "create", controller: "bops_config/application_types"},
+          user: {"id" => user.id, "name" => user.name, "role" => user.role},
+          application_type: {"code" => "advertConsent"},
+          changes: a_hash_including(
+            "code" => [nil, "advertConsent"],
+            "suffix" => [nil, "ADVT"]
+          ),
+          automated: false
+        ))
+      end
+    end
+  end
+
+  describe "#update" do
+    let!(:application_type) { create(:application_type, :inactive, code: "advertConsent", suffix: "ADVR") }
+
+    context "with invalid params" do
+      it "doesn't send an audit event" do
+        expect {
+          post :update, params: {id: application_type.id, application_type: {code: "", suffix: ""}}
+        }.not_to have_audit("updated.application_type")
+      end
+    end
+
+    context "with valid params" do
+      it "sends an audit event" do
+        expect {
+          post :update, params: {id: application_type.id, application_type: {code: "advertConsent", suffix: "ADVT"}}
+        }.to have_audit("updated.application_type").with_payload(a_hash_including(
+          engine: "bops_config",
+          params: {action: "update", controller: "bops_config/application_types", id: application_type.to_param},
+          user: {"id" => user.id, "name" => user.name, "role" => user.role},
+          application_type: {"code" => "advertConsent"},
+          changes: a_hash_including(
+            "suffix" => ["ADVR", "ADVT"]
+          ),
+          automated: false
+        ))
+      end
+    end
+  end
+end

engines/bops_core/app/controllers/concerns/bops_core/auditable_controller.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+module BopsCore
+  module AuditableController
+    extend ActiveSupport::Concern
+    include Auditable
+
+    AUDITABLE_ACTIONS = {}.tap do |actions|
+      actions.merge!(
+        "create" => "created",
+        "update" => "updated",
+        "destroy" => "destroyed"
+      )
+
+      actions.default_proc = ->(_, key) { key }
+    end.freeze
+
+    module ClassMethods
+      def audit(*actions, event: nil, payload: {}, **options)
+        after_action(only: actions, **options) do
+          default_event = [
+            AUDITABLE_ACTIONS[action_name],
+            controller_name.singularize
+          ].join(".")
+
+          audit(event || default_event, payload)
+        end
+      end
+    end
+  end
+end

engines/bops_core/app/jobs/concerns/bops_core/auditable_job.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+module BopsCore
+  module AuditableJob
+    extend ActiveSupport::Concern
+    include Auditable
+
+    module ClassMethods
+      def audit(event, payload: {}, **options)
+        after_perform(**options) do
+          audit(event, payload)
+        end
+      end
+    end
+  end
+end

engines/bops_core/app/lib/bops_core/auditable.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+module BopsCore
+  module Auditable
+    extend ActiveSupport::Concern
+
+    included do
+      class_attribute :audit_payload, instance_writer: false, default: -> { {} }
+    end
+
+    def audit(event, payload = {}, &)
+      event = "#{event}.bops_audit"
+
+      if payload.is_a?(Symbol)
+        payload = send(payload)
+      elsif payload.is_a?(Proc)
+        payload = instance_exec(&payload)
+      end
+
+      if audit_payload.is_a?(Symbol)
+        payload.merge!(send(audit_payload))
+      elsif audit_payload.is_a?(Proc)
+        payload.merge!(instance_exec(&audit_payload))
+      else
+        payload.merge!(audit_payload)
+      end
+
+      if block_given?
+        ActiveSupport::Notifications.instrument(event, payload, &)
+      else
+        ActiveSupport::Notifications.instrument(event, payload)
+      end
+    end
+  end
+end

engines/bops_core/app/mailers/concerns/bops_core/auditable_mailer.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module BopsCore
+  module AuditableMailer
+    extend ActiveSupport::Concern
+    include Auditable
+
+    module ClassMethods
+      def audit(*actions, event: nil, payload: {}, **options)
+        after_deliver(only: actions, **options) do
+          default_event = [mailer_name, action_name].join(".")
+          audit(event || default_event, payload)
+        end
+      end
+    end
+  end
+end

engines/bops_core/app/models/concerns/bops_core/auditable_model.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module BopsCore
+  module AuditableModel
+    extend ActiveSupport::Concern
+    include Auditable
+
+    included do
+      with_options instance_accessor: false do
+        class_attribute :audit_attributes, default: %w[id]
+        class_attribute :audit_changes, default: %w[created_at updated_at]
+      end
+    end
+
+    def audit_attributes
+      attributes.slice(*self.class.audit_attributes)
+    end
+
+    def audit_changes
+      previous_changes.except(*self.class.audit_changes)
+    end
+  end
+end

engines/bops_core/spec/bops_core_helper.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+Dir[BopsCore::Engine.root.join("spec/support/**/*.rb")].each { |f| require f }

engines/bops_core/spec/controllers/bops_core/auditable_controller_spec.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+require "bops_core_helper"
+
+RSpec.describe BopsCore::AuditableController, type: :controller do
+  controller(ActionController::Base) do
+    include BopsCore::AuditableController
+
+    audit :create, event: "event.scope", payload: {foo: "bar"}
+
+    def create
+    end
+  end
+
+  it "sends an audit event for the create action" do
+    expect {
+      post :create
+    }.to have_audit("event.scope").with_payload(foo: "bar")
+  end
+end