From e5f05166a062a9a6eb7c12e28728bfd5db7270e3 Mon Sep 17 00:00:00 2001
From: Sami Mazouz <sychocouldy@gmail.com>
Date: Tue, 10 Jan 2023 15:04:02 +0100
Subject: [PATCH] fix(subscriptions): post notifications not getting access
 checked

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
---
 extensions/subscriptions/extend.php           |  4 +-
 .../FilterVisiblePostsBeforeSending.php       | 28 ++++++++++++
 .../api/discussions/ReplyNotificationTest.php | 45 +++++++++++++++++++
 3 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 extensions/subscriptions/src/Notification/FilterVisiblePostsBeforeSending.php

diff --git a/extensions/subscriptions/extend.php b/extensions/subscriptions/extend.php
index c910e72967..e39da0de5b 100644
--- a/extensions/subscriptions/extend.php
+++ b/extensions/subscriptions/extend.php
@@ -21,6 +21,7 @@
 use Flarum\Post\Event\Restored;
 use Flarum\Subscriptions\HideIgnoredFromAllDiscussionsPage;
 use Flarum\Subscriptions\Listener;
+use Flarum\Subscriptions\Notification\FilterVisiblePostsBeforeSending;
 use Flarum\Subscriptions\Notification\NewPostBlueprint;
 use Flarum\Subscriptions\Query\SubscriptionFilterGambit;
 
@@ -36,7 +37,8 @@
         ->namespace('flarum-subscriptions', __DIR__.'/views'),
 
     (new Extend\Notification())
-        ->type(NewPostBlueprint::class, BasicDiscussionSerializer::class, ['alert', 'email']),
+        ->type(NewPostBlueprint::class, BasicDiscussionSerializer::class, ['alert', 'email'])
+        ->beforeSending(FilterVisiblePostsBeforeSending::class),
 
     (new Extend\ApiSerializer(DiscussionSerializer::class))
         ->attribute('subscription', function (DiscussionSerializer $serializer, Discussion $discussion) {
diff --git a/extensions/subscriptions/src/Notification/FilterVisiblePostsBeforeSending.php b/extensions/subscriptions/src/Notification/FilterVisiblePostsBeforeSending.php
new file mode 100644
index 0000000000..818c8e9d59
--- /dev/null
+++ b/extensions/subscriptions/src/Notification/FilterVisiblePostsBeforeSending.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace Flarum\Subscriptions\Notification;
+
+use Flarum\Notification\Blueprint\BlueprintInterface;
+
+class FilterVisiblePostsBeforeSending
+{
+    public function __invoke(BlueprintInterface $blueprint, array $recipients): array
+    {
+        if ($blueprint instanceof NewPostBlueprint) {
+            $newRecipients = [];
+
+            // Flarum has built-in access control for the notification subject,
+            // but subscriptions post notifications has the discussion as the subject.
+            // We'll add a post visibility check so that users can't get access to hidden replies by subscribing.
+            foreach ($recipients as $recipient) {
+                if ($blueprint->post->isVisibleTo($recipient)) {
+                    $newRecipients[] = $recipient;
+                }
+            }
+
+            return $newRecipients;
+        }
+
+        return $recipients;
+    }
+}
diff --git a/extensions/subscriptions/tests/integration/api/discussions/ReplyNotificationTest.php b/extensions/subscriptions/tests/integration/api/discussions/ReplyNotificationTest.php
index 0600865b7d..ea1aba479a 100644
--- a/extensions/subscriptions/tests/integration/api/discussions/ReplyNotificationTest.php
+++ b/extensions/subscriptions/tests/integration/api/discussions/ReplyNotificationTest.php
@@ -10,7 +10,9 @@
 namespace Flarum\Subscriptions\tests\integration\api\discussions;
 
 use Carbon\Carbon;
+use Flarum\Extend\ModelVisibility;
 use Flarum\Group\Group;
+use Flarum\Post\Post;
 use Flarum\Testing\integration\RetrievesAuthorizedUsers;
 use Flarum\Testing\integration\TestCase;
 use Flarum\User\User;
@@ -278,4 +280,47 @@ public function approving_reply_sends_reply_notification()
 
         $this->assertEquals(1, $mainUser->getUnreadNotificationCount());
     }
+
+    /** @test */
+    public function replying_to_a_discussion_with_a_restricted_post_only_sends_notifications_to_allowed_users()
+    {
+        // Add visibility scoper to only allow admin
+        // to see expected new post with content containing 'restricted-test-post'.
+        $this->extend(
+            (new ModelVisibility(Post::class))
+                ->scope(function (User $actor, $query) {
+                    if (! $actor->isAdmin()) {
+                        $query->where('content', 'not like', '%restricted-test-post%');
+                    }
+                })
+        );
+
+        $this->app();
+
+        /** @var User $allowedUser */
+        $allowedUser = User::query()->find(1);
+        $normalUser = User::query()->find(2);
+
+        $this->assertEquals(0, $allowedUser->getUnreadNotificationCount());
+        $this->assertEquals(0, $normalUser->getUnreadNotificationCount());
+
+        $this->send(
+            $this->request('POST', '/api/posts', [
+                'authenticatedAs' => 3,
+                'json' => [
+                    'data' => [
+                        'attributes' => [
+                            'content' => 'restricted-test-post',
+                        ],
+                        'relationships' => [
+                            'discussion' => ['data' => ['id' => 1]],
+                        ],
+                    ],
+                ],
+            ])
+        );
+
+        $this->assertEquals(1, $allowedUser->getUnreadNotificationCount());
+        $this->assertEquals(0, $normalUser->getUnreadNotificationCount());
+    }
 }