diff --git a/extensions/tags/extend.php b/extensions/tags/extend.php
index 8f48d169a..cead40942 100644
--- a/extensions/tags/extend.php
+++ b/extensions/tags/extend.php
@@ -71,7 +71,7 @@ return [
->load('post.discussion.tags'),
(new Extend\ApiController(FlarumController\ListDiscussionsController::class))
- ->addInclude(['tags', 'tags.state'])
+ ->addInclude(['tags', 'tags.state', 'tags.parent'])
->load('tags'),
(new Extend\ApiController(FlarumController\ShowDiscussionController::class))
@@ -96,7 +96,6 @@ return [
->globalPolicy(Access\GlobalPolicy::class),
(new Extend\ModelVisibility(Discussion::class))
- ->scope(Access\ScopeDiscussionVisibility::class)
->scopeAll(Access\ScopeDiscussionVisibilityForAbility::class),
(new Extend\ModelVisibility(Flag::class))
diff --git a/extensions/tags/src/Access/GlobalPolicy.php b/extensions/tags/src/Access/GlobalPolicy.php
index 94301e19d..a33ff2d2a 100644
--- a/extensions/tags/src/Access/GlobalPolicy.php
+++ b/extensions/tags/src/Access/GlobalPolicy.php
@@ -33,14 +33,27 @@ class GlobalPolicy extends AbstractPolicy
*/
public function can(User $actor, string $ability)
{
+ static $enoughPrimary;
+ static $enoughSecondary;
+
if (in_array($ability, ['viewDiscussions', 'startDiscussion'])) {
if ($actor->hasPermission($ability) && $actor->hasPermission('bypassTagCounts')) {
return $this->allow();
}
- $enoughPrimary = Tag::queryIdsWhereCan(Tag::query()->getQuery(), $actor, $ability, true, false)->count() >= $this->settings->get('flarum-tags.min_primary_tags');
- $enoughSecondary = Tag::queryIdsWhereCan(Tag::query()->getQuery(), $actor, $ability, false, true)->count() >= $this->settings->get('flarum-tags.min_secondary_tags');
- if ($enoughPrimary && $enoughSecondary) {
+ if (! isset($enoughPrimary[$actor->id][$ability])) {
+ $enoughPrimary[$actor->id][$ability] = Tag::whereHasPermission($actor, $ability)
+ ->where('tags.position', '!=', null)
+ ->count() >= $this->settings->get('flarum-tags.min_primary_tags');
+ }
+
+ if (! isset($enoughSecondary[$actor->id][$ability])) {
+ $enoughSecondary[$actor->id][$ability] = Tag::whereHasPermission($actor, $ability)
+ ->where('tags.position', '=', null)
+ ->count() >= $this->settings->get('flarum-tags.min_secondary_tags');
+ }
+
+ if ($enoughPrimary[$actor->id][$ability] && $enoughSecondary[$actor->id][$ability]) {
return $this->allow();
} else {
return $this->deny();
diff --git a/extensions/tags/src/Access/ScopeDiscussionVisibility.php b/extensions/tags/src/Access/ScopeDiscussionVisibility.php
deleted file mode 100644
index 7e004b8b4..000000000
--- a/extensions/tags/src/Access/ScopeDiscussionVisibility.php
+++ /dev/null
@@ -1,45 +0,0 @@
-where(function ($query) use ($actor) {
- $query
- ->whereNotIn('discussions.id', function ($query) use ($actor) {
- return $query->select('discussion_id')
- ->from('discussion_tag')
- ->whereNotIn('tag_id', function ($query) use ($actor) {
- Tag::queryIdsWhereCan($query->from('tags'), $actor, 'viewDiscussions');
- });
- })
- ->orWhere(function ($query) use ($actor) {
- $query->whereVisibleTo($actor, 'viewDiscussionsInRestrictedTags');
- });
- });
-
- // Hide discussions with no tags if the user doesn't have that global
- // permission.
- if (! $actor->hasPermission('viewDiscussions')) {
- $query->has('tags');
- }
- }
-}
diff --git a/extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php b/extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php
index 26a9310f7..585e4977a 100644
--- a/extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php
+++ b/extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php
@@ -12,6 +12,7 @@ namespace Flarum\Tags\Access;
use Flarum\Tags\Tag;
use Flarum\User\User;
use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Support\Str;
class ScopeDiscussionVisibilityForAbility
{
@@ -22,19 +23,46 @@ class ScopeDiscussionVisibilityForAbility
*/
public function __invoke(User $actor, Builder $query, $ability)
{
- if (substr($ability, 0, 4) === 'view') {
+ // Automatic scoping should be applied to the global `view` ability,
+ // and to arbitrary abilities that aren't subqueries of `view`.
+ // For example, if we want to scope discussions where the user can
+ // edit posts, this should apply.
+ // But if we are expanding a restriction of `view` (for example,
+ // `viewPrivate`), we shouldn't apply this query again.
+ if (substr($ability, 0, 4) === 'view' && $ability !== 'view') {
return;
}
- // If a discussion requires a certain permission in order for it to be
- // visible, then we can check if the user has been granted that
- // permission for any of the discussion's tags.
- $query->whereIn('discussions.id', function ($query) use ($actor, $ability) {
- return $query->select('discussion_id')
- ->from('discussion_tag')
- ->whereIn('tag_id', function ($query) use ($actor, $ability) {
- Tag::queryIdsWhereCan($query->from('tags'), $actor, 'discussion.'.$ability);
+ // Avoid an infinite recursive loop.
+ if (Str::endsWith($ability, 'InRestrictedTags')) {
+ return;
+ }
+
+ // `view` is a special case where the permission string is represented by `viewDiscussions`.
+ $permission = $ability === 'view' ? 'viewDiscussions' : $ability;
+
+ // Restrict discussions where users don't have necessary permissions in all tags.
+ // We use a double notIn instead of a doubleIn because the permission must be present in ALL tags,
+ // not just one.
+ $query->where(function ($query) use ($actor, $permission) {
+ $query
+ ->whereNotIn('discussions.id', function ($query) use ($actor, $permission) {
+ return $query->select('discussion_id')
+ ->from('discussion_tag')
+ ->whereNotIn('tag_id', function ($query) use ($actor, $permission) {
+ Tag::query()->setQuery($query->from('tags'))->whereHasPermission($actor, $permission)->select('tags.id');
+ });
+ })
+ ->orWhere(function ($query) use ($actor, $permission) {
+ // Allow extensions a way to override scoping for any given permission.
+ $query->whereVisibleTo($actor, "${permission}InRestrictedTags");
});
});
+
+ // Hide discussions with no tags if the user doesn't have that global
+ // permission.
+ if (! $actor->hasPermission($permission)) {
+ $query->has('tags');
+ }
}
}
diff --git a/extensions/tags/src/Access/ScopeFlagVisibility.php b/extensions/tags/src/Access/ScopeFlagVisibility.php
index f9547e3e0..b926fff5e 100644
--- a/extensions/tags/src/Access/ScopeFlagVisibility.php
+++ b/extensions/tags/src/Access/ScopeFlagVisibility.php
@@ -29,7 +29,7 @@ class ScopeFlagVisibility
return $query->selectRaw('1')
->from('discussion_tag')
->whereNotIn('tag_id', function ($query) use ($actor) {
- Tag::queryIdsWhereCan($query->from('tags'), $actor, 'discussion.viewFlags');
+ Tag::query()->setQuery($query->from('tags'))->whereHasPermission($actor, 'viewFlags')->select('tags.id');
})
->whereColumn('discussions.id', 'discussion_id');
});
diff --git a/extensions/tags/src/Access/ScopeTagVisibility.php b/extensions/tags/src/Access/ScopeTagVisibility.php
index 2802efe0b..61274825d 100644
--- a/extensions/tags/src/Access/ScopeTagVisibility.php
+++ b/extensions/tags/src/Access/ScopeTagVisibility.php
@@ -22,7 +22,7 @@ class ScopeTagVisibility
public function __invoke(User $actor, Builder $query)
{
$query->whereIn('id', function ($query) use ($actor) {
- Tag::queryIdsWhereCan($query, $actor, 'viewDiscussions');
+ Tag::query()->setQuery($query->from('tags'))->whereHasPermission($actor, 'viewDiscussions')->select('tags.id');
});
}
}
diff --git a/extensions/tags/src/Access/TagPolicy.php b/extensions/tags/src/Access/TagPolicy.php
index 3578712ca..25b74c6c3 100755
--- a/extensions/tags/src/Access/TagPolicy.php
+++ b/extensions/tags/src/Access/TagPolicy.php
@@ -15,25 +15,21 @@ use Flarum\User\User;
class TagPolicy extends AbstractPolicy
{
- /**
- * @param User $actor
- * @param Tag $tag
- * @return bool|null
- */
- public function startDiscussion(User $actor, Tag $tag)
+ public function can(User $actor, string $ability, Tag $tag)
{
+ if ($tag->parent_id !== null && ! $actor->can($ability, $tag->parent)) {
+ return $this->deny();
+ }
+
if ($tag->is_restricted) {
- return $actor->hasPermission('tag'.$tag->id.'.startDiscussion') ? $this->allow() : $this->deny();
+ $id = $tag->id;
+
+ return $actor->hasPermission("tag$id.$ability");
}
}
- /**
- * @param User $actor
- * @param Tag $tag
- * @return bool|null
- */
public function addToDiscussion(User $actor, Tag $tag)
{
- return $actor->can('startDiscussion', $tag) ? $this->allow() : $this->deny();
+ return $actor->can('startDiscussion', $tag);
}
}
diff --git a/extensions/tags/src/Tag.php b/extensions/tags/src/Tag.php
index 45f62a2f4..134d64e9a 100644
--- a/extensions/tags/src/Tag.php
+++ b/extensions/tags/src/Tag.php
@@ -15,7 +15,6 @@ use Flarum\Discussion\Discussion;
use Flarum\Group\Permission;
use Flarum\User\User;
use Illuminate\Database\Eloquent\Builder;
-use Illuminate\Database\Query\Builder as QueryBuilder;
/**
* @property int $id
@@ -219,7 +218,7 @@ class Tag extends AbstractModel
}
}
- public static function queryIdsWhereCan(QueryBuilder $base, User $user, string $currPermission, bool $includePrimary = true, bool $includeSecondary = true): QueryBuilder
+ public function scopeWhereHasPermission(Builder $query, User $user, string $currPermission): Builder
{
$hasGlobalPermission = $user->hasPermission($currPermission);
$isAdmin = $user->isAdmin();
@@ -236,30 +235,19 @@ class Tag extends AbstractModel
})
->values();
- $validTags = $base
- ->from('tags as s_tags')
+ return $query
->where(function ($query) use ($isAdmin, $hasGlobalPermission, $tagIdsWithPermission) {
$query
- ->whereIn('s_tags.id', function ($query) use ($isAdmin, $hasGlobalPermission, $tagIdsWithPermission) {
+ ->whereIn('tags.id', function ($query) use ($isAdmin, $hasGlobalPermission, $tagIdsWithPermission) {
static::buildPermissionSubquery($query, $isAdmin, $hasGlobalPermission, $tagIdsWithPermission);
})
->where(function ($query) use ($isAdmin, $hasGlobalPermission, $tagIdsWithPermission) {
$query
- ->whereIn('s_tags.parent_id', function ($query) use ($isAdmin, $hasGlobalPermission, $tagIdsWithPermission) {
+ ->whereIn('tags.parent_id', function ($query) use ($isAdmin, $hasGlobalPermission, $tagIdsWithPermission) {
static::buildPermissionSubquery($query, $isAdmin, $hasGlobalPermission, $tagIdsWithPermission);
})
- ->orWhere('s_tags.parent_id', null);
+ ->orWhere('tags.parent_id', null);
});
- })
- ->where(function ($query) use ($includePrimary, $includeSecondary) {
- if (! $includePrimary) {
- $query->where('s_tags.position', '=', null);
- }
- if (! $includeSecondary) {
- $query->where('s_tags.position', '!=', null);
- }
});
-
- return $validTags->select('s_tags.id');
}
}
diff --git a/extensions/tags/tests/integration/RetrievesRepresentativeTags.php b/extensions/tags/tests/integration/RetrievesRepresentativeTags.php
index 06d346f91..2eafbd111 100644
--- a/extensions/tags/tests/integration/RetrievesRepresentativeTags.php
+++ b/extensions/tags/tests/integration/RetrievesRepresentativeTags.php
@@ -25,6 +25,9 @@ trait RetrievesRepresentativeTags
['id' => 9, 'name' => 'Secondary 1', 'slug' => 'secondary-1', 'position' => null, 'parent_id' => null],
['id' => 10, 'name' => 'Secondary 2', 'slug' => 'secondary-2', 'position' => null, 'parent_id' => null],
['id' => 11, 'name' => 'Secondary Restricted', 'slug' => 'secondary-restricted', 'position' => null, 'parent_id' => null, 'is_restricted' => true],
+ ['id' => 12, 'name' => 'Primary Restricted 2', 'slug' => 'primary-2-restricted', 'position' => 100, 'parent_id' => null, 'is_restricted' => true],
+ ['id' => 13, 'name' => 'Primary Restricted 2 Child 1', 'slug' => 'primary-2-restricted-child-1', 'position' => 101, 'parent_id' => 12],
+ ['id' => 14, 'name' => 'Primary Restricted 3', 'slug' => 'primary-3-restricted', 'position' => 102, 'parent_id' =>null, 'is_restricted' => true],
];
}
}
diff --git a/extensions/tags/tests/integration/api/discussions/CreateTest.php b/extensions/tags/tests/integration/api/discussions/CreateTest.php
index a131e07ed..81bc6ac90 100644
--- a/extensions/tags/tests/integration/api/discussions/CreateTest.php
+++ b/extensions/tags/tests/integration/api/discussions/CreateTest.php
@@ -221,8 +221,8 @@ class CreateTest extends TestCase
'relationships' => [
'tags' => [
'data' => [
- ['type' => 'tags', 'id' => 3],
- ['type' => 'tags', 'id' => 4]
+ ['type' => 'tags', 'id' => 2],
+ ['type' => 'tags', 'id' => 3]
]
]
]
@@ -231,7 +231,7 @@ class CreateTest extends TestCase
])
);
- $this->assertEquals(422, $response->getStatusCode());
+ $this->assertEquals(201, $response->getStatusCode());
}
/**
diff --git a/extensions/tags/tests/integration/api/discussions/ListTest.php b/extensions/tags/tests/integration/api/discussions/ListTest.php
new file mode 100644
index 000000000..0290f6ff6
--- /dev/null
+++ b/extensions/tags/tests/integration/api/discussions/ListTest.php
@@ -0,0 +1,128 @@
+extension('flarum-tags');
+
+ $this->prepareDatabase([
+ 'tags' => $this->tags(),
+ 'users' => [
+ $this->normalUser(),
+ ],
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag5.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag8.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag11.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag13.viewDiscussions'],
+ ],
+ 'discussions' => [
+ ['id' => 1, 'title' => 'no tags', 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 2, 'title' => 'open tags', 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 3, 'title' => 'open tag, restricted child tag', 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 4, 'title' => 'open tag, one restricted secondary tag', 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 5, 'title' => 'all closed', 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 6, 'title' => 'closed parent, open child tag', 'user_id' => 1, 'comment_count' => 1],
+ ],
+ 'posts' => [
+ ['id' => 1, 'discussion_id' => 1, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 2, 'discussion_id' => 2, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 3, 'discussion_id' => 3, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 4, 'discussion_id' => 4, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 5, 'discussion_id' => 5, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 6, 'discussion_id' => 6, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ],
+ 'discussion_tag' => [
+ ['discussion_id' => 2, 'tag_id' => 1],
+ ['discussion_id' => 3, 'tag_id' => 2],
+ ['discussion_id' => 3, 'tag_id' => 5],
+ ['discussion_id' => 4, 'tag_id' => 1],
+ ['discussion_id' => 4, 'tag_id' => 11],
+ ['discussion_id' => 5, 'tag_id' => 6],
+ ['discussion_id' => 5, 'tag_id' => 7],
+ ['discussion_id' => 5, 'tag_id' => 8],
+ ['discussion_id' => 6, 'tag_id' => 12],
+ ['discussion_id' => 6, 'tag_id' => 13],
+ ],
+ ]);
+ }
+
+ /**
+ * @test
+ */
+ public function admin_sees_all()
+ {
+ $response = $this->send(
+ $this->request('GET', '/api/discussions', [
+ 'authenticatedAs' => 1
+ ])
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+
+ $data = json_decode($response->getBody()->getContents(), true)['data'];
+
+ $ids = Arr::pluck($data, 'id');
+ $this->assertEqualsCanonicalizing(['1', '2', '3', '4', '5', '6'], $ids);
+ }
+
+ /**
+ * @test
+ */
+ public function user_sees_where_allowed()
+ {
+ $response = $this->send(
+ $this->request('GET', '/api/discussions', [
+ 'authenticatedAs' => 2
+ ])
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+
+ $data = json_decode($response->getBody()->getContents(), true)['data'];
+
+ $ids = Arr::pluck($data, 'id');
+ $this->assertEqualsCanonicalizing(['1', '2', '3', '4'], $ids);
+ }
+
+ /**
+ * @test
+ */
+ public function guest_can_see_where_allowed()
+ {
+ $response = $this->send(
+ $this->request('GET', '/api/discussions')
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+
+ $data = json_decode($response->getBody()->getContents(), true)['data'];
+
+ $ids = Arr::pluck($data, 'id');
+ $this->assertEqualsCanonicalizing(['1', '2'], $ids);
+ }
+}
diff --git a/extensions/tags/tests/integration/api/discussions/UpdateTest.php b/extensions/tags/tests/integration/api/discussions/UpdateTest.php
new file mode 100644
index 000000000..b2e87f9b4
--- /dev/null
+++ b/extensions/tags/tests/integration/api/discussions/UpdateTest.php
@@ -0,0 +1,301 @@
+extension('flarum-tags');
+
+ $this->prepareDatabase([
+ 'tags' => $this->tags(),
+ 'users' => [
+ $this->normalUser(),
+ ],
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag5.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag8.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag8.startDiscussion'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag11.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag11.startDiscussion'],
+ ],
+ 'discussions' => [
+ ['id' => 1, 'title' => 'Discussion with post', 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 1],
+ ],
+ 'posts' => [
+ ['id' => 1, 'discussion_id' => 1, 'user_id' => 2, 'type' => 'comment', 'content' => 'Text
'],
+ ],
+ 'discussion_tag' => [
+ ['discussion_id' => 1, 'tag_id' => 1]
+ ]
+ ]);
+ }
+
+ /**
+ * @test
+ */
+ public function user_cant_change_tags_without_setting()
+ {
+ $response = $this->send(
+ $this->request('PATCH', '/api/discussions/1', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 2]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(403, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_can_change_tags_without_setting()
+ {
+ $this->setting('allow_tag_change', '-1');
+
+ $response = $this->send(
+ $this->request('PATCH', '/api/discussions/1', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 2]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function admin_can_add_primary_tag_beyond_limit()
+ {
+ $response = $this->send(
+ $this->request('PATCH', '/api/discussions/1', [
+ 'authenticatedAs' => 1,
+ 'json' => [
+ 'data' => [
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 1],
+ ['type' => 'tags', 'id' => 2]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_cant_add_primary_tag_beyond_limit()
+ {
+ $this->setting('allow_tag_change', '-1');
+
+ $response = $this->send(
+ $this->request('PATCH', '/api/discussions/1', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 1],
+ ['type' => 'tags', 'id' => 2]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(422, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_cant_add_tag_where_can_view_but_cant_start()
+ {
+ $this->setting('allow_tag_change', '-1');
+
+ $response = $this->send(
+ $this->request('PATCH', '/api/discussions/1', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 5]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(403, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_can_add_tag_where_can_view_and_can_start()
+ {
+ $this->setting('allow_tag_change', '-1');
+
+ $response = $this->send(
+ $this->request('PATCH', '/api/discussions/1', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 1],
+ ['type' => 'tags', 'id' => 11]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_cant_add_child_tag_without_parent_tag()
+ {
+ $this->setting('allow_tag_change', '-1');
+
+ $response = $this->send(
+ $this->request('PATCH', '/api/discussions/1', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'attributes' => [
+ 'title' => 'test - too-obscure',
+ 'content' => 'predetermined content for automated testing - too-obscure',
+ ],
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 4]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(403, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_can_add_child_tag_with_parent_tag()
+ {
+ $this->setting('allow_tag_change', '-1');
+
+ $response = $this->send(
+ $this->request('PATCH', '/api/discussions/1', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 2],
+ ['type' => 'tags', 'id' => 3]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function primary_tag_required_by_default()
+ {
+ $this->setting('allow_tag_change', '-1');
+
+ $response = $this->send(
+ $this->request('PATCH', '/api/discussions/1', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 11]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(422, $response->getStatusCode());
+ }
+}
diff --git a/extensions/tags/tests/integration/visibility/TagVisibilityTest.php b/extensions/tags/tests/integration/api/tags/ListTest.php
similarity index 88%
rename from extensions/tags/tests/integration/visibility/TagVisibilityTest.php
rename to extensions/tags/tests/integration/api/tags/ListTest.php
index 30705222b..44aa9ee5b 100644
--- a/extensions/tags/tests/integration/visibility/TagVisibilityTest.php
+++ b/extensions/tags/tests/integration/api/tags/ListTest.php
@@ -7,7 +7,7 @@
* LICENSE file that was distributed with this source code.
*/
-namespace Flarum\Tags\Tests\integration\visibility;
+namespace Flarum\Tags\Tests\integration\api\tags;
use Flarum\Group\Group;
use Flarum\Tags\Tests\integration\RetrievesRepresentativeTags;
@@ -15,7 +15,7 @@ use Flarum\Testing\integration\RetrievesAuthorizedUsers;
use Flarum\Testing\integration\TestCase;
use Illuminate\Support\Arr;
-class TagVisibilityTest extends TestCase
+class ListTest extends TestCase
{
use RetrievesAuthorizedUsers;
use RetrievesRepresentativeTags;
@@ -56,7 +56,8 @@ class TagVisibilityTest extends TestCase
$data = json_decode($response->getBody()->getContents(), true)['data'];
- $this->assertEquals(['1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11'], Arr::pluck($data, 'id'));
+ $ids = Arr::pluck($data, 'id');
+ $this->assertEquals(['1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11', '12', '13', '14'], $ids);
}
/**
@@ -77,7 +78,8 @@ class TagVisibilityTest extends TestCase
// 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'));
+ $ids = Arr::pluck($data, 'id');
+ $this->assertEquals(['1', '2', '3', '4', '9', '10', '11'], $ids);
}
/**
@@ -93,7 +95,7 @@ class TagVisibilityTest extends TestCase
$data = json_decode($response->getBody()->getContents(), true)['data'];
- // Order-independent comparison
- $this->assertEquals(['1', '2', '3', '4', '9', '10'], Arr::pluck($data, 'id'));
+ $ids = Arr::pluck($data, 'id');
+ $this->assertEquals(['1', '2', '3', '4', '9', '10'], $ids);
}
}
diff --git a/extensions/tags/tests/integration/authorization/PolicyTest.php b/extensions/tags/tests/integration/authorization/GlobalPolicyTest.php
similarity index 98%
rename from extensions/tags/tests/integration/authorization/PolicyTest.php
rename to extensions/tags/tests/integration/authorization/GlobalPolicyTest.php
index 9fc750dec..a75b219a8 100644
--- a/extensions/tags/tests/integration/authorization/PolicyTest.php
+++ b/extensions/tags/tests/integration/authorization/GlobalPolicyTest.php
@@ -15,7 +15,7 @@ use Flarum\Testing\integration\RetrievesAuthorizedUsers;
use Flarum\Testing\integration\TestCase;
use Flarum\User\User;
-class PolicyTest extends TestCase
+class GlobalPolicyTest extends TestCase
{
use RetrievesAuthorizedUsers;
use RetrievesRepresentativeTags;
diff --git a/extensions/tags/tests/integration/authorization/TagPolicyTest.php b/extensions/tags/tests/integration/authorization/TagPolicyTest.php
new file mode 100644
index 000000000..42076deb4
--- /dev/null
+++ b/extensions/tags/tests/integration/authorization/TagPolicyTest.php
@@ -0,0 +1,112 @@
+extension('flarum-tags');
+
+ $this->prepareDatabase([
+ 'tags' => $this->tags(),
+ 'users' => [
+ $this->normalUser(),
+ ],
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag6.arbitraryAbility!'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag8.arbitraryAbility!'],
+ ]
+ ]);
+ }
+
+ /**
+ * @test
+ */
+ public function has_ability_when_allowed_in_restricted_tag()
+ {
+ $this->app();
+
+ $tag = Tag::find(6);
+
+ $this->assertTrue(User::find(2)->can('arbitraryAbility!', $tag));
+ }
+
+ /**
+ * @test
+ */
+ public function has_ability_in_child_when_allowed_in_top_tag_and_child()
+ {
+ $this->app();
+
+ $tag = Tag::find(8);
+
+ $this->assertTrue(User::find(2)->can('arbitraryAbility!', $tag));
+ }
+
+ /**
+ * @test
+ */
+ public function doesnt_have_ability_in_child_when_allowed_in_child_but_not_parent()
+ {
+ $this->app();
+
+ $this->database()->table('group_permission')->where('permission', 'tag6.arbitraryAbility!')->delete();
+
+ $tag = Tag::find(8);
+
+ $this->assertFalse(User::find(2)->can('arbitraryAbility!', $tag));
+ }
+
+ /**
+ * @test
+ */
+ public function nonrestricted_tag_falls_back_to_global_when_allowed()
+ {
+ $this->prepareDatabase([
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'arbitraryAbility!']
+ ]
+ ]);
+
+ $this->app();
+
+ $tag = Tag::find(1);
+
+ $this->assertTrue(User::find(2)->can('arbitraryAbility!', $tag));
+ }
+
+ /**
+ * @test
+ */
+ public function nonrestricted_tag_falls_back_to_global_when_not_allowed()
+ {
+ $this->app();
+
+ $tag = Tag::find(1);
+
+ $this->assertFalse(User::find(2)->can('arbitraryAbility!', $tag));
+ }
+}
diff --git a/extensions/tags/tests/integration/visibility/DiscussionVisibilityTest.php b/extensions/tags/tests/integration/visibility/DiscussionVisibilityTest.php
index 77039dbbf..f47a5d2cf 100644
--- a/extensions/tags/tests/integration/visibility/DiscussionVisibilityTest.php
+++ b/extensions/tags/tests/integration/visibility/DiscussionVisibilityTest.php
@@ -7,13 +7,15 @@
* LICENSE file that was distributed with this source code.
*/
-namespace Flarum\Tags\Tests\integration\visibility;
+namespace Flarum\Tags\Tests\integration\api\discussions;
-use Carbon\Carbon;
+use Flarum\Discussion\Discussion;
use Flarum\Group\Group;
use Flarum\Tags\Tests\integration\RetrievesRepresentativeTags;
use Flarum\Testing\integration\RetrievesAuthorizedUsers;
use Flarum\Testing\integration\TestCase;
+use Flarum\User\Guest;
+use Flarum\User\User;
use Illuminate\Support\Arr;
class DiscussionVisibilityTest extends TestCase
@@ -31,19 +33,36 @@ class DiscussionVisibilityTest extends TestCase
$this->extension('flarum-tags');
$this->prepareDatabase([
+ 'tags' => $this->tags(),
+ 'users' => [
+ $this->normalUser(),
+ ],
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag5.arbitraryAbility'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag8.arbitraryAbility'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag11.arbitraryAbility'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag13.arbitraryAbility'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag14.arbitraryAbility'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'arbitraryAbility'],
+ ['group_id' => Group::GUEST_ID, 'permission' => 'arbitraryAbility']
+ ],
'discussions' => [
- ['id' => 1, 'title' => 'no tags', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
- ['id' => 2, 'title' => 'open tags', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
- ['id' => 3, 'title' => 'open tag, restricted child tag', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
- ['id' => 4, 'title' => 'open tag, one restricted secondary tag', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
- ['id' => 5, 'title' => 'all closed', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 1, 'title' => 'no tags', 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 2, 'title' => 'open tags', 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 3, 'title' => 'open tag, restricted child tag', 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 4, 'title' => 'open tag, one restricted secondary tag', 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 5, 'title' => 'all closed', 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 6, 'title' => 'closed parent, open child tag', 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 7, 'title' => 'one closed primary tag', 'user_id' => 1, 'comment_count' => 1],
],
'posts' => [
- ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => ''],
- ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => ''],
- ['id' => 3, 'discussion_id' => 3, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => ''],
- ['id' => 4, 'discussion_id' => 4, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => ''],
- ['id' => 5, 'discussion_id' => 5, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 1, 'discussion_id' => 1, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 2, 'discussion_id' => 2, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 3, 'discussion_id' => 3, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 4, 'discussion_id' => 4, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 5, 'discussion_id' => 5, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 6, 'discussion_id' => 6, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 7, 'discussion_id' => 7, 'user_id' => 1, 'type' => 'comment', 'content' => ''],
],
'discussion_tag' => [
['discussion_id' => 2, 'tag_id' => 1],
@@ -54,16 +73,10 @@ class DiscussionVisibilityTest extends TestCase
['discussion_id' => 5, 'tag_id' => 6],
['discussion_id' => 5, 'tag_id' => 7],
['discussion_id' => 5, 'tag_id' => 8],
+ ['discussion_id' => 6, 'tag_id' => 12],
+ ['discussion_id' => 6, 'tag_id' => 13],
+ ['discussion_id' => 7, 'tag_id' => 14],
],
- 'tags' => $this->tags(),
- 'users' => [
- $this->normalUser(),
- ],
- 'group_permission' => [
- ['group_id' => Group::MEMBER_ID, 'permission' => 'tag5.viewDiscussions'],
- ['group_id' => Group::MEMBER_ID, 'permission' => 'tag8.viewDiscussions'],
- ['group_id' => Group::MEMBER_ID, 'permission' => 'tag11.viewDiscussions']
- ]
]);
}
@@ -72,18 +85,13 @@ class DiscussionVisibilityTest extends TestCase
*/
public function admin_sees_all()
{
- $response = $this->send(
- $this->request('GET', '/api/discussions', [
- 'authenticatedAs' => 1
- ])
- );
+ $this->app();
- $this->assertEquals(200, $response->getStatusCode());
+ $user = User::find(1);
+ $discussions = Discussion::whereVisibleTo($user, 'arbitraryAbility')->get();
- $data = json_decode($response->getBody()->getContents(), true)['data'];
-
- $ids = Arr::pluck($data, 'id');
- $this->assertEqualsCanonicalizing(['1', '2', '3', '4', '5'], $ids);
+ $ids = Arr::pluck($discussions, 'id');
+ $this->assertEqualsCanonicalizing([1, 2, 3, 4, 5, 6, 7], $ids);
}
/**
@@ -91,37 +99,54 @@ class DiscussionVisibilityTest extends TestCase
*/
public function user_sees_where_allowed()
{
- $response = $this->send(
- $this->request('GET', '/api/discussions', [
- 'authenticatedAs' => 2
- ])
- );
+ $this->app();
- $this->assertEquals(200, $response->getStatusCode());
+ $user = User::find(2);
+ $discussions = Discussion::whereVisibleTo($user, 'arbitraryAbility')->get();
- $data = json_decode($response->getBody()->getContents(), true)['data'];
-
- // 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.
- $ids = Arr::pluck($data, 'id');
- $this->assertEqualsCanonicalizing(['1', '2', '3', '4'], $ids);
+ $ids = Arr::pluck($discussions, 'id');
+ $this->assertEqualsCanonicalizing([1, 2, 3, 4, 7], $ids);
}
/**
* @test
*/
- public function guest_cant_see_restricted_or_children_of_restricted()
+ public function user_sees_only_in_restricted_tags_without_global_perm()
{
- $response = $this->send(
- $this->request('GET', '/api/discussions')
- );
+ $this->database()->table('group_permission')->where('permission', 'arbitraryAbility')->delete();
- $this->assertEquals(200, $response->getStatusCode());
+ $user = User::find(2);
+ $discussions = Discussion::whereVisibleTo($user, 'arbitraryAbility')->get();
- $data = json_decode($response->getBody()->getContents(), true)['data'];
+ $ids = Arr::pluck($discussions, 'id');
+ $this->assertEqualsCanonicalizing([7], $ids);
+ }
- $ids = Arr::pluck($data, 'id');
- $this->assertEqualsCanonicalizing(['1', '2'], $ids);
+ /**
+ * @test
+ */
+ public function guest_can_see_where_allowed()
+ {
+ $this->app();
+
+ $user = new Guest();
+ $discussions = Discussion::whereVisibleTo($user, 'arbitraryAbility')->get();
+
+ $ids = Arr::pluck($discussions, 'id');
+ $this->assertEqualsCanonicalizing([1, 2], $ids);
+ }
+
+ /**
+ * @test
+ */
+ public function guest_cant_see_without_global_perm()
+ {
+ $this->database()->table('group_permission')->where('permission', 'arbitraryAbility')->delete();
+
+ $user = new Guest();
+ $discussions = Discussion::whereVisibleTo($user, 'arbitraryAbility')->get();
+
+ $ids = Arr::pluck($discussions, 'id');
+ $this->assertEqualsCanonicalizing([], $ids);
}
}