[suspend][core] [1.x] fix: suspended users can remove avatar

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),

extensions/suspend/src/Listener/PreventAvatarDeletionBySuspendedUser.php

@@ -0,0 +1,25 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Suspend\Listener;
+
+use Flarum\User\Exception\PermissionDeniedException;
+
+class PreventAvatarDeletionBySuspendedUser
+{
+    public function handle($event)
+    {
+        $actor = $event->actor;
+        $user = $event->user;
+
+        if ($actor->id === $user->id && $user->suspended_until) {
+            throw new PermissionDeniedException();
+        }
+    }
+}

extensions/suspend/tests/integration/api/users/RemoveAvatarTest.php

@@ -0,0 +1,96 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Suspend\Tests\integration\api\users;
+
+use Carbon\Carbon;
+use Flarum\Testing\integration\RetrievesAuthorizedUsers;
+use Flarum\Testing\integration\TestCase;
+use Psr\Http\Message\ResponseInterface;
+
+class RemoveAvatarTest extends TestCase
+{
+    use RetrievesAuthorizedUsers;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+
+        $this->extension('flarum-suspend');
+
+        $this->prepareDatabase([
+            'users' => [
+                ['id' => 1, 'username' => 'Muralf', 'email' => '[email protected]', 'is_email_confirmed' => 1],
+                $this->normalUser(),
+                ['id' => 3, 'username' => 'acme', 'email' => '[email protected]', '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' => '[email protected]', 'is_email_confirmed' => 1],
+                ['id' => 5, 'username' => 'acme5', 'email' => '[email protected]', '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,
+            ])
+        );
+    }
+}

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);