From efbb10806157a220e4d1af1ae21678eb16ec9ed0 Mon Sep 17 00:00:00 2001 From: Branko Arnaudovski Date: Wed, 23 Oct 2019 16:30:45 +0200 Subject: [PATCH] Implement Moderators audit logs (#3449) --- .../concerns/audit_instrumentation.rb | 16 +++++ app/controllers/internal/tags_controller.rb | 3 + .../audit/save_to_persistent_storage_job.rb | 21 +++++++ app/models/audit_log.rb | 5 ++ app/services/audit/event/payload.rb | 22 +++++++ app/services/audit/event/util.rb | 53 ++++++++++++++++ app/services/audit/helper.rb | 9 +++ app/services/audit/notification.rb | 43 +++++++++++++ app/services/audit/subscribe.rb | 19 ++++++ config/initializers/audit_events.rb | 9 +++ .../20190708105607_create_audit_logs.rb | 10 +++ ...654_add_additional_columns_to_audit_log.rb | 6 ++ ...806_add_jsonb_data_column_to_audit_logs.rb | 6 ++ db/schema.rb | 13 ++++ spec/factories/activesupport_events.rb | 11 ++++ spec/factories/audit_logs.rb | 4 ++ spec/models/audit_log_spec.rb | 9 +++ spec/requests/internal/moderator_logs_spec.rb | 40 ++++++++++++ spec/services/audit/event/util_spec.rb | 16 +++++ spec/services/audit/helper_spec.rb | 7 +++ spec/services/audit/notification_spec.rb | 62 +++++++++++++++++++ spec/services/audit/subscribe_spec.rb | 21 +++++++ 22 files changed, 405 insertions(+) create mode 100644 app/controllers/concerns/audit_instrumentation.rb create mode 100644 app/jobs/audit/save_to_persistent_storage_job.rb create mode 100644 app/models/audit_log.rb create mode 100644 app/services/audit/event/payload.rb create mode 100644 app/services/audit/event/util.rb create mode 100644 app/services/audit/helper.rb create mode 100644 app/services/audit/notification.rb create mode 100644 app/services/audit/subscribe.rb create mode 100644 config/initializers/audit_events.rb create mode 100644 db/migrate/20190708105607_create_audit_logs.rb create mode 100644 db/migrate/20190801132654_add_additional_columns_to_audit_log.rb create mode 100644 db/migrate/20190906193806_add_jsonb_data_column_to_audit_logs.rb create mode 100644 spec/factories/activesupport_events.rb create mode 100644 spec/factories/audit_logs.rb create mode 100644 spec/models/audit_log_spec.rb create mode 100644 spec/requests/internal/moderator_logs_spec.rb create mode 100644 spec/services/audit/event/util_spec.rb create mode 100644 spec/services/audit/helper_spec.rb create mode 100644 spec/services/audit/notification_spec.rb create mode 100644 spec/services/audit/subscribe_spec.rb diff --git a/app/controllers/concerns/audit_instrumentation.rb b/app/controllers/concerns/audit_instrumentation.rb new file mode 100644 index 000000000..1b020e1d3 --- /dev/null +++ b/app/controllers/concerns/audit_instrumentation.rb @@ -0,0 +1,16 @@ +module AuditInstrumentation + extend ActiveSupport::Concern + + included do + def notify(listener, user, slug) + Audit::Notification.notify(listener) do |payload| + payload.user_id = user.id + payload.roles = user.roles.pluck(:name) + payload.slug = slug + if block_given? + payload.data = yield + end + end + end + end +end diff --git a/app/controllers/internal/tags_controller.rb b/app/controllers/internal/tags_controller.rb index 556952511..a0bac4bc4 100644 --- a/app/controllers/internal/tags_controller.rb +++ b/app/controllers/internal/tags_controller.rb @@ -1,4 +1,5 @@ class Internal::TagsController < Internal::ApplicationController + include AuditInstrumentation layout "internal" def index @@ -23,6 +24,8 @@ class Internal::TagsController < Internal::ApplicationController add_moderator if @add_user_id remove_moderator if @remove_user_id @tag.update!(tag_params) + + notify(:internal, current_user, __method__) { tag_params.dup } redirect_to "/internal/tags/#{params[:id]}" end diff --git a/app/jobs/audit/save_to_persistent_storage_job.rb b/app/jobs/audit/save_to_persistent_storage_job.rb new file mode 100644 index 000000000..38d4badbd --- /dev/null +++ b/app/jobs/audit/save_to_persistent_storage_job.rb @@ -0,0 +1,21 @@ +module Audit + class SaveToPersistentStorageJob < ApplicationJob + queue_as :audit_logs + + def perform(event_string) + Audit::Event::Util.deserialize(event_string). + then { |event| build_params(event) }. + then { |params| AuditLog.create!(params) } + end + + def build_params(event) + { + user_id: event.payload[:user_id], + roles: event.payload[:roles], + slug: event.payload[:slug], + category: event.name, + data: event.payload[:data] + } + end + end +end diff --git a/app/models/audit_log.rb b/app/models/audit_log.rb new file mode 100644 index 000000000..c7dc18335 --- /dev/null +++ b/app/models/audit_log.rb @@ -0,0 +1,5 @@ +class AuditLog < ApplicationRecord + belongs_to :user + + validates :user_id, presence: true +end diff --git a/app/services/audit/event/payload.rb b/app/services/audit/event/payload.rb new file mode 100644 index 000000000..96edbc0e9 --- /dev/null +++ b/app/services/audit/event/payload.rb @@ -0,0 +1,22 @@ +module Audit + module Event + class Payload + ## + # Definition of Event payload. + # + # New instance object is used as block parameter in Audit::Notification.notify method. + attr_accessor :user_id, :roles, :slug, :data + + ## + # Use the initializer to define default values for the payload. + + def initialize + @roles = [] + @slug = :undefined + @data = {} + + yield(self) + end + end + end +end diff --git a/app/services/audit/event/util.rb b/app/services/audit/event/util.rb new file mode 100644 index 000000000..30f993000 --- /dev/null +++ b/app/services/audit/event/util.rb @@ -0,0 +1,53 @@ +module Audit + module Event + class Util + class << self + ## + # These class methods are only used to serialize the object send toward + # ActiveJob. Up until Rails 6, Rails cannot serialize Time objects, + # therefore we need custom serialization. + # + # Additional method is used as Warning, which helps to notify when it is + # save to remove this custom serialization for ActiveSupport::Notifications::Event object + # more at: https://edgeapi.rubyonrails.org/classes/ActiveJob/Serializers/ObjectSerializer.html + # and https://edgeapi.rubyonrails.org/classes/ActiveJob/SerializationError.html + # for supported class instances + + def deserialize(string) + obsolete_usage_warn + + transform_values(string). + then { |obj| ActiveSupport::Notifications::Event.new(*obj) } + end + + def serialize(event) + obsolete_usage_warn + + ActiveSupport::JSON.encode event + end + + private + + def obsolete_usage_warn + warn_message = <<-WARN.strip_heredoc + This Util class becomes obsolete from Rails 6. + Rails 6 adds out of the box, support for Active Job message serialization + for class like Time. + + You can save delete this Util class and remove the usage from + Audit::Notification.listen method, or in any other places + WARN + + Rails.logger.warn(warn_message) if Rails.version.match?(/\A6.\d.\w+/) + end + + def transform_values(string) + ActiveSupport::JSON.decode(string).deep_symbolize_keys.tap do |h| + h[:time] = Time.zone.iso8601(h[:time]) + h[:end] = Time.zone.iso8601(h[:end]) + end.values_at(:name, :time, :end, :transaction_id, :payload) + end + end + end + end +end diff --git a/app/services/audit/helper.rb b/app/services/audit/helper.rb new file mode 100644 index 000000000..84883f0aa --- /dev/null +++ b/app/services/audit/helper.rb @@ -0,0 +1,9 @@ +module Audit + module Helper + NOTIFICATION_SUFFIX = ".audit.log".freeze + + def instrument_name(name) + "#{name}#{NOTIFICATION_SUFFIX}" + end + end +end diff --git a/app/services/audit/notification.rb b/app/services/audit/notification.rb new file mode 100644 index 000000000..d62b5e786 --- /dev/null +++ b/app/services/audit/notification.rb @@ -0,0 +1,43 @@ +module Audit + class Notification + ## + # Main class for wrapping ActiveSupport Instrumentation API. + # + # This class represent main entry point for receiving and notifying custom + # events, implemented according to + # https://guides.rubyonrails.org/active_support_instrumentation.html#creating-custom-events + + class << self + include Audit::Helper + + ## + # Audit::Notification.notify method, receives listener name, which is registered through + # Audit::Notification.listen and the event payload, passed as a block. + # + # Object of Audit::Event::Payload is send to the block as argument. This way, + # the payload object follows the rules defined in Audit::Event::Payload + # + # Example: + # Audit::Notification.notify('listener_name') do |payload| + # payload.user_id = current_user.id + # payload.roles = current_user.roles.pluck(:name) + # end + + def notify(listener, &block) + return unless block_given? + + ActiveSupport::Notifications.instrument(instrument_name(listener), Audit::Event::Payload.new(&block)) + end + + ## + # Audit::Notification.listen receives Events sent from ActiveSupport Instrumentation API. + # Then, this event is serialized and send to background job. + + def listen(*args) + ActiveSupport::Notifications::Event.new(*args). + then { |event| Audit::Event::Util.serialize(event) }. + then { |event_job| Audit::SaveToPersistentStorageJob.perform_later(event_job) } + end + end + end +end diff --git a/app/services/audit/subscribe.rb b/app/services/audit/subscribe.rb new file mode 100644 index 000000000..132208842 --- /dev/null +++ b/app/services/audit/subscribe.rb @@ -0,0 +1,19 @@ +module Audit + class Subscribe + class << self + include Audit::Helper + + def listen(*listeners) + listeners.each do |listener| + ActiveSupport::Notifications.subscribe(instrument_name(listener)) do |*args| + Audit::Notification.listen(*args) + end + end + end + + def forget(*listeners) + listeners.each { |listener| ActiveSupport::Notifications.unsubscribe(instrument_name(listener)) } + end + end + end +end diff --git a/config/initializers/audit_events.rb b/config/initializers/audit_events.rb new file mode 100644 index 000000000..2f05fe3b2 --- /dev/null +++ b/config/initializers/audit_events.rb @@ -0,0 +1,9 @@ +## +# Custom Audit Instrumentation +# +# Put here all custom listener names, for later usage in Audit Instrumentation +# +# Example: +# Audit::Subscribe.listen :admin, :quest_user + +Audit::Subscribe.listen(:moderator, :internal) unless Rails.env.test? diff --git a/db/migrate/20190708105607_create_audit_logs.rb b/db/migrate/20190708105607_create_audit_logs.rb new file mode 100644 index 000000000..11cb38e42 --- /dev/null +++ b/db/migrate/20190708105607_create_audit_logs.rb @@ -0,0 +1,10 @@ +class CreateAuditLogs < ActiveRecord::Migration[5.2] + def change + create_table :audit_logs do |t| + t.references :user, foreign_key: true + t.string :roles, array: true + + t.timestamps + end + end +end diff --git a/db/migrate/20190801132654_add_additional_columns_to_audit_log.rb b/db/migrate/20190801132654_add_additional_columns_to_audit_log.rb new file mode 100644 index 000000000..144e0905a --- /dev/null +++ b/db/migrate/20190801132654_add_additional_columns_to_audit_log.rb @@ -0,0 +1,6 @@ +class AddAdditionalColumnsToAuditLog < ActiveRecord::Migration[5.2] + def change + add_column(:audit_logs, :slug, :string) + add_column(:audit_logs, :category, :string) + end +end diff --git a/db/migrate/20190906193806_add_jsonb_data_column_to_audit_logs.rb b/db/migrate/20190906193806_add_jsonb_data_column_to_audit_logs.rb new file mode 100644 index 000000000..98dce50c4 --- /dev/null +++ b/db/migrate/20190906193806_add_jsonb_data_column_to_audit_logs.rb @@ -0,0 +1,6 @@ +class AddJsonbDataColumnToAuditLogs < ActiveRecord::Migration[5.2] + def change + add_column :audit_logs, :data, :jsonb, null: false, default: {} + add_index :audit_logs, :data, using: :gin + end +end diff --git a/db/schema.rb b/db/schema.rb index fef46ea16..519ecb218 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -145,6 +145,18 @@ ActiveRecord::Schema.define(version: 2019_09_18_104106) do t.index ["user_id"], name: "index_articles_on_user_id" end + create_table "audit_logs", force: :cascade do |t| + t.string "category" + t.datetime "created_at", null: false + t.jsonb "data", default: {}, null: false + t.string "roles", array: true + t.string "slug" + t.datetime "updated_at", null: false + t.bigint "user_id" + t.index ["data"], name: "index_audit_logs_on_data", using: :gin + t.index ["user_id"], name: "index_audit_logs_on_user_id" + end + create_table "backup_data", force: :cascade do |t| t.datetime "created_at", null: false t.bigint "instance_id", null: false @@ -1178,6 +1190,7 @@ ActiveRecord::Schema.define(version: 2019_09_18_104106) do t.index ["user_id"], name: "index_webhook_endpoints_on_user_id" end + add_foreign_key "audit_logs", "users" add_foreign_key "badge_achievements", "badges" add_foreign_key "badge_achievements", "users" add_foreign_key "chat_channel_memberships", "chat_channels" diff --git a/spec/factories/activesupport_events.rb b/spec/factories/activesupport_events.rb new file mode 100644 index 000000000..955db904a --- /dev/null +++ b/spec/factories/activesupport_events.rb @@ -0,0 +1,11 @@ +FactoryBot.define do + factory :activesupport_event, class: ActiveSupport::Notifications::Event do + name { "audit.log" } + time { Timecop.freeze(Time.zone.now) } + ending { time + 10.seconds } + transaction_id { Faker::Crypto.md5 } + payload { {} } + + initialize_with { new(name, time, ending, transaction_id, payload) } + end +end diff --git a/spec/factories/audit_logs.rb b/spec/factories/audit_logs.rb new file mode 100644 index 000000000..3143600ff --- /dev/null +++ b/spec/factories/audit_logs.rb @@ -0,0 +1,4 @@ +FactoryBot.define do + factory :audit_log do + end +end diff --git a/spec/models/audit_log_spec.rb b/spec/models/audit_log_spec.rb new file mode 100644 index 000000000..622ac3017 --- /dev/null +++ b/spec/models/audit_log_spec.rb @@ -0,0 +1,9 @@ +require "rails_helper" + +RSpec.describe AuditLog, type: :model do + let(:user) { create(:user) } + let(:audit_log) { create(:audit_log, user_id: user.id) } + + it { is_expected.to belong_to(:user) } + it { is_expected.to validate_presence_of(:user_id) } +end diff --git a/spec/requests/internal/moderator_logs_spec.rb b/spec/requests/internal/moderator_logs_spec.rb new file mode 100644 index 000000000..86985f8b3 --- /dev/null +++ b/spec/requests/internal/moderator_logs_spec.rb @@ -0,0 +1,40 @@ +require "rails_helper" + +RSpec.describe "/internal/tags", type: :request do + let(:super_admin) { create(:user, :super_admin) } + let(:tag_moderator) { create(:user) } + let!(:tag) { create(:tag) } + let(:listener) { :internal } + + before do + sign_in super_admin + Audit::Subscribe.listen listener + end + + after do + Audit::Subscribe.forget listener + end + + def update_params(tag_moderator_id) + { + tag: { + tag_moderator_id: tag_moderator_id + } + } + end + + describe "POST /internal/tag/:id" do + it "creates entry for #update action" do + allow(AssignTagModerator).to receive(:add_tag_moderators) + + perform_enqueued_jobs do + put "/internal/tags/#{tag.id}", params: update_params(tag_moderator.id.to_s) + log = AuditLog.where(user_id: super_admin.id, slug: :update) + expected = update_params(tag_moderator.id.to_s)[:tag] + + expect(log.first.data.symbolize_keys).to eq expected + expect(log.count).to eq(1) + end + end + end +end diff --git a/spec/services/audit/event/util_spec.rb b/spec/services/audit/event/util_spec.rb new file mode 100644 index 000000000..1175bb554 --- /dev/null +++ b/spec/services/audit/event/util_spec.rb @@ -0,0 +1,16 @@ +require "rails_helper" + +RSpec.describe Audit::Event::Util, type: :service do + let(:utils) { described_class } + let!(:event) { build(:activesupport_event) } + + describe "Serialization" do + it "evaluates to same object" do + compare_to = utils.deserialize(utils.serialize(event)) + + expect(event.class).to eq(compare_to.class) + expect(event.time.iso8601.in_time_zone).to eq(compare_to.time.iso8601) + expect(event.end.iso8601.in_time_zone).to eq(compare_to.end.iso8601) + end + end +end diff --git a/spec/services/audit/helper_spec.rb b/spec/services/audit/helper_spec.rb new file mode 100644 index 000000000..3a4bdc732 --- /dev/null +++ b/spec/services/audit/helper_spec.rb @@ -0,0 +1,7 @@ +require "rails_helper" + +RSpec.describe Audit::Helper, type: :service do + it "has a notification suffix" do + expect(Audit::Helper::NOTIFICATION_SUFFIX).not_to be nil + end +end diff --git a/spec/services/audit/notification_spec.rb b/spec/services/audit/notification_spec.rb new file mode 100644 index 000000000..8a0afe65b --- /dev/null +++ b/spec/services/audit/notification_spec.rb @@ -0,0 +1,62 @@ +require "rails_helper" + +RSpec.describe Audit::Notification, type: :service do + let!(:listener) { Faker::Alphanumeric.alpha(10) } + let(:user) { build(:user, :admin) } + let(:queue_name) { Audit::SaveToPersistentStorageJob.queue_name } + let(:job_class) { Audit::SaveToPersistentStorageJob } + + before do + Audit::Subscribe.listen listener + end + + after do + Audit::Subscribe.forget listener + end + + def notify + described_class.notify(listener) do |payload| + payload.user_id = user.id + payload.roles = user.roles.pluck(:name) + end + end + + describe "Publishing and receiving events" do + context "when payload is missing" do + it "event is not created" do + allow(described_class).to receive(:listen) + described_class.notify(listener) + + expect(described_class).not_to have_received(:listen) + end + end + + context "when payload is present" do + it "receives an event" do + allow(described_class).to receive(:listen) + notify + + expect(described_class).to have_received(:listen) + end + end + end + + describe "Queueing job for saving an event" do + it "can enqueue job on dedicated queue name" do + expect { notify }.to have_enqueued_job(job_class).on_queue(queue_name.to_s) + end + end + + describe "Saving to database" do + it "creates an AuditLog record" do + user.save + perform_enqueued_jobs do + notify + end + + event_record = AuditLog.find_by(user_id: user.id) + expect(event_record.user).to eq(user) + expect(event_record.roles).to eq(user.roles.pluck(:name)) + end + end +end diff --git a/spec/services/audit/subscribe_spec.rb b/spec/services/audit/subscribe_spec.rb new file mode 100644 index 000000000..b17bea896 --- /dev/null +++ b/spec/services/audit/subscribe_spec.rb @@ -0,0 +1,21 @@ +require "rails_helper" + +RSpec.describe Audit::Subscribe, type: :service do + let(:notifications) { ActiveSupport::Notifications } + let(:listeners) { %i[moderator visitor] } + let(:listener_suffix) { Audit::Helper::NOTIFICATION_SUFFIX } + + before do + listeners.each do |listener| + allow(notifications).to receive(:subscribe).with([listener, listener_suffix].join) + end + end + + it "can subscribe to custom listeners" do + described_class.listen(*listeners) + + listeners.each do |listener| + expect(notifications).to have_received(:subscribe).with([listener, listener_suffix].join) + end + end +end