flarum / issue-archive

0 stars 0 forks source link

Adjustment to suspended users #48

Open katosdev opened 2 years ago

katosdev commented 2 years ago

Feature Request

Is your feature request related to a problem? Please describe. At the moment, when a user is suspended they can still perform actions on the forum such as changing their avatar. This could potentially lead to avatars or signatures that are derogatory in nature or otherwise break the community rules.

Describe the solution you'd like I propose that;

This approach would allow extensibility such as allowing users to post to help tags or byobu, so that they can chat with the team or dispute a ban without it spilling out to the forums. I’d further propose: removing the user avatar, and denying the ability to set a new one (obviously adjustable by permissions)

davwheat commented 2 years ago

so that they can chat with the team or dispute a ban without it spilling out to the forums

Seems like a good solution for flarum/issue-archive#52!

davwheat commented 2 years ago

Looks like we already track the Avatar issue under flarum/issue-archive#214.

I'm in favour of using this issue instead as it's a more broader fix.

askvortsov1 commented 2 years ago

Special groups are challenging, because we don't have any more reserved IDs. We would either have to use a random ID per installation, which could be hard to work with, or move around existing IDs, which is very very messy.

Since suspend is essentially "read only mode", I think we could hook into the activation / deactivation system proposed in another recent issue, where suspended users would face the same restrictions as users without activation.

davwheat commented 2 years ago

Could we use negative numbers as the IDs?

SychO9 commented 2 years ago

Special groups are challenging, because we don't have any more reserved IDs.

That's unfortunate. Also interestingly enough, we set the guest group to suspended users atm, which I don't know what it really does.

There is still another approach we could take for this, we could have a setting in suspend, to choose a group to auto assign/unassign when a user gets suspended/unsuspended, I believe that would work well enough for this purpose.

katosdev commented 2 years ago

There is still another approach we could take for this, we could have a setting in suspend, to choose a group to auto assign/unassign when a user gets suspended/unsuspended, I believe that would work well enough for this purpose.

I like this approach. I feel like simply having them as unauthenticated leaves us open to the issues of them being able to change avatars etc, which is where we're having issues with derogatory / troll display pictures.

matteocontrini commented 2 years ago

Isn't https://github.com/flarum/suspend/pull/30 already there to fix this?

askvortsov1 commented 2 years ago

There is still another approach we could take for this, we could have a setting in suspend, to choose a group to auto assign/unassign when a user gets suspended/unsuspended, I believe that would work well enough for this purpose.

I like this approach. I feel like simply having them as unauthenticated leaves us open to the issues of them being able to change avatars etc, which is where we're having issues with derogatory / troll display pictures.

There's 2 challenges with a group based approach:

  1. We don't currently have negative permissions. That's probably why guest is used for suspend (btw, guests can't change avatar or perform any other write/update/delete operations). But introducing a new group for suspended wouldn't work as elegantly as we think, because there'd be no way to give a permission to all members except the group.
  2. Auto assigning / unassigning groups feels like something that should be automated through Automoderator. There'll be a lot of redundant logic for the UI and logic.
SychO9 commented 2 years ago

But introducing a new group for suspended wouldn't work as elegantly as we think, because there'd be no way to give a permission to all members except the group.

Good point, with the current system, a special suspend group would actually make no difference than how it currently assigns the Guest group.

Suspend currently completely overwrites all user groups and only leaves the guest group, the only reason a suspended user can still change avatar, is because there is no avatar permission in place. I'm sceptical about the need to disallow actions specifically for suspended users at all, or if we do add such specific checks it should check for guest group instead.

However, if every action has a permission, like the avatar change, then suspended users won't be able to edit that at all, and that might be a better solution, simply adding permissions to said actions.

https://github.com/flarum/suspend/blob/5cfb471aa5eb48becfa13f09f9049e3b780d2234/src/RevokeAccessFromSuspendedUsers.php#L22-L32

Coming back to this

I think we could hook into the activation / deactivation system proposed in another recent issue, where suspended users would face the same restrictions as users without activation.

Suspended users already face the same restrictions though, both non activated users and suspended are only in the guest group.

https://github.com/flarum/core/blob/b8754c7d7db07bbea25d15e285da9dadeece72ce/src/User/User.php#L717-L727

askvortsov1 commented 2 years ago

Suspended users already face the same restrictions though, both non activated users and suspended are only in the guest group.

In this case, with regards to restricting avatars, we should assert that users are not guests before allowing them to upload an avatar.