[Tags] Cannot use use model-less ::class gate on discussion and tags when tags extensions is enabled
- Dominant language
- No language data
- Stars
- 0
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
## Bug Report
**Current Behavior**
Extensions that use model-less abilities on the Discuss and Tag policies fail when the Tags extension is enabled.
The ability to use model-less ::class abilities on policies was added in flarum/framework#1977 . It's correctly unit-tested in https://github.com/flarum/core/blob/master/tests/unit/User/AbstractPolicyTest.php but there are no unit tests for each policy specifically, and no tests with core extensions enabled.
**Steps to Reproduce**
Unit tests showing the issue.
Works:
```php
extend((new Policy())->modelPolicy(Discussion::class, DiscussionPolicy::class));
}
/**
* @test
*/
public function callPolicy()
{
$this->app();
$user = User::findOrFail(1);
$this->assertTrue($user->can('createSpecial', Discussion::class));
}
}
class DiscussionPolicy extends AbstractPolicy
{
public function createSpecial(User $actor)
{
return $this->allow();
}
}
```
Doesn't work (added extension tags):
```php
extension('flarum-tags');
$this->extend((new Policy())->modelPolicy(Discussion::class, DiscussionPolicy::class));
}
/**
* @test
*/
public function callPolicy()
{
$this->app();
$user = User::findOrFail(1);
$this->assertTrue($user->can('createSpecial', Discussion::class));
}
}
class DiscussionPolicy extends AbstractPolicy
{
public function createSpecial(User $actor)
{
return $this->allow();
}
}
```
Error:
```
There was 1 error:
1) Tests\integration\ModelLessPolicyTest::callPolicy
TypeError: Argument 3 passed to Flarum\Tags\Access\DiscussionPolicy::can() must be an instance of Flarum\Discussion\Discussion, string given
/[...]/vendor/flarum/tags/src/Access/DiscussionPolicy.php:39
/[...]/vendor/flarum/core/src/User/Access/AbstractPolicy.php:61
/[...]/vendor/flarum/core/src/User/Access/Gate.php:78
/[...]/vendor/flarum/core/src/User/User.php:775
/[...]/tests/integration/ModelLessPolicyTest.php:30
```
**Expected Behavior**
A clear and concise description of what you expected to happen.
**Screenshots**
If applicable, add screenshots to help explain your problem.
**Environment**
- Flarum version: 1.0.1
- Hosting environment: local
- PHP version: 7.4
**Possible Solution**
The problem is that the third parameter to `DiscussionPolicy::can` was type-hinted as `Discussion` in the Tags extension when it will be `string` for any call to a model-less ability:
https://github.com/flarum/tags/blob/453169d8093feb1ad523cfdbdcf47c3b3a391cab/src/Access/DiscussionPolicy.php#L39
The tag policy seems to be affected by the same issue:
https://github.com/flarum/tags/blob/453169d8093feb1ad523cfdbdcf47c3b3a391cab/src/Access/TagPolicy.php#L18
The PHP type-hint should probably be removed. The PHPdoc could be updated to `Discussion|string` (respectively, `Tag|string`). Then we should probably return early if the third parameter isn't a model, something like `if (!($discussion instanceof Discussion)) { return; }`
**Additional Context**
As a workaround I have passed `new Discussion` to the gate but that's not ideal since `::class` is supposed to be supported.
I have not checked if other core extensions are impacted by this.
This is not a regression, it seems to have been broken since the start. The `Discussion` type-hint on the third parameter has been there for years. There must just have been no-one trying to use a ::class ability on the Discussion policy yet.
Another reason this might not have manifested earlier is because of the changes to the policies we did a few versions ago. Previously the `can()` method(s) would not have been called if an ability method existed with that name. But now I think we always call all the ability and all the `can` methods and then compute the value based on all responses.
Contributor guide
Research direction
Start with src/Access/DiscussionPolicy.php and src/Access/TagPolicy.php, then run the reproduced tests in tests/integration/ModelLessPolicyTest.php and review tests/unit/User/AbstractPolicyTest.php. Verify model-less abilities work with the Tags extension enabled for both policies, while model-based policy checks continue to pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- authorization
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100