Fix unauthorized view of restricted tags (#145)

* test: Not allowed to see tags even if included as a relation
* fix: Scope loaded related tags where actor has permission

extensions/tags/src/Api/Controller/ListTagsController.php

@@ -12,7 +12,7 @@ namespace Flarum\Tags\Api\Controller;
 use Flarum\Api\Controller\AbstractListController;
 use Flarum\Http\RequestUtil;
 use Flarum\Tags\Api\Serializer\TagSerializer;
-use Flarum\Tags\Tag;
+use Flarum\Tags\TagRepository;
 use Psr\Http\Message\ServerRequestInterface;
 use Tobscure\JsonApi\Document;
 
@@ -40,14 +40,11 @@ class ListTagsController extends AbstractListController
     ];
 
     /**
-     * @var \Flarum\Tags\Tag
+     * @var TagRepository
      */
     protected $tags;
 
-    /**
-     * @param \Flarum\Tags\Tag $tags
-     */
-    public function __construct(Tag $tags)
+    public function __construct(TagRepository $tags)
     {
         $this->tags = $tags;
     }
@@ -64,8 +61,10 @@ class ListTagsController extends AbstractListController
             $include = array_merge($include, ['lastPostedDiscussion.tags', 'lastPostedDiscussion.state']);
         }
 
-        $tags = $this->tags->whereVisibleTo($actor)->withStateFor($actor)->get();
-
-        return $tags->load($include);
+        return $this->tags
+            ->with($include, $actor)
+            ->whereVisibleTo($actor)
+            ->withStateFor($actor)
+            ->get();
     }
 }

extensions/tags/src/Api/Controller/ShowTagController.php

@@ -12,7 +12,7 @@ namespace Flarum\Tags\Api\Controller;
 use Flarum\Api\Controller\AbstractShowController;
 use Flarum\Http\RequestUtil;
 use Flarum\Tags\Api\Serializer\TagSerializer;
-use Flarum\Tags\Tag;
+use Flarum\Tags\TagRepository;
 use Illuminate\Support\Arr;
 use Psr\Http\Message\ServerRequestInterface;
 use Tobscure\JsonApi\Document;
@@ -32,11 +32,11 @@ class ShowTagController extends AbstractShowController
     ];
 
     /**
-     * @var Tag
+     * @var TagRepository
      */
     private $tags;
 
-    public function __construct(Tag $tags)
+    public function __construct(TagRepository $tags)
     {
         $this->tags = $tags;
     }
@@ -51,8 +51,8 @@ class ShowTagController extends AbstractShowController
         $include = $this->extractInclude($request);
 
         return $this->tags
+            ->with($include, $actor)
             ->whereVisibleTo($actor)
-            ->with($include)
             ->where('slug', $slug)
             ->firstOrFail();
     }

extensions/tags/src/TagRepository.php

@@ -14,6 +14,8 @@ use Illuminate\Database\Eloquent\Builder;
 
 class TagRepository
 {
+    private const TAG_RELATIONS = ['children', 'parent'];
+
     /**
      * Get a new query builder for the tags table.
      *
@@ -24,6 +26,28 @@ class TagRepository
         return Tag::query();
     }
 
+    /**
+     * @param array|string $relations
+     * @param User $actor
+     * @return Builder
+     */
+    public function with($relations, User $actor): Builder
+    {
+        $relationsArray = is_string($relations) ? explode(',', $relations) : $relations;
+
+        foreach (self::TAG_RELATIONS as $relation) {
+            if (in_array($relation, $relationsArray, true)) {
+                $relationsArray = array_diff($relationsArray, [$relation]);
+
+                $relationsArray[$relation] = function ($query) use ($actor) {
+                    $query->whereVisibleTo($actor);
+                };
+            }
+        }
+
+        return $this->query()->with($relationsArray);
+    }
+
     /**
      * Find a tag by ID, optionally making sure it is visible to a certain
      * user, or throw an exception.

extensions/tags/tests/integration/api/tags/ListTest.php

@@ -82,6 +82,33 @@ class ListTest extends TestCase
         $this->assertEquals(['1', '2', '3', '4', '9', '10', '11'], $ids);
     }
 
+    /**
+     * @test
+     */
+    public function user_sees_where_allowed_with_included_tags()
+    {
+        $response = $this->send(
+            $this->request('GET', '/api/tags', [
+                'authenticatedAs' => 2,
+            ])->withQueryParams([
+                'include' => 'children'
+            ])
+        );
+
+        $this->assertEquals(200, $response->getStatusCode());
+
+        $responseBody = json_decode($response->getBody()->getContents(), true);
+
+        $data = $responseBody['data'];
+        $included = $responseBody['included'];
+
+        // 5 isnt included because parent access doesnt necessarily give child access
+        // 6, 7, 8 aren't included because child access shouldnt work unless parent
+        // access is also given.
+        $this->assertEquals(['1', '2', '3', '4', '9', '10', '11'], Arr::pluck($data, 'id'));
+        $this->assertEquals(['3', '4'], Arr::pluck($included, 'id'));
+    }
+
     /**
      * @test
      */