diff --git a/extensions/suspend/extend.php b/extensions/suspend/extend.php index fe924d00b7..5a94d7433e 100644 --- a/extensions/suspend/extend.php +++ b/extensions/suspend/extend.php @@ -19,6 +19,7 @@ use Flarum\Suspend\Notification\UserUnsuspendedBlueprint; use Flarum\Suspend\Query\SuspendedFilterGambit; use Flarum\Suspend\RevokeAccessFromSuspendedUsers; +use Flarum\User\Event\AvatarDeleting; use Flarum\User\Event\Saving; use Flarum\User\Filter\UserFilterer; use Flarum\User\Search\UserSearcher; @@ -50,7 +51,8 @@ (new Extend\Event()) ->listen(Saving::class, Listener\SaveSuspensionToDatabase::class) ->listen(Suspended::class, Listener\SendNotificationWhenUserIsSuspended::class) - ->listen(Unsuspended::class, Listener\SendNotificationWhenUserIsUnsuspended::class), + ->listen(Unsuspended::class, Listener\SendNotificationWhenUserIsUnsuspended::class) + ->listen(AvatarDeleting::class, Listener\PreventAvatarDeletionBySuspendedUser::class), (new Extend\Policy()) ->modelPolicy(User::class, UserPolicy::class), diff --git a/extensions/suspend/src/Listener/PreventAvatarDeletionBySuspendedUser.php b/extensions/suspend/src/Listener/PreventAvatarDeletionBySuspendedUser.php new file mode 100644 index 0000000000..9d3595e394 --- /dev/null +++ b/extensions/suspend/src/Listener/PreventAvatarDeletionBySuspendedUser.php @@ -0,0 +1,18 @@ +actor; + $user = $event->user; + + if ($actor->id === $user->id && $user->suspended_until) { + throw new PermissionDeniedException(); + } + } +} diff --git a/extensions/suspend/tests/integration/api/users/RemoveAvatarTest.php b/extensions/suspend/tests/integration/api/users/RemoveAvatarTest.php new file mode 100644 index 0000000000..a2b9fc9ef7 --- /dev/null +++ b/extensions/suspend/tests/integration/api/users/RemoveAvatarTest.php @@ -0,0 +1,89 @@ +extension('flarum-suspend'); + + $this->prepareDatabase([ + 'users' => [ + ['id' => 1, 'username' => 'Muralf', 'email' => 'muralf@machine.local', 'is_email_confirmed' => 1], + $this->normalUser(), + ['id' => 3, 'username' => 'acme', 'email' => 'acme@machine.local', 'is_email_confirmed' => 1, 'suspended_until' => Carbon::now()->addDay(), 'suspend_message' => 'You have been suspended.', 'suspend_reason' => 'Suspended for acme reasons.'], + ['id' => 4, 'username' => 'acme4', 'email' => 'acme4@machine.local', 'is_email_confirmed' => 1], + ['id' => 5, 'username' => 'acme5', 'email' => 'acme5@machine.local', 'is_email_confirmed' => 1, 'suspended_until' => Carbon::now()->subDay(), 'suspend_message' => 'You have been suspended.', 'suspend_reason' => 'Suspended for acme reasons.'], + ], + 'groups' => [ + ['id' => 5, 'name_singular' => 'can_edit_users', 'name_plural' => 'can_edit_users', 'is_hidden' => 0] + ], + 'group_user' => [ + ['user_id' => 2, 'group_id' => 5] + ], + 'group_permission' => [ + ['permission' => 'user.edit', 'group_id' => 5], + ] + ]); + } + + /** + * @test + * @dataProvider allowedToRemoveAvatar + */ + public function can_remove_avatar_if_allowed(?int $authenticatedAs, int $targetUserId) + { + $response = $this->sendRemoveAvatarRequest($authenticatedAs, $targetUserId); + + $this->assertEquals(200, $response->getStatusCode(), $response->getBody()->getContents()); + } + + /** + * @test + * @dataProvider notAllowedToRemoveAvatar + */ + public function cannot_remove_avatar_if_not_allowed(?int $authenticatedAs, int $targetUserId) + { + $response = $this->sendRemoveAvatarRequest($authenticatedAs, $targetUserId); + + $this->assertEquals(403, $response->getStatusCode(), $response->getBody()->getContents()); + } + + public function allowedToRemoveAvatar(): array + { + return [ + [1, 4, 'Admin can remove avatar of normal user'], + [4, 4, 'Normal user can remove their own avatar'], + [1, 3, 'Admin can remove avatar of suspended user'], + [2, 3, 'Normal user with permission can remove avatar of suspended user'], + ]; + } + + public function notAllowedToRemoveAvatar(): array + { + return [ + [4, 2, 'Normal user cannot remove avatar of another user'], + [4, 3, 'Normal user cannot remove avatar of suspended user'], + [3, 3, 'Suspended user cannot remove their own avatar'], + ]; + } + + protected function sendRemoveAvatarRequest(?int $authenticatedAs, int $targetUserId): ResponseInterface + { + return $this->send( + $this->request('DELETE', "/api/users/$targetUserId/avatar", [ + 'authenticatedAs' => $authenticatedAs, + ]) + ); + } +} diff --git a/framework/core/src/User/Command/DeleteAvatarHandler.php b/framework/core/src/User/Command/DeleteAvatarHandler.php index f54954a4af..3784342b76 100644 --- a/framework/core/src/User/Command/DeleteAvatarHandler.php +++ b/framework/core/src/User/Command/DeleteAvatarHandler.php @@ -56,12 +56,12 @@ public function handle(DeleteAvatar $command) $actor->assertCan('edit', $user); } - $this->uploader->remove($user); - $this->events->dispatch( new AvatarDeleting($user, $actor) ); + $this->uploader->remove($user); + $user->save(); $this->dispatchEventsFor($user, $actor);