diff options
author | Eugen Rochko <eugen@zeonfederated.com> | 2022-08-25 20:39:40 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-08-25 20:39:40 +0200 |
commit | 0396acf39ea902688374fac65fa7ef5dc4c05512 (patch) | |
tree | f5349303e930976f8accd22453dd4bc5024c32f5 | |
parent | 99aed9069d4319d53779c483142e6649f3fef17b (diff) |
Add audit log entries for user roles (#19040)
* Refactor audit log schema * Add audit log entries for user roles
27 files changed, 151 insertions, 99 deletions
diff --git a/app/controllers/admin/roles_controller.rb b/app/controllers/admin/roles_controller.rb index 3e502ccc4..d76aa745b 100644 --- a/app/controllers/admin/roles_controller.rb +++ b/app/controllers/admin/roles_controller.rb @@ -23,6 +23,7 @@ module Admin @role.current_account = current_account if @role.save + log_action :create, @role redirect_to admin_roles_path else render :new @@ -39,6 +40,7 @@ module Admin @role.current_account = current_account if @role.update(resource_params) + log_action :update, @role redirect_to admin_roles_path else render :edit @@ -48,6 +50,7 @@ module Admin def destroy authorize @role, :destroy? @role.destroy! + log_action :destroy, @role redirect_to admin_roles_path end diff --git a/app/controllers/admin/users/roles_controller.rb b/app/controllers/admin/users/roles_controller.rb index 0db50cee9..f5dfc643d 100644 --- a/app/controllers/admin/users/roles_controller.rb +++ b/app/controllers/admin/users/roles_controller.rb @@ -14,6 +14,7 @@ module Admin @user.current_account = current_account if @user.update(resource_params) + log_action :change_role, @user redirect_to admin_account_path(@user.account_id), notice: I18n.t('admin.accounts.change_role.changed_msg') else render :show diff --git a/app/controllers/concerns/accountable_concern.rb b/app/controllers/concerns/accountable_concern.rb index 87d62478d..c1349915f 100644 --- a/app/controllers/concerns/accountable_concern.rb +++ b/app/controllers/concerns/accountable_concern.rb @@ -3,7 +3,11 @@ module AccountableConcern extend ActiveSupport::Concern - def log_action(action, target, options = {}) - Admin::ActionLog.create(account: current_account, action: action, target: target, recorded_changes: options.stringify_keys) + def log_action(action, target) + Admin::ActionLog.create( + account: current_account, + action: action, + target: target + ) end end diff --git a/app/helpers/admin/action_logs_helper.rb b/app/helpers/admin/action_logs_helper.rb index 47eeeaac3..3e9fe17f4 100644 --- a/app/helpers/admin/action_logs_helper.rb +++ b/app/helpers/admin/action_logs_helper.rb @@ -2,64 +2,31 @@ module Admin::ActionLogsHelper def log_target(log) - if log.target - linkable_log_target(log.target) - else - log_target_from_history(log.target_type, log.recorded_changes) - end - end - - private - - def linkable_log_target(record) - case record.class.name + case log.target_type when 'Account' - link_to record.acct, admin_account_path(record.id) + link_to log.human_identifier, admin_account_path(log.target_id) when 'User' - link_to record.account.acct, admin_account_path(record.account_id) + link_to log.human_identifier, admin_account_path(log.route_param) + when 'UserRole' + link_to log.human_identifier, admin_roles_path(log.target_id) when 'CustomEmoji' - record.shortcode + log.human_identifier when 'Report' - link_to "##{record.id}", admin_report_path(record) + link_to "##{log.human_identifier}", admin_report_path(log.target_id) when 'DomainBlock', 'DomainAllow', 'EmailDomainBlock', 'UnavailableDomain' - link_to record.domain, "https://#{record.domain}" + link_to log.human_identifier, "https://#{log.human_identifier}" when 'Status' - link_to record.account.acct, ActivityPub::TagManager.instance.url_for(record) + link_to log.human_identifier, log.permalink when 'AccountWarning' - link_to record.target_account.acct, admin_account_path(record.target_account_id) + link_to log.human_identifier, admin_account_path(log.target_id) when 'Announcement' - link_to truncate(record.text), edit_admin_announcement_path(record.id) + link_to truncate(log.human_identifier), edit_admin_announcement_path(log.target_id) when 'IpBlock' - "#{record.ip}/#{record.ip.prefix} (#{I18n.t("simple_form.labels.ip_block.severities.#{record.severity}")})" + log.human_identifier when 'Instance' - record.domain + log.human_identifier when 'Appeal' - link_to record.account.acct, disputes_strike_path(record.strike) - end - end - - def log_target_from_history(type, attributes) - case type - when 'User' - attributes['username'] - when 'CustomEmoji' - attributes['shortcode'] - when 'DomainBlock', 'DomainAllow', 'EmailDomainBlock', 'UnavailableDomain' - link_to attributes['domain'], "https://#{attributes['domain']}" - when 'Status' - tmp_status = Status.new(attributes.except('reblogs_count', 'favourites_count')) - - if tmp_status.account - link_to tmp_status.account&.acct || "##{tmp_status.account_id}", admin_account_path(tmp_status.account_id) - else - I18n.t('admin.action_logs.deleted_status') - end - when 'Announcement' - truncate(attributes['text'].is_a?(Array) ? attributes['text'].last : attributes['text']) - when 'IpBlock' - "#{attributes['ip']}/#{attributes['ip'].prefix} (#{I18n.t("simple_form.labels.ip_block.severities.#{attributes['severity']}")})" - when 'Instance' - attributes['domain'] + link_to log.human_identifier, disputes_strike_path(log.route_param) end end end diff --git a/app/models/account.rb b/app/models/account.rb index 628692d22..d25afeb89 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -362,6 +362,10 @@ class Account < ApplicationRecord username end + def to_log_human_identifier + acct + end + def excluded_from_timeline_account_ids Rails.cache.fetch("exclude_account_ids_for:#{id}") { block_relationships.pluck(:target_account_id) + blocked_by_relationships.pluck(:account_id) + mute_relationships.pluck(:target_account_id) } end diff --git a/app/models/account_warning.rb b/app/models/account_warning.rb index 6067b54b7..961a078b9 100644 --- a/app/models/account_warning.rb +++ b/app/models/account_warning.rb @@ -43,4 +43,8 @@ class AccountWarning < ApplicationRecord def overruled? overruled_at.present? end + + def to_log_human_identifier + target_account.acct + end end diff --git a/app/models/admin/action_log.rb b/app/models/admin/action_log.rb index 401bfd9ac..4fa8008f5 100644 --- a/app/models/admin/action_log.rb +++ b/app/models/admin/action_log.rb @@ -9,38 +9,42 @@ # action :string default(""), not null # target_type :string # target_id :bigint(8) -# recorded_changes :text default(""), not null # created_at :datetime not null # updated_at :datetime not null +# human_identifier :string +# route_param :string +# permalink :string # class Admin::ActionLog < ApplicationRecord - serialize :recorded_changes + self.ignored_columns = %w( + recorded_changes + ) belongs_to :account belongs_to :target, polymorphic: true, optional: true default_scope -> { order('id desc') } + before_validation :set_human_identifier + before_validation :set_route_param + before_validation :set_permalink + def action super.to_sym end - before_validation :set_changes - private - def set_changes - case action - when :destroy, :create - self.recorded_changes = target.attributes - when :update, :promote, :demote - self.recorded_changes = target.previous_changes - when :change_email - self.recorded_changes = ActiveSupport::HashWithIndifferentAccess.new( - email: [target.email, nil], - unconfirmed_email: [nil, target.unconfirmed_email] - ) - end + def set_human_identifier + self.human_identifier = target.to_log_human_identifier if target.respond_to?(:to_log_human_identifier) + end + + def set_route_param + self.route_param = target.to_log_route_param if target.respond_to?(:to_log_route_param) + end + + def set_permalink + self.permalink = target.to_log_permalink if target.respond_to?(:to_log_permalink) end end diff --git a/app/models/admin/action_log_filter.rb b/app/models/admin/action_log_filter.rb index 0f2f712a2..6382cd782 100644 --- a/app/models/admin/action_log_filter.rb +++ b/app/models/admin/action_log_filter.rb @@ -12,6 +12,7 @@ class Admin::ActionLogFilter reject_appeal: { target_type: 'Appeal', action: 'reject' }.freeze, assigned_to_self_report: { target_type: 'Report', action: 'assigned_to_self' }.freeze, change_email_user: { target_type: 'User', action: 'change_email' }.freeze, + change_role_user: { target_type: 'User', action: 'change_role' }.freeze, confirm_user: { target_type: 'User', action: 'confirm' }.freeze, approve_user: { target_type: 'User', action: 'approve' }.freeze, reject_user: { target_type: 'User', action: 'reject' }.freeze, @@ -22,6 +23,7 @@ class Admin::ActionLogFilter create_domain_block: { target_type: 'DomainBlock', action: 'create' }.freeze, create_email_domain_block: { target_type: 'EmailDomainBlock', action: 'create' }.freeze, create_unavailable_domain: { target_type: 'UnavailableDomain', action: 'create' }.freeze, + create_user_role: { target_type: 'UserRole', action: 'create' }.freeze, demote_user: { target_type: 'User', action: 'demote' }.freeze, destroy_announcement: { target_type: 'Announcement', action: 'destroy' }.freeze, destroy_custom_emoji: { target_type: 'CustomEmoji', action: 'destroy' }.freeze, @@ -31,6 +33,7 @@ class Admin::ActionLogFilter destroy_instance: { target_type: 'Instance', action: 'destroy' }.freeze, destroy_unavailable_domain: { target_type: 'UnavailableDomain', action: 'destroy' }.freeze, destroy_status: { target_type: 'Status', action: 'destroy' }.freeze, + destroy_user_role: { target_type: 'UserRole', action: 'destroy' }.freeze, disable_2fa_user: { target_type: 'User', action: 'disable' }.freeze, disable_custom_emoji: { target_type: 'CustomEmoji', action: 'disable' }.freeze, disable_user: { target_type: 'User', action: 'disable' }.freeze, @@ -52,6 +55,7 @@ class Admin::ActionLogFilter update_announcement: { target_type: 'Announcement', action: 'update' }.freeze, update_custom_emoji: { target_type: 'CustomEmoji', action: 'update' }.freeze, update_status: { target_type: 'Status', action: 'update' }.freeze, + update_user_role: { target_type: 'UserRole', action: 'update' }.freeze, unblock_email_account: { target_type: 'Account', action: 'unblock_email' }.freeze, }.freeze diff --git a/app/models/announcement.rb b/app/models/announcement.rb index f8183aabc..bedced9de 100644 --- a/app/models/announcement.rb +++ b/app/models/announcement.rb @@ -34,6 +34,10 @@ class Announcement < ApplicationRecord before_validation :set_all_day before_validation :set_published, on: :create + def to_log_human_identifier + text + end + def publish! update!(published: true, published_at: Time.now.utc, scheduled_at: nil) end diff --git a/app/models/appeal.rb b/app/models/appeal.rb index 1f32cfa8b..6fbf60b39 100644 --- a/app/models/appeal.rb +++ b/app/models/appeal.rb @@ -52,6 +52,14 @@ class Appeal < ApplicationRecord update!(rejected_at: Time.now.utc, rejected_by_account: current_account) end + def to_log_human_identifier + account.acct + end + + def to_log_route_param + account_warning_id + end + private def validate_time_frame diff --git a/app/models/custom_emoji.rb b/app/models/custom_emoji.rb index 289e3b66f..077ce559a 100644 --- a/app/models/custom_emoji.rb +++ b/app/models/custom_emoji.rb @@ -46,7 +46,7 @@ class CustomEmoji < ApplicationRecord scope :local, -> { where(domain: nil) } scope :remote, -> { where.not(domain: nil) } scope :alphabetic, -> { order(domain: :asc, shortcode: :asc) } - scope :by_domain_and_subdomains, ->(domain) { where(domain: domain).or(where(arel_table[:domain].matches('%.' + domain))) } + scope :by_domain_and_subdomains, ->(domain) { where(domain: domain).or(where(arel_table[:domain].matches("%.#{domain}"))) } scope :listed, -> { local.where(disabled: false).where(visible_in_picker: true) } remotable_attachment :image, LIMIT @@ -67,6 +67,10 @@ class CustomEmoji < ApplicationRecord copy.tap(&:save!) end + def to_log_human_identifier + shortcode + end + class << self def from_text(text, domain = nil) return [] if text.blank? diff --git a/app/models/domain_allow.rb b/app/models/domain_allow.rb index 6aa9267fe..65f494fed 100644 --- a/app/models/domain_allow.rb +++ b/app/models/domain_allow.rb @@ -19,6 +19,10 @@ class DomainAllow < ApplicationRecord scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } + def to_log_human_identifier + domain + end + class << self def allowed?(domain) !rule_for(domain).nil? diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb index a15206b5e..b08687787 100644 --- a/app/models/domain_block.rb +++ b/app/models/domain_block.rb @@ -31,6 +31,10 @@ class DomainBlock < ApplicationRecord scope :with_user_facing_limitations, -> { where(severity: [:silence, :suspend]).or(where(reject_media: true)) } scope :by_severity, -> { order(Arel.sql('(CASE severity WHEN 0 THEN 1 WHEN 1 THEN 2 WHEN 2 THEN 0 END), reject_media, domain')) } + def to_log_human_identifier + domain + end + def policies if suspend? [:suspend] diff --git a/app/models/email_domain_block.rb b/app/models/email_domain_block.rb index f9d74332b..661f6727d 100644 --- a/app/models/email_domain_block.rb +++ b/app/models/email_domain_block.rb @@ -26,6 +26,10 @@ class EmailDomainBlock < ApplicationRecord # Used for adding multiple blocks at once attr_accessor :other_domains + def to_log_human_identifier + domain + end + def history @history ||= Trends::History.new('email_domain_blocks', id) end diff --git a/app/models/form/account_batch.rb b/app/models/form/account_batch.rb index dcf155840..98f2cad3e 100644 --- a/app/models/form/account_batch.rb +++ b/app/models/form/account_batch.rb @@ -101,7 +101,7 @@ class Form::AccountBatch def reject_account(account) authorize(account.user, :reject?) - log_action(:reject, account.user, username: account.username) + log_action(:reject, account.user) account.suspend!(origin: :local) AccountDeletionWorker.perform_async(account.id, { 'reserve_username' => false }) end diff --git a/app/models/instance.rb b/app/models/instance.rb index 36110ee40..edbf02a6d 100644 --- a/app/models/instance.rb +++ b/app/models/instance.rb @@ -48,6 +48,8 @@ class Instance < ApplicationRecord domain end + alias to_log_human_identifier to_param + delegate :exhausted_deliveries_days, to: :delivery_failure_tracker def availability_over_days(num_days, end_date = Time.now.utc.to_date) diff --git a/app/models/ip_block.rb b/app/models/ip_block.rb index e1ab59806..f40c8a0b1 100644 --- a/app/models/ip_block.rb +++ b/app/models/ip_block.rb @@ -27,6 +27,10 @@ class IpBlock < ApplicationRecord after_commit :reset_cache + def to_log_human_identifier + "#{record.ip}/#{record.ip.prefix}" + end + class << self def blocked?(remote_ip) blocked_ips_map = Rails.cache.fetch(CACHE_KEY) { FastIpMap.new(IpBlock.where(severity: :no_access).pluck(:ip)) } diff --git a/app/models/report.rb b/app/models/report.rb index 2efb6d4a7..42c869dd4 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -115,6 +115,10 @@ class Report < ApplicationRecord Report.where.not(id: id).where(target_account_id: target_account_id).unresolved.exists? end + def to_log_human_identifier + id + end + def history subquery = [ Admin::ActionLog.where( @@ -136,6 +140,8 @@ class Report < ApplicationRecord Admin::ActionLog.from(Arel::Nodes::As.new(subquery, Admin::ActionLog.arel_table)) end + private + def set_uri self.uri = ActivityPub::TagManager.instance.generate_uri_for(self) if uri.nil? && account.local? end diff --git a/app/models/status.rb b/app/models/status.rb index 4828d6340..7eff990aa 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -166,6 +166,14 @@ class Status < ApplicationRecord ].compact.join("\n\n") end + def to_log_human_identifier + account.acct + end + + def to_log_permalink + ActivityPub::TagManager.instance.uri_for(self) + end + def reply? !in_reply_to_id.nil? || attributes['reply'] end diff --git a/app/models/unavailable_domain.rb b/app/models/unavailable_domain.rb index 5e8870bde..dfc0ef14e 100644 --- a/app/models/unavailable_domain.rb +++ b/app/models/unavailable_domain.rb @@ -16,6 +16,10 @@ class UnavailableDomain < ApplicationRecord after_commit :reset_cache! + def to_log_human_identifier + domain + end + private def reset_cache! diff --git a/app/models/user.rb b/app/models/user.rb index 9833300cd..18b9d5928 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -181,6 +181,14 @@ class User < ApplicationRecord update!(disabled: false) end + def to_log_human_identifier + account.acct + end + + def to_log_route_param + account_id + end + def confirm new_user = !confirmed? self.approved = true if open_registrations? && !sign_up_from_ip_requires_approval? diff --git a/app/models/user_role.rb b/app/models/user_role.rb index 57a56c0b0..74dfdc220 100644 --- a/app/models/user_role.rb +++ b/app/models/user_role.rb @@ -155,6 +155,10 @@ class UserRole < ApplicationRecord end end + def to_log_human_identifier + name + end + private def in_permissions?(privilege) diff --git a/config/locales/en.yml b/config/locales/en.yml index e495ef841..5c309ab11 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -235,6 +235,7 @@ en: approve_user: Approve User assigned_to_self_report: Assign Report change_email_user: Change E-mail for User + change_role_user: Change Role of User confirm_user: Confirm User create_account_warning: Create Warning create_announcement: Create Announcement @@ -244,6 +245,7 @@ en: create_email_domain_block: Create E-mail Domain Block create_ip_block: Create IP rule create_unavailable_domain: Create Unavailable Domain + create_user_role: Create Role demote_user: Demote User destroy_announcement: Delete Announcement destroy_custom_emoji: Delete Custom Emoji @@ -254,6 +256,7 @@ en: destroy_ip_block: Delete IP rule destroy_status: Delete Post destroy_unavailable_domain: Delete Unavailable Domain + destroy_user_role: Destroy Role disable_2fa_user: Disable 2FA disable_custom_emoji: Disable Custom Emoji disable_sign_in_token_auth_user: Disable E-mail Token Authentication for User @@ -281,11 +284,13 @@ en: update_custom_emoji: Update Custom Emoji update_domain_block: Update Domain Block update_status: Update Post + update_user_role: Update Role actions: approve_appeal_html: "%{name} approved moderation decision appeal from %{target}" approve_user_html: "%{name} approved sign-up from %{target}" assigned_to_self_report_html: "%{name} assigned report %{target} to themselves" change_email_user_html: "%{name} changed the e-mail address of user %{target}" + change_role_user_html: "%{name} changed role of %{target}" confirm_user_html: "%{name} confirmed e-mail address of user %{target}" create_account_warning_html: "%{name} sent a warning to %{target}" create_announcement_html: "%{name} created new announcement %{target}" @@ -295,9 +300,10 @@ en: create_email_domain_block_html: "%{name} blocked e-mail domain %{target}" create_ip_block_html: "%{name} created rule for IP %{target}" create_unavailable_domain_html: "%{name} stopped delivery to domain %{target}" + create_user_role_html: "%{name} created %{target} role" demote_user_html: "%{name} demoted user %{target}" destroy_announcement_html: "%{name} deleted announcement %{target}" - destroy_custom_emoji_html: "%{name} destroyed emoji %{target}" + destroy_custom_emoji_html: "%{name} deleted emoji %{target}" destroy_domain_allow_html: "%{name} disallowed federation with domain %{target}" destroy_domain_block_html: "%{name} unblocked domain %{target}" destroy_email_domain_block_html: "%{name} unblocked e-mail domain %{target}" @@ -305,6 +311,7 @@ en: destroy_ip_block_html: "%{name} deleted rule for IP %{target}" destroy_status_html: "%{name} removed post by %{target}" destroy_unavailable_domain_html: "%{name} resumed delivery to domain %{target}" + destroy_user_role_html: "%{name} deleted %{target} role" disable_2fa_user_html: "%{name} disabled two factor requirement for user %{target}" disable_custom_emoji_html: "%{name} disabled emoji %{target}" disable_sign_in_token_auth_user_html: "%{name} disabled e-mail token authentication for %{target}" @@ -332,7 +339,7 @@ en: update_custom_emoji_html: "%{name} updated emoji %{target}" update_domain_block_html: "%{name} updated domain block for %{target}" update_status_html: "%{name} updated post by %{target}" - deleted_status: "(deleted post)" + update_user_role_html: "%{name} changed %{target} role" empty: No logs found. filter_by_action: Filter by action filter_by_user: Filter by user diff --git a/db/migrate/20220824164433_add_human_identifier_to_admin_action_logs.rb b/db/migrate/20220824164433_add_human_identifier_to_admin_action_logs.rb new file mode 100644 index 000000000..2cb8cddf1 --- /dev/null +++ b/db/migrate/20220824164433_add_human_identifier_to_admin_action_logs.rb @@ -0,0 +1,7 @@ +class AddHumanIdentifierToAdminActionLogs < ActiveRecord::Migration[6.1] + def change + add_column :admin_action_logs, :human_identifier, :string + add_column :admin_action_logs, :route_param, :string + add_column :admin_action_logs, :permalink, :string + end +end diff --git a/db/post_migrate/20220824164532_remove_recorded_changes_from_admin_action_logs.rb b/db/post_migrate/20220824164532_remove_recorded_changes_from_admin_action_logs.rb new file mode 100644 index 000000000..c42d5df8f --- /dev/null +++ b/db/post_migrate/20220824164532_remove_recorded_changes_from_admin_action_logs.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class RemoveRecordedChangesFromAdminActionLogs < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + safety_assured { remove_column :admin_action_logs, :recorded_changes, :text } + end +end diff --git a/db/schema.rb b/db/schema.rb index 15ab2e85e..83fd9549c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_08_08_101323) do +ActiveRecord::Schema.define(version: 2022_08_24_164532) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -205,9 +205,11 @@ ActiveRecord::Schema.define(version: 2022_08_08_101323) do t.string "action", default: "", null: false t.string "target_type" t.bigint "target_id" - t.text "recorded_changes", default: "", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "human_identifier" + t.string "route_param" + t.string "permalink" t.index ["account_id"], name: "index_admin_action_logs_on_account_id" t.index ["target_type", "target_id"], name: "index_admin_action_logs_on_target_type_and_target_id" end diff --git a/spec/helpers/admin/action_log_helper_spec.rb b/spec/helpers/admin/action_log_helper_spec.rb index 60f5ecdcc..9d7ed4ab7 100644 --- a/spec/helpers/admin/action_log_helper_spec.rb +++ b/spec/helpers/admin/action_log_helper_spec.rb @@ -3,32 +3,4 @@ require 'rails_helper' RSpec.describe Admin::ActionLogsHelper, type: :helper do - klass = Class.new do - include ActionView::Helpers - include Admin::ActionLogsHelper - end - - let(:hoge) { klass.new } - - describe '#log_target' do - after do - hoge.log_target(log) - end - - context 'log.target' do - let(:log) { double(target: true) } - - it 'calls linkable_log_target' do - expect(hoge).to receive(:linkable_log_target).with(log.target) - end - end - - context '!log.target' do - let(:log) { double(target: false, target_type: :type, recorded_changes: :change) } - - it 'calls log_target_from_history' do - expect(hoge).to receive(:log_target_from_history).with(log.target_type, log.recorded_changes) - end - end - end end |