Move audit log to database (#96)

app/interactors/application_interactor.rb

@@ -2,16 +2,17 @@ class ApplicationInteractor
   include Interactor
 
   def audit_log
+    return if context.identity.nil?
     result = context.success? ? "success" : "failure"
-    level = context.success? ? :info : :error
     payload = {
+      request_id: Thread.current[:request_id],
       action: "#{self.class.name}",
       result: result,
       error: context.error&.message,
       subject: context.identity&.subject,
       cert_common_name: context.request&.try(:common_name),
       kv_path: context.request&.try(:kv_path)
     }.compact!
-    AuditLogger.new.send(level, payload)
+    AuditLog.create!(payload)
   end
 end

app/models/audit_log.rb

@@ -0,0 +1,7 @@
+class AuditLog < ApplicationRecord
+  validates :request_id, :action, :result, :subject, presence: true
+
+  if Config[:db_encryption]
+    encrypts :request_id, :action, :result, :error, :subject, :cert_common_name, :kv_path
+  end
+end

config/astral.yml

@@ -31,9 +31,6 @@ shared:
   app_registry_client_cert:
   app_registry_client_key:
 
-  # User activity logging
-  audit_log_file: <%= "#{Rails.root.join('log')}/astral-audit.log" %>
-
   oidc_client_id:
   oidc_client_secret:
   oidc_redirect_uris: http://localhost:8250/oidc/callback

db/migrate/20241120172746_create_audit_logs.rb

@@ -0,0 +1,15 @@
+class CreateAuditLogs < ActiveRecord::Migration[7.2]
+  def change
+    create_table :audit_logs do |t|
+      t.string :request_id, null: false
+      t.string :action, null: false
+      t.string :result, null: false
+      t.string :error, null: true
+      t.string :subject, null: false
+      t.string :cert_common_name, null: true
+      t.string :kv_path, null: true
+      t.timestamps
+    end
+    add_index :audit_logs, [ :subject, :created_at ]
+  end
+end

test/interactors/application_interactor_test.rb

@@ -5,13 +5,7 @@ def setup
     @domain = domains(:owner_match)
     @identity = Identity.new(subject: @domain.users.first)
     @cr = Requests::CertIssueRequest.new(common_name: @domain.fqdn)
-    @log = Tempfile.new("log-test")
-    Config[:audit_log_file] = @log.path
-  end
-
-  def teardown
-    @log.close
-    @log.unlink
+    Thread.current[:request_id] = "request_id"
   end
 
   test ".call will be logged as success" do
@@ -23,7 +17,9 @@ def call
                      end)
     rslt = SuccessAction.call(identity: @identity, request: @cr)
     assert rslt.success?
-    assert_match %Q("action":"SuccessAction","result":"success","subject":"[email protected]","cert_common_name":"example.com"), @log.readlines.last
+    log = AuditLog.last
+    expected = { "action"=>"SuccessAction", "result"=>"success", "subject"=>"[email protected]", "cert_common_name"=>"example.com" }
+    assert expected <= log.attributes
   end
 
   test ".call will be logged as failure" do
@@ -36,6 +32,8 @@ def call
                      end)
     rslt = FailAction.call(identity: @identity, request: @cr)
     assert_not rslt.success?
-    assert_match %Q("action":"FailAction","result":"failure","subject":"[email protected]","cert_common_name":"example.com"), @log.readlines.last
+    log = AuditLog.last
+    expected = { "action"=>"FailAction", "result"=>"failure", "subject"=>"[email protected]", "cert_common_name"=>"example.com" }
+    assert expected <= log.attributes
   end
 end

test/interactors/authenticate_identity_test.rb

@@ -4,6 +4,7 @@ class AuthenticateIdentityTest < ActiveSupport::TestCase
   def setup
     @interactor = AuthenticateIdentity
     @identity = Identity.new(subject: "[email protected]", groups: [ "admin_group" ])
+    Thread.current[:request_id] = "request_id"
   end
 
   test ".call success" do

test/interactors/authorize_cert_request_test.rb

@@ -6,6 +6,7 @@ def setup
     @identity = Identity.new(subject: @domain.users.first)
     @cr = Requests::CertIssueRequest.new(common_name: @domain.fqdn)
     @interactor = AuthorizeCertRequest
+    Thread.current[:request_id] = "request_id"
   end
 
   test ".call with matching owner" do

test/interactors/obtain_cert_test.rb

@@ -4,11 +4,13 @@ class ObtainCertTest < ActiveSupport::TestCase
   def setup
     @interactor = ObtainCert
     @cert = OpenStruct.new(certificate: "certificate", ca_chain: "ca_chain")
+    Thread.current[:request_id] = "request_id"
   end
 
   test ".call success" do
     request = Requests::CertIssueRequest.new
     identity = Identity.new
+    identity.sub = SecureRandom.hex(4)
     mock = Minitest::Mock.new
     mock.expect :call, @cert, [ identity, request ]
     Services::Certificate.stub :issue_cert, mock do
@@ -21,6 +23,7 @@ def setup
   test ".call failure" do
     request = Requests::CertIssueRequest.new
     identity = Identity.new
+    identity.sub = SecureRandom.hex(4)
     mock = Minitest::Mock.new
     mock.expect :call, nil, [ identity, request ]
     Services::Certificate.stub :issue_cert, mock do

test/models/audit_log_test.rb

@@ -0,0 +1,30 @@
+require "test_helper"
+
+class AuditLogTest < ActiveSupport::TestCase
+  def setup
+    @attributes = {
+      request_id: "uuid1",
+      action: "string1",
+      result: "string2",
+      subject: "string3",
+      cert_common_name: "string4"
+    }
+    @audit_log = AuditLog.new(@attributes)
+  end
+
+  test "#new should set attributes from attributes argument" do
+    @attributes.each do |key, value|
+      assert_equal value, @audit_log.send(key), "Attribute #{key} was not set correctly"
+    end
+  end
+
+  test "#valid? should be valid with valid attributes" do
+    assert @audit_log.valid?
+  end
+
+  test "#valid? should require an result" do
+    @audit_log.result = nil
+    assert_not @audit_log.valid?
+    assert_includes @audit_log.errors[:result], "can't be blank"
+  end
+end