fix(approval): post approved event triggered when not approving

Includes transpiled JS/TS, and Typescript declaration files (typings).

[skip ci]

.github/workflows/frontend.yml

@@ -10,7 +10,7 @@ jobs:
       backend_directory: ./
       js_package_manager: yarn
       cache_dependency_path: ./yarn.lock
-      main_git_branch: main
+      main_git_branch: 1.x
       enable_tests: true
       # @TODO: fix bundlewatch
       enable_bundlewatch: false

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## [v1.8.2](https://github.com/flarum/framework/compare/v1.8.1...v1.8.2)
+### Fixed
+* suspended users can abuse avatar upload [#3890]
+* missing compat exports [#3888]
+
 ## [v1.8.1](https://github.com/flarum/framework/compare/v1.8.0...v1.8.1)
 ### Fixed
 * recover temporary solution for html entities in browser title (e72541e35de4f71f9d870bbd9bb46ddf586bdf1d)

extensions/approval/src/Listener/ApproveContent.php

@@ -28,18 +28,34 @@ class ApproveContent
         $attributes = $event->data['attributes'];
         $post = $event->post;
 
+        // Nothing to do if it is already approved.
+        if ($post->is_approved) {
+            return;
+        }
+
+        /*
+         * We approve a post in one of two cases:
+         * - The post was unapproved and the allowed action is approving it. We trigger an event.
+         * - The post was unapproved and the allowed actor is hiding or un-hiding it.
+         *   We approve it silently if the action is unhiding.
+         */
+        $approvingSilently = false;
+
         if (isset($attributes['isApproved'])) {
             $event->actor->assertCan('approve', $post);
 
             $isApproved = (bool) $attributes['isApproved'];
-        } elseif (! empty($attributes['isHidden']) && $event->actor->can('approve', $post)) {
+        } elseif (isset($attributes['isHidden']) && $event->actor->can('approve', $post)) {
             $isApproved = true;
+            $approvingSilently = $attributes['isHidden'];
         }
 
         if (! empty($isApproved)) {
             $post->is_approved = true;
 
-            $post->raise(new PostWasApproved($post, $event->actor));
+            if (! $approvingSilently) {
+                $post->raise(new PostWasApproved($post, $event->actor));
+            }
         }
     }
 }

extensions/mentions/src/Formatter/FormatPostMentions.php

@@ -11,7 +11,6 @@ namespace Flarum\Mentions\Formatter;
 
 use Flarum\Discussion\Discussion;
 use Flarum\Http\SlugManager;
-use Flarum\Post\CommentPost;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use s9e\TextFormatter\Renderer;
 use s9e\TextFormatter\Utils;

extensions/suspend/src/Access/UserPolicy.php

@@ -25,4 +25,11 @@ class UserPolicy extends AbstractPolicy
             return $this->deny();
         }
     }
+
+    public function uploadAvatar(User $actor, User $user)
+    {
+        if ($actor->suspended_until && $actor->suspended_until->isFuture()) {
+            return $this->deny();
+        }
+    }
 }

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

@@ -0,0 +1,103 @@
+<?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 Laminas\Diactoros\UploadedFile;
+use Psr\Http\Message\ResponseInterface;
+
+class UploadAvatarTest extends TestCase
+{
+    use RetrievesAuthorizedUsers;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->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],
+            ]
+        ]);
+    }
+
+    /**
+     * @dataProvider allowedToUploadAvatar
+     * @test
+     */
+    public function can_suspend_user_if_allowed(?int $authenticatedAs, int $targetUserId, string $message)
+    {
+        $response = $this->sendUploadAvatarRequest($authenticatedAs, $targetUserId);
+
+        $this->assertEquals(200, $response->getStatusCode(), $response->getBody()->getContents());
+    }
+
+    /**
+     * @dataProvider unallowedToUploadAvatar
+     * @test
+     */
+    public function cannot_suspend_user_if_not_allowed(?int $authenticatedAs, int $targetUserId, string $message)
+    {
+        $response = $this->sendUploadAvatarRequest($authenticatedAs, $targetUserId);
+
+        $this->assertEquals(403, $response->getStatusCode(), $response->getBody()->getContents());
+    }
+
+    public function allowedToUploadAvatar(): array
+    {
+        return [
+            [1, 2, 'Admin can upload avatar for any user'],
+            [2, 3, 'User with permission can upload avatar for suspended user'],
+            [2, 2, 'User with permission can upload avatar for self'],
+            [2, 4, 'User with permission can upload avatar for other user'],
+            [1, 1, 'Admin can upload avatar for self'],
+            [5, 5, 'Suspended user can upload avatar for self if suspension expired'],
+        ];
+    }
+
+    public function unallowedToUploadAvatar(): array
+    {
+        return [
+            [3, 3, 'Suspended user cannot upload avatar for self'],
+            [3, 2, 'Suspended user cannot upload avatar for other user'],
+            [4, 3, 'User without permission cannot upload avatar for suspended user'],
+            [4, 2, 'User without permission cannot upload avatar for other user'],
+            [5, 2, 'Suspended user cannot upload avatar for other user if suspension expired'],
+        ];
+    }
+
+    protected function sendUploadAvatarRequest(?int $authenticatedAs, int $targetUserId): ResponseInterface
+    {
+        return $this->send(
+            $this->request('POST', "/api/users/$targetUserId/avatar", [
+                'authenticatedAs' => $authenticatedAs,
+            ])->withHeader('Content-Type', 'multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW')->withUploadedFiles([
+                'avatar' => new UploadedFile(__DIR__.'/../../../fixtures/avatar.png', 0, UPLOAD_ERR_OK, 'avatar.png', 'image/png')
+            ])
+        );
+    }
+}

framework/core/js/dist-typings/admin/compat.d.ts

@@ -122,6 +122,7 @@ declare const _default: {
     'components/TextEditorButton': typeof import("../common/components/TextEditorButton").default;
     'components/Tooltip': typeof import("../common/components/Tooltip").default;
     'components/EditUserModal': typeof import("../common/components/EditUserModal").default;
+    'components/LabelValue': typeof import("../common/components/LabelValue").default;
     Model: typeof import("../common/Model").default;
     Application: typeof import("../common/Application").default;
     'helpers/fullTime': typeof import("../common/helpers/fullTime").default;

framework/core/js/dist-typings/common/compat.d.ts

@@ -86,6 +86,7 @@ import isObject from './utils/isObject';
 import AlertManagerState from './states/AlertManagerState';
 import ModalManagerState from './states/ModalManagerState';
 import PageState from './states/PageState';
+import LabelValue from './components/LabelValue';
 declare const _default: {
     extenders: {
         Model: typeof import("./extenders/Model").default;
@@ -174,6 +175,7 @@ declare const _default: {
     'components/TextEditorButton': typeof TextEditorButton;
     'components/Tooltip': typeof Tooltip;
     'components/EditUserModal': typeof EditUserModal;
+    'components/LabelValue': typeof LabelValue;
     Model: typeof Model;
     Application: typeof Application;
     'helpers/fullTime': typeof fullTime;

framework/core/js/dist-typings/forum/compat.d.ts

@@ -71,6 +71,7 @@ import BasicEditorDriver from '../common/utils/BasicEditorDriver';
 import routes from './routes';
 import ForumApplication from './ForumApplication';
 import isSafariMobile from './utils/isSafariMobile';
+import AccessTokensList from './components/AccessTokensList';
 declare const _default: {
     extenders: {
         Model: typeof import("../common/extenders/Model").default;
@@ -159,6 +160,7 @@ declare const _default: {
     'components/TextEditorButton': typeof import("../common/components/TextEditorButton").default;
     'components/Tooltip': typeof import("../common/components/Tooltip").default;
     'components/EditUserModal': typeof import("../common/components/EditUserModal").default;
+    'components/LabelValue': typeof import("../common/components/LabelValue").default;
     Model: typeof import("../common/Model").default;
     Application: typeof import("../common/Application").default;
     'helpers/fullTime': typeof import("../common/helpers/fullTime").default;
@@ -276,6 +278,7 @@ declare const _default: {
     'components/DiscussionListItem': typeof DiscussionListItem;
     'components/LoadingPost': typeof LoadingPost;
     'components/PostsUserPage': typeof PostsUserPage;
+    'components/AccessTokensList': typeof AccessTokensList;
     'resolvers/DiscussionPageResolver': typeof DiscussionPageResolver;
     routes: typeof routes;
     ForumApplication: typeof ForumApplication;

framework/core/js/src/common/compat.ts

@@ -90,6 +90,7 @@ import isObject from './utils/isObject';
 import AlertManagerState from './states/AlertManagerState';
 import ModalManagerState from './states/ModalManagerState';
 import PageState from './states/PageState';
+import LabelValue from './components/LabelValue';
 
 export default {
   extenders,
@@ -167,6 +168,7 @@ export default {
   'components/TextEditorButton': TextEditorButton,
   'components/Tooltip': Tooltip,
   'components/EditUserModal': EditUserModal,
+  'components/LabelValue': LabelValue,
   Model: Model,
   Application: Application,
   'helpers/fullTime': fullTime,

framework/core/js/src/forum/compat.ts

@@ -75,6 +75,7 @@ import BasicEditorDriver from '../common/utils/BasicEditorDriver';
 import routes from './routes';
 import ForumApplication from './ForumApplication';
 import isSafariMobile from './utils/isSafariMobile';
+import AccessTokensList from './components/AccessTokensList';
 
 export default Object.assign(compat, {
   'utils/PostControls': PostControls,
@@ -150,6 +151,7 @@ export default Object.assign(compat, {
   'components/DiscussionListItem': DiscussionListItem,
   'components/LoadingPost': LoadingPost,
   'components/PostsUserPage': PostsUserPage,
+  'components/AccessTokensList': AccessTokensList,
   'resolvers/DiscussionPageResolver': DiscussionPageResolver,
   routes: routes,
   ForumApplication: ForumApplication,

framework/core/src/Foundation/Application.php

@@ -21,7 +21,7 @@ class Application
      *
      * @var string
      */
-    const VERSION = '1.8.1';
+    const VERSION = '1.8.2';
 
     /**
      * The IoC container for the Flarum application.

framework/core/src/Queue/ExceptionHandler.php

@@ -9,7 +9,6 @@
 
 namespace Flarum\Queue;
 
-use Exception;
 use Illuminate\Contracts\Debug\ExceptionHandler as ExceptionHandling;
 use Psr\Log\LoggerInterface;
 use Throwable;

framework/core/src/User/Access/UserPolicy.php

@@ -39,4 +39,15 @@ class UserPolicy extends AbstractPolicy
             return $this->allow();
         }
     }
+
+    public function uploadAvatar(User $actor, User $user)
+    {
+        if ($actor->id === $user->id) {
+            return $this->allow();
+        }
+
+        if ($actor->id !== $user->id) {
+            return $actor->can('edit', $user);
+        }
+    }
 }

framework/core/src/User/Command/UploadAvatarHandler.php

@@ -68,9 +68,7 @@ class UploadAvatarHandler
 
         $user = $this->users->findOrFail($command->userId);
 
-        if ($actor->id !== $user->id) {
-            $actor->assertCan('edit', $user);
-        }
+        $actor->assertCan('uploadAvatar', $user);
 
         $this->validator->assertValid(['avatar' => $command->file]);