Add logging of admin actions #5757

Gargron · 2017-11-19T19:27:16Z

Notes:

For updates, promotes and demotes, it stores changed attributes in the log entry
For creates and destroys, it stores all of the record's attributes in the log entry

nightpool · 2017-11-19T20:01:56Z

these should show up when viewing the page for the account that an action is taken on, probably?

joshuap · 2017-11-19T21:41:21Z

How to store info about record in the log after it's deleted?

We've solved this by copying the table, i.e. if the table is emoji, a duplicate deleted_emoji table would be created. The target in the admin_action_logs would then reference the DeletedEmoji model rather than Emoji. DeletedEmoji may contain a subset of attributes (e.g., only the ones needed to display the deleted record in logs).

Store some extra data e.g. from which to which role user is promoted/demoted?

For this, maybe add a metadata json/hstore column to admin_action_logs? Logfmt comes to mind, which represents logs using simple key-value pairs. User promotion would look something like:

action=promote account=admin target=@alice role=admin from_role=user

Representing logs like that can make them easier to export to other systems, too.

Gargron · 2017-11-19T22:31:09Z

@joshuap I've looked up how acts_as_auditable does it - when a record is destroyed, it puts all of the records's attributes into a hstore. Although, especially with CustomEmoji, that might be kind of pointless since the image file is not stored alongside and therefore no recovery or display of that data would actually be useful.

If I don't do anything at all, the only thing I could do then is display "admin deleted CustomEmoji#345" which is kind of useless. So I think the middle ground is maybe, as you said, storing only attributes necessary for the barest minimum useful display, such as "admin deleted :cool_emoji:"

There's also acts_as_paranoid and similar approaches. I think that's gonna be useful for the statuses table, but probably not worth it for custom_emojis..? Separately from this PR, I want to make sure status deletions and account suspensions become reversible (with n days retention e.g. data is really deleted after 30 or 90 days instead of instantly)

joshuap · 2017-11-20T04:44:18Z

I'm not a huge fan of default_scope in acts_as_paranoid (or in general), but I've used/contributed to the paranoia gem which does the same thing. It also has an option to skip the default_scope in case you want to use a more explicit approach when hiding deleted records.

I'm curious, how large can the statuses table grow on an instance connected to most of the federated network?

Signed-off-by: Marcin Mikołajczak <me@m4sk.in>

…todon into feature-mod-accountability

nightpool · 2017-11-23T00:12:48Z

app/controllers/admin/reports_controller.rb

@@ -25,12 +25,15 @@ def update
    def process_report
      case params[:outcome].to_s
      when 'resolve'
-        @report.update(action_taken_by_current_attributes)
+        @report.update!(action_taken_by_current_attributes)
+        log_action :dismiss, @report


should probably be :resolve to match the existing terminology. (or the name of the button should be changed, which i think is probably wrong)

My concern is that that any action "resolves" a report. In this line, it's a resolve without any side effects like silence/suspend. It'd be weird if we had "resolves" logged for these but not for the silence/suspend versions. Therefore, I used a more clear terminology, "dismiss", i.e. "resolve without doing anything"

hmm. why aren't we logging both? I.e., suspending a user could be:

log_action :resolve, @report log_action :suspend, @user

otherwise you'll have no idea what report a given action was taken in response to

angristan · 2017-11-23T00:14:57Z

Wouldn't it be nice to have the possibility to make this page public or at least partially, allowing the users to have more transparency regarding admins/mods actions?
Just giving an idea, it's already a nice feature, so thanks for your work.

nightpool

Changes LGTM, but they should probably also include a Site Settings log type, right? I couldn't find logging for that anywhere.

* Update terminology (e-mail blacklist) imho looks better * Update en.yml

…todon into feature-mod-accountability

Gargron · 2017-11-23T23:24:17Z

The commit history of this PR is an awful mess because I wasn't able to rebase due to PRs made against this PR. :(

Addressed

nullkal

We need to run i18n-tasks normalize.

* i18n: (zh-CN) Add missing translations for multiple PRs. Related PRs: #5838 #5762 #5835 #5837 #5832 #5823 #5814 #5757 * i18n: (zh-CN) Fix translation for #5823 / #5835 * i18n: (zh-CN) Improve translations * i18n: (zh-CN) Improve translations * i18n: (zh-CN) Change `发送者` to `作者` * i18n: (zh-CN) Add missing translations for #5862 * i18n: (zh-CN) Add missing translation for #5874 * i18n: (zh-CN) Improve translations for keyboard shortcuts

* Add logging of admin actions * Update brakeman whitelist * Log creates, updates and destroys with history of changes * i18n: Update Polish translation (mastodon#5782) Signed-off-by: Marcin Mikołajczak <me@m4sk.in> * Split admin navigation into moderation and administration * Redesign audit log page * 🇵🇱 (mastodon#5795) * Add color coding to audit log * Change dismiss->resolve, log all outcomes of report as resolve * Update terminology (e-mail blacklist) (mastodon#5796) * Update terminology (e-mail blacklist) imho looks better * Update en.yml * Fix code style issues * i18n-tasks normalize

…on#5849) * i18n: (zh-CN) Add missing translations for multiple PRs. Related PRs: mastodon#5838 mastodon#5762 mastodon#5835 mastodon#5837 mastodon#5832 mastodon#5823 mastodon#5814 mastodon#5757 * i18n: (zh-CN) Fix translation for mastodon#5823 / mastodon#5835 * i18n: (zh-CN) Improve translations * i18n: (zh-CN) Improve translations * i18n: (zh-CN) Change `发送者` to `作者` * i18n: (zh-CN) Add missing translations for mastodon#5862 * i18n: (zh-CN) Add missing translation for mastodon#5874 * i18n: (zh-CN) Improve translations for keyboard shortcuts

Gargron added moderation Administration and moderation tooling work in progress Not to be merged, currently being worked on labels Nov 19, 2017

Gargron self-assigned this Nov 19, 2017

Add logging of admin actions

fe805b0

Gargron force-pushed the feature-mod-accountability branch from c402329 to fe805b0 Compare November 19, 2017 19:33

Update brakeman whitelist

2bef3ec

Gargron removed the work in progress Not to be merged, currently being worked on label Nov 22, 2017

Log creates, updates and destroys with history of changes

b5d5063

Gargron force-pushed the feature-mod-accountability branch from 0d2a299 to b5d5063 Compare November 22, 2017 01:39

Marcin Mikołajczak and others added 4 commits November 22, 2017 22:34

i18n: Update Polish translation (#5782)

013b604

Signed-off-by: Marcin Mikołajczak <me@m4sk.in>

Split admin navigation into moderation and administration

8ea0ded

Merge branch 'feature-mod-accountability' of github.com:tootsuite/mas…

81471ee

…todon into feature-mod-accountability

Redesign audit log page

1c8fcf7

Gargron force-pushed the feature-mod-accountability branch from 7d1ca74 to 1c8fcf7 Compare November 23, 2017 00:01

nightpool reviewed Nov 23, 2017

View reviewed changes

nightpool previously requested changes Nov 23, 2017

View reviewed changes

Marcin Mikołajczak and others added 7 commits November 23, 2017 21:55

🇵🇱 (#5795)

7696ffb

Add color coding to audit log

3ab42f1

Change dismiss->resolve, log all outcomes of report as resolve

7a6a44c

Update terminology (e-mail blacklist) (#5796)

3ca1722

* Update terminology (e-mail blacklist) imho looks better * Update en.yml

Fix code style issues

63f7e0c

Merge branch 'master' into feature-mod-accountability

4b71e1e

Merge branch 'feature-mod-accountability' of github.com:tootsuite/mas…

a351b26

…todon into feature-mod-accountability

nullkal reviewed Nov 24, 2017

View reviewed changes

i18n-tasks normalize

84315da

nullkal approved these changes Nov 24, 2017

View reviewed changes

Gargron merged commit e84fecb into master Nov 24, 2017

Gargron deleted the feature-mod-accountability branch November 24, 2017 01:05

mayaeh mentioned this pull request Nov 24, 2017

i18n: Update ja translations #5804

Merged

Gargron mentioned this pull request Nov 27, 2017

Bump version to 2.1.0rc1 #5834

Merged

10 tasks

SerCom-KC mentioned this pull request Nov 29, 2017

Update Chinese (Simplified) translations for version 2.1.0rc1 #5849

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add logging of admin actions #5757

Add logging of admin actions #5757

Gargron commented Nov 19, 2017 •

edited

nightpool commented Nov 19, 2017

joshuap commented Nov 19, 2017

Gargron commented Nov 19, 2017

joshuap commented Nov 20, 2017

nightpool Nov 23, 2017 •

edited

Gargron Nov 23, 2017

nightpool Nov 23, 2017

angristan commented Nov 23, 2017

nightpool left a comment

Gargron commented Nov 23, 2017

nullkal left a comment

Add logging of admin actions #5757

Add logging of admin actions #5757

Conversation

Gargron commented Nov 19, 2017 • edited

nightpool commented Nov 19, 2017

joshuap commented Nov 19, 2017

Gargron commented Nov 19, 2017

joshuap commented Nov 20, 2017

nightpool Nov 23, 2017 • edited

Choose a reason for hiding this comment

Gargron Nov 23, 2017

Choose a reason for hiding this comment

nightpool Nov 23, 2017

Choose a reason for hiding this comment

angristan commented Nov 23, 2017

nightpool left a comment

Choose a reason for hiding this comment

Gargron commented Nov 23, 2017

nullkal left a comment

Choose a reason for hiding this comment

Gargron commented Nov 19, 2017 •

edited

nightpool Nov 23, 2017 •

edited