Core: [Suspend] Suspended users can edit/delete own posts

[jordanjay29]

There's not much needed to recreate this, suspend a test user with a post or two and then attempt to edit or delete the past posts as that user.

This appears to be unintentional, but presents some real problems for security and moderation. It's possible for a user to erase evidence of why they were suspended, to openly dispute their suspension, or use it to retaliate by editing/removing critical posts.

[luceos]

Adding to this, should we also prevent edits when discussions are locked or soft deleted?

[Ralkage]

@luceos most definitely, this is something that is core with most if not all popular forum software. When a discussion is locked or deleted, no posts should be allowed to be edited by a user (or even staff at that given it's a configurable permission) as this helps prevent cases where users try to cover their tracks when being punished or questioned.

[pflstr]

I think, this should be handled by the permissions setting.

Right now, we have general permissions like Allow post editing and Allow renaming which can be extended (overridden) for certain user groups (e.g. admins and mods), Edit and delete posts and Rename discussions. Why shouldn't we have a default user group Suspended user and overriding settings that limit the ability to edit posts or rename discussions? In this case, each forum admin could decide what suspended users are allowed to do.

[jordanjay29]

I like pflstr's idea a lot.

[pflstr]

default user group should probably be better called built in user group, a group that is automatically assigned to suspended users. One might even differ between temporarily and permanently suspended users.

[tobyzerner]

Not sure there is a need for suspended users to have a user group - what permissions would you ever want to override? The intention is that suspended users should have the permissions of a guest.

[pflstr]

@tobscure Not sure there is a need for suspended users to have a user group - what permissions would you ever want to override? The intention is that suspended users should have the permissions of a guest.

One might for example allow suspended users to have private conversations with moderators.

[tobyzerner]

That's a good point.

In that case, @sijad, the approach we're taking in #17 is not going to work, because it doesn't allow the user to be identified in subsequent policy checks. Let's leave the suspend logic as-is, and add in new logic in core/PostPolicy::edit, core/DiscussionPolicy::rename, and tags/DiscussionPolicy::tag which ensures that the user is actually allowed to perform these actions.

In order to do this I'm thinking we'll need to introduce some new permissions: discussion.own.editPosts, discussion.own.rename, and discussion.own.tag. These do not need to be exposed in the admin UI for now but they need to be set by default in the database so that they are granted to the Member group.

Then we should add a blanket check for these discussion.own.* permissions in DiscussionPolicy::can:

        if ($actor->id === $discussion->start_user_id && $actor->hasPermission('discussion.own.'.$ability)) {
            return true;
        }

Finally, the specific policy methods will act to revoke the permission rather than grant it. So for example, DiscussionPolicy::rename:

    public function rename(User $actor, Discussion $discussion)
    {
        if ($discussion->start_user_id == $actor->id) {
            $allowRenaming = $this->settings->get('allow_renaming');

            if (($allowRenaming === 'reply' && $discussion->participants_count > 1)
                || ($allowRenaming !== '-1' && $discussion->start_time->diffInMinutes() > $allowRenaming) {
                return false;
            }
        }
    }

This will solve our problem with suspend because suspended users will not be granted these discussion.own.* permissions, because they have permissions equivalent to a guest.

After this groundwork is done, then we can consider adding a new group for suspended users to grant them custom permissions.