Skip to content

Commit ed48409

Browse files
committed
fix(tags): exclude bypassTagCounts from DiscussionPolicy catch-all
The catch-all `can()` method in DiscussionPolicy intercepted every discussion ability, including the `bypassTagCounts` meta-permission, and checked for a per-tag variant (e.g. `tag6.discussion.bypassTagCounts`) that never exists. This caused the permission check to always deny non-admin users, even when they had been explicitly granted `bypassTagCounts`, whenever the discussion was in a restricted tag. Fixes #4537
1 parent 8e70ea4 commit ed48409

3 files changed

Lines changed: 55 additions & 0 deletions

File tree

extensions/tags/src/Access/DiscussionPolicy.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ public function __construct(SettingsRepositoryInterface $settings)
3838
*/
3939
public function can(User $actor, $ability, Discussion $discussion)
4040
{
41+
// Meta-permissions that govern tag validation logic (not tag-scoped
42+
// access) must not be intercepted by this catch-all. They have no
43+
// per-tag variant and will always deny for non-admins if left in.
44+
if ($ability === 'bypassTagCounts') {
45+
return;
46+
}
47+
4148
// Wrap all discussion permission checks with some logic pertaining to
4249
// the discussion's tags. If the discussion has a tag that has been
4350
// restricted, the user must have the permission for that tag.

extensions/tags/tests/integration/api/discussions/CreateTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,4 +325,5 @@ public function primary_tag_required_by_default()
325325

326326
$this->assertEquals(422, $response->getStatusCode());
327327
}
328+
328329
}

extensions/tags/tests/integration/api/discussions/UpdateTest.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,4 +298,51 @@ public function primary_tag_required_by_default()
298298

299299
$this->assertEquals(422, $response->getStatusCode());
300300
}
301+
302+
/**
303+
* @test
304+
* Non-admin with bypassTagCounts cannot exceed the primary tag limit when
305+
* the discussion is in a restricted tag. The DiscussionPolicy catch-all
306+
* must not intercept the bypassTagCounts meta-permission check.
307+
*/
308+
public function user_with_bypass_permission_can_exceed_primary_tag_limit_in_restricted_tag()
309+
{
310+
$this->setting('allow_tag_change', '-1');
311+
312+
$this->prepareDatabase([
313+
'group_permission' => [
314+
['group_id' => Group::MEMBER_ID, 'permission' => 'bypassTagCounts'],
315+
['group_id' => Group::MEMBER_ID, 'permission' => 'tag6.viewForum'],
316+
['group_id' => Group::MEMBER_ID, 'permission' => 'tag6.startDiscussion'],
317+
['group_id' => Group::MEMBER_ID, 'permission' => 'tag6.discussion.tag'],
318+
['group_id' => Group::MEMBER_ID, 'permission' => 'tag12.viewForum'],
319+
['group_id' => Group::MEMBER_ID, 'permission' => 'tag12.startDiscussion'],
320+
['group_id' => Group::MEMBER_ID, 'permission' => 'tag12.discussion.tag'],
321+
],
322+
'discussion_tag' => [
323+
['discussion_id' => 1, 'tag_id' => 6],
324+
],
325+
]);
326+
327+
// max_primary_tags defaults to 1; the user sets 2 restricted primary tags
328+
$response = $this->send(
329+
$this->request('PATCH', '/api/discussions/1', [
330+
'authenticatedAs' => 2,
331+
'json' => [
332+
'data' => [
333+
'relationships' => [
334+
'tags' => [
335+
'data' => [
336+
['type' => 'tags', 'id' => 6],
337+
['type' => 'tags', 'id' => 12],
338+
]
339+
]
340+
]
341+
],
342+
],
343+
])
344+
);
345+
346+
$this->assertEquals(200, $response->getStatusCode());
347+
}
301348
}

0 commit comments

Comments
 (0)