diff --git a/src/User/Access/AbstractPolicy.php b/src/User/Access/AbstractPolicy.php index ef32c539f4..3b4579d744 100644 --- a/src/User/Access/AbstractPolicy.php +++ b/src/User/Access/AbstractPolicy.php @@ -43,13 +43,13 @@ protected function forceDeny() * @param User $user * @param string $ability * @param $instance - * @return bool|void + * @return string|void */ public function checkAbility(User $actor, string $ability, $instance) { // If a specific method for this ability is defined, // call that and return any non-null results if (method_exists($this, $ability)) { - $result = call_user_func_array([$this, $ability], [$actor, $instance]); + $result = $this->sanitizeResult(call_user_func_array([$this, $ability], [$actor, $instance])); if (! is_null($result)) { return $result; @@ -58,7 +58,31 @@ public function checkAbility(User $actor, string $ability, $instance) // If a "total access" method is defined, try that. if (method_exists($this, 'can')) { - return call_user_func_array([$this, 'can'], [$actor, $ability, $instance]); + return $this->sanitizeResult(call_user_func_array([$this, 'can'], [$actor, $ability, $instance])); } } + + /** + * Allows `true` to be used in place of `->allow()`, and `false` instead of `->deny()` + * This allows more concise and intuitive code, by returning boolean statements:. + * + * WITHOUT THIS: + * `return SOME_BOOLEAN_LOGIC ? $this->allow() : $this->deny(); + * + * WITH THIS: + * `return SOME_BOOLEAN_LOGIC; + * + * @param mixed $result + * @return string|void + */ + public function sanitizeResult($result) + { + if ($result === true) { + return $this->allow(); + } elseif ($result === false) { + return $this->deny(); + } + + return $result; + } } diff --git a/tests/unit/User/AbstractPolicyTest.php b/tests/unit/User/AbstractPolicyTest.php index d2ba2efacd..db573343ce 100644 --- a/tests/unit/User/AbstractPolicyTest.php +++ b/tests/unit/User/AbstractPolicyTest.php @@ -9,9 +9,8 @@ namespace Flarum\Tests\unit\User; -use Flarum\Event\GetPermission; use Flarum\Tests\unit\TestCase; -use Flarum\User\AbstractPolicy; +use Flarum\User\Access\AbstractPolicy; use Flarum\User\User; use Illuminate\Events\Dispatcher; use Mockery as m; @@ -19,32 +18,50 @@ class AbstractPolicyTest extends TestCase { private $policy; - private $dispatcher; + /** + * @inheritDoc + */ protected function setUp(): void { $this->policy = m::mock(CustomUserPolicy::class)->makePartial(); - $this->dispatcher = new Dispatcher(); - $this->dispatcher->subscribe($this->policy); - User::setEventDispatcher($this->dispatcher); + User::setEventDispatcher(new Dispatcher()); } - public function test_policy_can_be_called_with_object() + /** + * @inheritDoc + */ + protected function tearDown(): void { - $this->policy->shouldReceive('edit')->andReturn(true); + m::close(); + } - $allowed = $this->dispatcher->until(new GetPermission(new User(), 'edit', new User())); + public function test_policy_can_be_called_with_object() + { + $allowed = $this->policy->checkAbility(new User(), 'create', new User()); - $this->assertTrue($allowed); + $this->assertEquals(AbstractPolicy::ALLOW, $allowed); } public function test_policy_can_be_called_with_class() { - $this->policy->shouldReceive('create')->andReturn(true); + $allowed = $this->policy->checkAbility(new User(), 'edit', User::class); + + $this->assertEquals(AbstractPolicy::DENY, $allowed); + } + + public function test_policy_converts_true_to_ALLOW() + { + $allowed = $this->policy->checkAbility(new User(), 'somethingRandom', User::class); + + $this->assertEquals(AbstractPolicy::ALLOW, $allowed); + } - $allowed = $this->dispatcher->until(new GetPermission(new User(), 'create', User::class)); + public function test_policy_converts_false_to_DENY() + { + $allowed = $this->policy->checkAbility(new User(), 'somethingElseRandom', User::class); - $this->assertTrue($allowed); + $this->assertEquals(AbstractPolicy::DENY, $allowed); } } @@ -54,11 +71,21 @@ class CustomUserPolicy extends AbstractPolicy public function create(User $actor) { - return true; + return $this->allow(); } - public function edit(User $actor, User $user) + public function edit(User $actor, $target) + { + return $this->deny(); + } + + public function somethingRandom(User $actor, $target) { return true; } + + public function somethingElseRandom(User $actor, $target) + { + return false; + } }