Fix discussions hidden from all users including admins (#140)

* test: Start by adding tests for this case scenario
* fix: All discussions hidden from everyone when minSecondary > 0 and no secondary tags
* test: Test the scenario for primary tags as well
* fix: Properly fix the general case of more tags required than available
* Adjust to only check with the `viewForum` ability
* Test Cases for private discussions
This commit is contained in:
Sami Mazouz 2021-09-20 16:19:43 +01:00 committed by GitHub
parent fb365c98e5
commit 2f37a8bd58
2 changed files with 140 additions and 4 deletions

View File

@ -44,15 +44,31 @@ class GlobalPolicy extends AbstractPolicy
if (in_array($ability, ['viewForum', 'startDiscussion'])) {
if (! isset($enoughPrimary[$actor->id][$ability])) {
$enoughPrimary[$actor->id][$ability] = Tag::whereHasPermission($actor, $ability)
$primaryTagsWhereNeedsPermission = $this->settings->get('flarum-tags.min_primary_tags');
$primaryTagsWhereHasPermission = Tag::whereHasPermission($actor, $ability)
->where('tags.position', '!=', null)
->count() >= $this->settings->get('flarum-tags.min_primary_tags');
->count();
if ($ability === 'viewForum') {
$primaryTagsCount = Tag::query()->where('position', '!=', null)->count();
$enoughPrimary[$actor->id][$ability] = $primaryTagsWhereHasPermission >= min($primaryTagsCount, $primaryTagsWhereNeedsPermission);
} else {
$enoughPrimary[$actor->id][$ability] = $primaryTagsWhereHasPermission >= $primaryTagsWhereNeedsPermission;
}
}
if (! isset($enoughSecondary[$actor->id][$ability])) {
$enoughSecondary[$actor->id][$ability] = Tag::whereHasPermission($actor, $ability)
$secondaryTagsWhereNeedsPermission = $this->settings->get('flarum-tags.min_secondary_tags');
$secondaryTagsWhereHasPermission = Tag::whereHasPermission($actor, $ability)
->where('tags.position', '=', null)
->count() >= $this->settings->get('flarum-tags.min_secondary_tags');
->count();
if ($ability === 'viewForum') {
$secondaryTagsCount = Tag::query()->where(['position' => null, 'parent_id' => null])->count();
$enoughSecondary[$actor->id][$ability] = $secondaryTagsWhereHasPermission >= min($secondaryTagsCount, $secondaryTagsWhereNeedsPermission);
} else {
$enoughSecondary[$actor->id][$ability] = $secondaryTagsWhereHasPermission >= $secondaryTagsWhereNeedsPermission;
}
}
if ($enoughPrimary[$actor->id][$ability] && $enoughSecondary[$actor->id][$ability]) {

View File

@ -47,6 +47,12 @@ class ListTest extends TestCase
['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' => 'private, no tags', 'user_id' => 1, 'comment_count' => 1, 'is_private' => 1],
['id' => 8, 'title' => 'private, open tags', 'user_id' => 1, 'comment_count' => 1, 'is_private' => 1],
['id' => 9, 'title' => 'private, open tag, restricted child tag', 'user_id' => 1, 'comment_count' => 1, 'is_private' => 1],
['id' => 10, 'title' => 'private, open tag, one restricted secondary tag', 'user_id' => 1, 'comment_count' => 1, 'is_private' => 1],
['id' => 11, 'title' => 'private, all closed', 'user_id' => 1, 'comment_count' => 1, 'is_private' => 1],
['id' => 12, 'title' => 'private, closed parent, open child tag', 'user_id' => 1, 'comment_count' => 1, 'is_private' => 1],
],
'posts' => [
['id' => 1, 'discussion_id' => 1, 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p></p></t>'],
@ -55,18 +61,34 @@ class ListTest extends TestCase
['id' => 4, 'discussion_id' => 4, 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p></p></t>'],
['id' => 5, 'discussion_id' => 5, 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p></p></t>'],
['id' => 6, 'discussion_id' => 6, 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p></p></t>'],
['id' => 7, 'discussion_id' => 7, 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p></p></t>'],
['id' => 8, 'discussion_id' => 8, 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p></p></t>'],
['id' => 9, 'discussion_id' => 9, 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p></p></t>'],
['id' => 10, 'discussion_id' => 10, 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p></p></t>'],
['id' => 11, 'discussion_id' => 11, 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p></p></t>'],
['id' => 12, 'discussion_id' => 12, 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p></p></t>'],
],
'discussion_tag' => [
['discussion_id' => 2, 'tag_id' => 1],
['discussion_id' => 8, 'tag_id' => 1],
['discussion_id' => 3, 'tag_id' => 2],
['discussion_id' => 3, 'tag_id' => 5],
['discussion_id' => 9, 'tag_id' => 2],
['discussion_id' => 9, 'tag_id' => 5],
['discussion_id' => 4, 'tag_id' => 1],
['discussion_id' => 4, 'tag_id' => 11],
['discussion_id' => 10, 'tag_id' => 1],
['discussion_id' => 10, 'tag_id' => 11],
['discussion_id' => 5, 'tag_id' => 6],
['discussion_id' => 5, 'tag_id' => 7],
['discussion_id' => 5, 'tag_id' => 8],
['discussion_id' => 11, 'tag_id' => 6],
['discussion_id' => 11, 'tag_id' => 7],
['discussion_id' => 11, 'tag_id' => 8],
['discussion_id' => 6, 'tag_id' => 12],
['discussion_id' => 6, 'tag_id' => 13],
['discussion_id' => 12, 'tag_id' => 12],
['discussion_id' => 12, 'tag_id' => 13],
],
]);
}
@ -125,4 +147,102 @@ class ListTest extends TestCase
$ids = Arr::pluck($data, 'id');
$this->assertEqualsCanonicalizing(['1', '2'], $ids);
}
/**
* @test
*/
public function admin_can_see_where_allowed_when_more_primary_tags_are_required_than_available()
{
$this->actor_can_see_where_allowed_when_more_primary_tags_are_required_than_available(1, ['1', '2', '3', '4', '5', '6']);
}
/**
* @test
*/
public function user_can_see_where_allowed_when_more_primary_tags_are_required_than_available()
{
$this->actor_can_see_where_allowed_when_more_primary_tags_are_required_than_available(2, ['1', '2', '3', '4']);
}
/**
* @test
*/
public function guest_can_see_where_allowed_when_more_primary_tags_are_required_than_available()
{
$this->actor_can_see_where_allowed_when_more_primary_tags_are_required_than_available(0, ['1', '2']);
}
/**
* @test
*/
public function admin_can_see_where_allowed_when_more_secondary_tags_are_required_than_available()
{
$this->actor_can_see_where_allowed_when_more_secondary_tags_are_required_than_available(1, ['1', '2', '3', '4', '5', '6']);
}
/**
* @test
*/
public function user_can_see_where_allowed_when_more_secondary_tags_are_required_than_available()
{
$this->actor_can_see_where_allowed_when_more_secondary_tags_are_required_than_available(2, ['1', '2', '3', '4']);
}
/**
* @test
*/
public function guest_can_see_where_allowed_when_more_secondary_tags_are_required_than_available()
{
$this->actor_can_see_where_allowed_when_more_secondary_tags_are_required_than_available(0, ['1', '2']);
}
public function actor_can_see_where_allowed_when_more_primary_tags_are_required_than_available(int $actorId, array $expectedDiscussions)
{
$this->setting('flarum-tags.min_primary_tags', 100);
$this->database()->table('tags')
->where('position', '!=', null)
->update(['position' => null]);
if ($actorId) {
$reqParams = [
'authenticatedAs' => $actorId
];
}
$response = $this->send(
$this->request('GET', '/api/discussions', $reqParams ?? [])
);
$this->assertEquals(200, $response->getStatusCode());
$data = json_decode($response->getBody()->getContents(), true)['data'];
$ids = Arr::pluck($data, 'id');
$this->assertEqualsCanonicalizing($expectedDiscussions, $ids);
}
public function actor_can_see_where_allowed_when_more_secondary_tags_are_required_than_available(int $actorId, array $expectedDiscussions)
{
$this->setting('flarum-tags.min_secondary_tags', 1);
$this->database()->table('tags')
->where(['position' => null, 'parent_id' => null])
->update(['position' => 200]);
if ($actorId) {
$reqParams = [
'authenticatedAs' => $actorId
];
}
$response = $this->send(
$this->request('GET', '/api/discussions', $reqParams ?? [])
);
$this->assertEquals(200, $response->getStatusCode());
$data = json_decode($response->getBody()->getContents(), true)['data'];
$ids = Arr::pluck($data, 'id');
$this->assertEqualsCanonicalizing($expectedDiscussions, $ids);
}
}