Notifications: Fixed send content permission checking

Added test and changed logic to properly check the view permissions for
the notification receiver before sending.
Required change to permissions applicator to allow the user to be
manually determined, and a service provider update to provide the class
as a singleton without a specific user, so it checks the current logged
in user on demand.

app/Activity/Notifications/Handlers/BaseNotificationHandler.php

@@ -10,11 +10,6 @@ use BookStack\Users\Models\User;
 
 abstract class BaseNotificationHandler implements NotificationHandler
 {
-    public function __construct(
-        protected PermissionApplicator $permissionApplicator
-    ) {
-    }
-
     /**
      * @param class-string<BaseActivityNotification> $notification
      * @param int[] $userIds
@@ -35,7 +30,8 @@ abstract class BaseNotificationHandler implements NotificationHandler
             }
 
             // Prevent sending if the user does not have access to the related content
-            if (!$this->permissionApplicator->checkOwnableUserAccess($relatedModel, 'view')) {
+            $permissions = new PermissionApplicator($user);
+            if (!$permissions->checkOwnableUserAccess($relatedModel, 'view')) {
                 continue;
             }
 

app/Activity/Notifications/NotificationManager.php

@@ -24,7 +24,7 @@ class NotificationManager
         $handlersToRun = $this->handlers[$activityType] ?? [];
         foreach ($handlersToRun as $handlerClass) {
             /** @var NotificationHandler $handler */
-            $handler = app()->make($handlerClass);
+            $handler = new $handlerClass();
             $handler->handle($activity, $detail, $user);
         }
     }

app/App/Providers/AppServiceProvider.php

@@ -9,6 +9,7 @@ use BookStack\Entities\Models\Bookshelf;
 use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Page;
 use BookStack\Exceptions\BookStackExceptionHandlerPage;
+use BookStack\Permissions\PermissionApplicator;
 use BookStack\Settings\SettingService;
 use BookStack\Util\CspService;
 use GuzzleHttp\Client;
@@ -79,5 +80,9 @@ class AppServiceProvider extends ServiceProvider
                 'timeout' => 3,
             ]);
         });
+
+        $this->app->singleton(PermissionApplicator::class, function ($app) {
+            return new PermissionApplicator(null);
+        });
     }
 }

app/Permissions/PermissionApplicator.php

@@ -8,7 +8,6 @@ use BookStack\Entities\Models\Page;
 use BookStack\Permissions\Models\EntityPermission;
 use BookStack\Users\Models\HasCreatorAndUpdater;
 use BookStack\Users\Models\HasOwner;
-use BookStack\Users\Models\Role;
 use BookStack\Users\Models\User;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Query\Builder as QueryBuilder;
@@ -16,6 +15,11 @@ use InvalidArgumentException;
 
 class PermissionApplicator
 {
+    public function __construct(
+        protected ?User $user = null
+    ) {
+    }
+
     /**
      * Checks if an entity has a restriction set upon it.
      *
@@ -173,7 +177,7 @@ class PermissionApplicator
      */
     protected function currentUser(): User
     {
-        return user();
+        return $this->user ?? user();
     }
 
     /**

tests/Activity/WatchTest.php

@@ -312,4 +312,21 @@ class WatchTest extends TestCase
                 && str_contains($mailContent, 'Created By: ' . $admin->name);
         });
     }
+
+    public function test_notifications_not_sent_if_lacking_view_permission_for_related_item()
+    {
+        $notifications = Notification::fake();
+        $editor = $this->users->editor();
+        $page = $this->entities->page();
+
+        $watches = new UserEntityWatchOptions($editor, $page);
+        $watches->updateWatchLevel('comments');
+        $this->permissions->disableEntityInheritedPermissions($page);
+
+        $this->asAdmin()->post("/comment/{$page->id}", [
+            'text' => 'My new comment response',
+        ])->assertOk();
+
+        $notifications->assertNothingSentTo($editor);
+    }
 }