Merge pull request #1711 from unboxed/add-custom-instrumentation-for-…

…auditing

Use ActiveSupport::Notifications for auditing

.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

@@ -81,6 +81,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