flarum / flarum/issue-archive

[Tags] Cannot use use model-less ::class gate on discussion and tags when tags extensions is enabled

Open
#87 0 comments 0 reactions 0 assignees View on GitHub
type/bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.