Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suspended users shouldn't be allowed to change avatar #214

Closed
matteocontrini opened this issue Apr 24, 2020 · 13 comments
Closed

Suspended users shouldn't be allowed to change avatar #214

matteocontrini opened this issue Apr 24, 2020 · 13 comments

Comments

@matteocontrini
Copy link

Suspended users are currently allowed to change the avatar, potentially as a workaround for spreading spam or something else.

Suspended users shouldn't be allowed to do that.

@clarkwinkelmann
Copy link
Member

The problem is here

https://github.com/flarum/core/blob/b87c7189cc526cfc28e6eb207c8d19a2d8d14c06/src/User/Command/UploadAvatarHandler.php#L67-L69

We should create a new method on the user policy for editAvatar and move that logic there so it can be affected by other extensions.

That's actually very similar to the changes I recently made to the bio extension. It was previously hard-coded in the controller and I moved it to a policy FriendsOfFlarum/user-bio@1a2655d#diff-dfca4adbcf6b2db93354567377bf9d41

We will still need an additional ->can() call in there because that's where a suspend would actually return false. Just moving the current code to a policy isn't enough but would be best practice. We could introduce a new permission to decide which users are allowed to have an avatar or just query another permission any user should have.

@clarkwinkelmann
Copy link
Member

This might be something to solve while we split the permissions for flarum/framework#1965

@stale
Copy link

stale bot commented Feb 9, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

@stale stale bot added the stale label Feb 9, 2021
@PeopleInside
Copy link

not stale

@PeopleInside
Copy link

Will be nice if this can be fixed in Beta 16

@stale stale bot removed the stale label Feb 10, 2021
@stale
Copy link

stale bot commented Jun 3, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

@stale stale bot added the stale label Jun 3, 2021
@PeopleInside
Copy link

Not stale nice stale bot :)

@stale stale bot removed the stale label Jun 3, 2021
@luceos luceos added the org/keep label Jun 8, 2021
@hxri
Copy link

hxri commented Aug 19, 2021

I would Like to work on this.

@askvortsov1
Copy link
Sponsor Member

I would Like to work on this.

Great to hear! Would you also be interested in some aspects of the linked issue by any chance? Feel free to jump into that discussion 😁

@askvortsov1
Copy link
Sponsor Member

@hxri If you still want to work on this, https://github.com/flarum/core/issues/3033#issuecomment-906090910 might be a good and simple approach (pending discussion, might want to give it a few days unless there are other ideas / objections).

@hxri
Copy link

hxri commented Aug 26, 2021

@hxri If you still want to work on this, #3033 (comment) might be a good and simple approach (pending discussion, might want to give it a few days unless there are other ideas / objections).

Sure! I will work on #3033 (comment) first.

@askvortsov1
Copy link
Sponsor Member

@hxri If you still want to work on this, #3033 (comment) might be a good and simple approach (pending discussion, might want to give it a few days unless there are other ideas / objections).

Sure! I will work on #3033 (comment) first.

Thanks! Since the current suspend implementation adds users to the "guest" group, if we don't allow guests to change their avatar, that should be enough to close that issue.

@SychO9
Copy link
Member

SychO9 commented Sep 27, 2023

flarum/framework#3890

@SychO9 SychO9 closed this as completed Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants