Policy Extender and Tests (#2461)

Policy application has also been refactored, so that policies return one of `allow`, `deny`, `forceAllow`, `forceDeny`. The result of a set of policies is no longer the first non-null result, but rather the highest priority result (forceDeny > forceAllow > deny > allow, so if a single forceDeny is present, that beats out all other returned results). This removes order in which extensions boot as a factor.
This commit is contained in:
Alexander Skvortsov 2020-12-08 19:10:06 -05:00 committed by GitHub
parent 8901073d12
commit d1dfa758e4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 597 additions and 125 deletions

View File

@ -7,38 +7,28 @@
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\Discussion;
namespace Flarum\Discussion\Access;
use Flarum\Discussion\Discussion;
use Flarum\Settings\SettingsRepositoryInterface;
use Flarum\User\AbstractPolicy;
use Flarum\User\Access\AbstractPolicy;
use Flarum\User\User;
use Illuminate\Contracts\Events\Dispatcher;
class DiscussionPolicy extends AbstractPolicy
{
/**
* {@inheritdoc}
*/
protected $model = Discussion::class;
/**
* @var SettingsRepositoryInterface
*/
protected $settings;
/**
* @var Dispatcher
*/
protected $events;
/**
* @param SettingsRepositoryInterface $settings
* @param Dispatcher $events
*/
public function __construct(SettingsRepositoryInterface $settings, Dispatcher $events)
public function __construct(SettingsRepositoryInterface $settings)
{
$this->settings = $settings;
$this->events = $events;
}
/**
@ -49,7 +39,7 @@ class DiscussionPolicy extends AbstractPolicy
public function can(User $actor, $ability)
{
if ($actor->hasPermission('discussion.'.$ability)) {
return true;
return $this->allow();
}
}
@ -66,7 +56,7 @@ class DiscussionPolicy extends AbstractPolicy
if ($allowRenaming === '-1'
|| ($allowRenaming === 'reply' && $discussion->participant_count <= 1)
|| ($discussion->created_at->diffInMinutes() < $allowRenaming)) {
return true;
return $this->allow();
}
}
}
@ -83,7 +73,7 @@ class DiscussionPolicy extends AbstractPolicy
&& (! $discussion->hidden_at || $discussion->hidden_user_id == $actor->id)
&& $actor->can('reply', $discussion)
) {
return true;
return $this->allow();
}
}
}

View File

@ -23,7 +23,6 @@ class DiscussionServiceProvider extends AbstractServiceProvider
$events = $this->app->make('events');
$events->subscribe(DiscussionMetadataUpdater::class);
$events->subscribe(DiscussionPolicy::class);
$events->listen(
Renamed::class,

View File

@ -11,6 +11,9 @@ namespace Flarum\Event;
use Flarum\User\User;
/**
* @deprecated beta 15, remove beta 16
*/
class GetPermission
{
/**

69
src/Extend/Policy.php Normal file
View File

@ -0,0 +1,69 @@
<?php
/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\Extend;
use Flarum\Extension\Extension;
use Flarum\User\Access\AbstractPolicy;
use Illuminate\Contracts\Container\Container;
class Policy implements ExtenderInterface
{
private $globalPolicies = [];
private $modelPolicies = [];
/**
* Add a custom policy for when an ability check is ran without a model instance.
*
* @param string $policy ::class attribute of policy class, which must extend Flarum\User\AbstractPolicy
*/
public function globalPolicy(string $policy)
{
$this->globalPolicies[] = $policy;
return $this;
}
/**
* Add a custom policy for when an ability check is ran on an instance of a model.
*
* @param string $modelClass The ::class attribute of the model you are applying policies to.
* This model should extend from \Flarum\Database\AbstractModel.
* @param string $policy ::class attribute of policy class, which must extend Flarum\User\AbstractPolicy
*/
public function modelPolicy(string $modelClass, string $policy)
{
if (! array_key_exists($modelClass, $this->modelPolicies)) {
$this->modelPolicies[$modelClass] = [];
}
$this->modelPolicies[$modelClass][] = $policy;
return $this;
}
public function extend(Container $container, Extension $extension = null)
{
$container->extend('flarum.policies', function ($existingPolicies) {
foreach ($this->modelPolicies as $modelClass => $addPolicies) {
if (! array_key_exists($modelClass, $existingPolicies)) {
$existingPolicies[$modelClass] = [];
}
foreach ($addPolicies as $policy) {
$existingPolicies[$modelClass][] = $policy;
}
}
$existingPolicies[AbstractPolicy::GLOBAL] = array_merge($existingPolicies[AbstractPolicy::GLOBAL], $this->globalPolicies);
return $existingPolicies;
});
}
}

View File

@ -7,18 +7,13 @@
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\Group;
namespace Flarum\Group\Access;
use Flarum\User\AbstractPolicy;
use Flarum\User\Access\AbstractPolicy;
use Flarum\User\User;
class GroupPolicy extends AbstractPolicy
{
/**
* {@inheritdoc}
*/
protected $model = Group::class;
/**
* @param User $actor
* @param string $ability
@ -27,7 +22,7 @@ class GroupPolicy extends AbstractPolicy
public function can(User $actor, $ability)
{
if ($actor->hasPermission('group.'.$ability)) {
return true;
return $this->allow();
}
}
}

View File

@ -19,9 +19,6 @@ class GroupServiceProvider extends AbstractServiceProvider
*/
public function boot()
{
$events = $this->app->make('events');
$events->subscribe(GroupPolicy::class);
Group::registerVisibilityScoper(new ScopeGroupVisibility(), 'view');
}
}

View File

@ -7,39 +7,27 @@
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\Post;
namespace Flarum\Post\Access;
use Carbon\Carbon;
use Flarum\Post\Post;
use Flarum\Settings\SettingsRepositoryInterface;
use Flarum\User\AbstractPolicy;
use Flarum\User\Access\AbstractPolicy;
use Flarum\User\User;
use Illuminate\Contracts\Events\Dispatcher;
class PostPolicy extends AbstractPolicy
{
/**
* {@inheritdoc}
*/
protected $model = Post::class;
/**
* @var SettingsRepositoryInterface
*/
protected $settings;
/**
* @var Dispatcher
*/
protected $events;
/**
* @param SettingsRepositoryInterface $settings
* @param Dispatcher $events
*/
public function __construct(SettingsRepositoryInterface $settings, Dispatcher $events)
public function __construct(SettingsRepositoryInterface $settings)
{
$this->settings = $settings;
$this->events = $events;
}
/**
@ -51,7 +39,7 @@ class PostPolicy extends AbstractPolicy
public function can(User $actor, $ability, Post $post)
{
if ($actor->can($ability.'Posts', $post->discussion)) {
return true;
return $this->allow();
}
}
@ -71,7 +59,7 @@ class PostPolicy extends AbstractPolicy
if ($allowEditing === '-1'
|| ($allowEditing === 'reply' && $post->number >= $post->discussion->last_post_number)
|| ($post->created_at->diffInMinutes(new Carbon) < $allowEditing)) {
return true;
return $this->allow();
}
}
}

View File

@ -51,9 +51,6 @@ class PostServiceProvider extends AbstractServiceProvider
$this->setPostTypes();
$events = $this->app->make('events');
$events->subscribe(PostPolicy::class);
Post::registerVisibilityScoper(new ScopePostVisibility(), 'view');
}

View File

@ -0,0 +1,64 @@
<?php
/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\User\Access;
use Flarum\User\User;
abstract class AbstractPolicy
{
public const GLOBAL = 'GLOBAL';
public const ALLOW = 'ALLOW';
public const DENY = 'DENY';
public const FORCE_ALLOW = 'FORCE_ALLOW';
public const FORCE_DENY = 'FORCE_DENY';
protected function allow()
{
return static::ALLOW;
}
protected function deny()
{
return static::DENY;
}
protected function forceAllow()
{
return static::FORCE_ALLOW;
}
protected function forceDeny()
{
return static::FORCE_DENY;
}
/**
* @param User $user
* @param string $ability
* @param $instance
* @return bool|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]);
if (! is_null($result)) {
return $result;
}
}
// If a "total access" method is defined, try that.
if (method_exists($this, 'can')) {
return call_user_func_array([$this, 'can'], [$actor, $ability, $instance]);
}
}
}

131
src/User/Access/Gate.php Normal file
View File

@ -0,0 +1,131 @@
<?php
/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\User\Access;
use Flarum\Database\AbstractModel;
use Flarum\Event\GetPermission;
use Flarum\User\User;
use Illuminate\Contracts\Container\Container;
use Illuminate\Contracts\Events\Dispatcher;
use Illuminate\Support\Arr;
class Gate
{
protected const EVALUATION_CRITERIA_PRIORITY = [
AbstractPolicy::FORCE_DENY => false,
AbstractPolicy::FORCE_ALLOW => true,
AbstractPolicy::DENY => false,
AbstractPolicy::ALLOW => true,
];
/**
* @var Container
*/
protected $container;
/**
* @var Dispatcher
*/
protected $events;
/**
* @var array
*/
protected $policyClasses;
/**
* @var array
*/
protected $policies;
/**
* @param Dispatcher $events
*/
public function __construct(Container $container, Dispatcher $events, array $policyClasses)
{
$this->container = $container;
$this->events = $events;
$this->policyClasses = $policyClasses;
}
/**
* Determine if the given ability should be granted for the current user.
*
* @param User $actor
* @param string $ability
* @param string|AbstractModel $model
* @return bool
*/
public function allows(User $actor, string $ability, $model): bool
{
$results = [];
$appliedPolicies = [];
if ($model) {
$modelClasses = is_string($model) ? [$model] : array_merge(class_parents(($model)), [get_class($model)]);
foreach ($modelClasses as $class) {
$appliedPolicies = array_merge($appliedPolicies, $this->getPolicies($class));
}
} else {
$appliedPolicies = $this->getPolicies(AbstractPolicy::GLOBAL);
}
foreach ($appliedPolicies as $policy) {
$results[] = $policy->checkAbility($actor, $ability, $model);
}
foreach (static::EVALUATION_CRITERIA_PRIORITY as $criteria => $decision) {
if (in_array($criteria, $results, true)) {
return $decision;
}
}
// START OLD DEPRECATED SYSTEM
// Fire an event so that core and extension modelPolicies can hook into
// this permission query and explicitly grant or deny the
// permission.
$allowed = $this->events->until(
new GetPermission($actor, $ability, $model)
);
if (! is_null($allowed)) {
return $allowed;
}
// END OLD DEPRECATED SYSTEM
// If no policy covered this permission query, we will only grant
// the permission if the actor's groups have it. Otherwise, we will
// not allow the user to perform this action.
if ($actor->isAdmin() || ($actor->hasPermission($ability))) {
return true;
}
return false;
}
/**
* Get all policies for a given model and ability.
*/
protected function getPolicies(string $model)
{
$compiledPolicies = Arr::get($this->policies, $model);
if (is_null($compiledPolicies)) {
$policyClasses = Arr::get($this->policyClasses, $model, []);
$compiledPolicies = array_map(function ($policyClass) {
return $this->container->make($policyClass);
}, $policyClasses);
Arr::set($this->policies, $model, $compiledPolicies);
}
return $compiledPolicies;
}
}

View File

@ -7,15 +7,12 @@
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\User;
namespace Flarum\User\Access;
use Flarum\User\User;
class UserPolicy extends AbstractPolicy
{
/**
* {@inheritdoc}
*/
protected $model = User::class;
/**
* @param User $actor
* @param string $ability
@ -24,7 +21,7 @@ class UserPolicy extends AbstractPolicy
public function can(User $actor, $ability)
{
if ($actor->hasPermission('user.'.$ability)) {
return true;
return $this->allow();
}
}
}

View File

@ -1,60 +0,0 @@
<?php
/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\User;
use Flarum\Event\GetPermission;
use Illuminate\Contracts\Events\Dispatcher;
class Gate
{
/**
* @var Dispatcher
*/
protected $events;
/**
* @param Dispatcher $events
*/
public function __construct(Dispatcher $events)
{
$this->events = $events;
}
/**
* Determine if the given ability should be granted for the current user.
*
* @param User $actor
* @param string $ability
* @param array|mixed $arguments
* @return bool
*/
public function allows($actor, $ability, $arguments)
{
// Fire an event so that core and extension policies can hook into
// this permission query and explicitly grant or deny the
// permission.
$allowed = $this->events->until(
new GetPermission($actor, $ability, $arguments)
);
if (! is_null($allowed)) {
return $allowed;
}
// If no policy covered this permission query, we will only grant
// the permission if the actor's groups have it. Otherwise, we will
// not allow the user to perform this action.
if ($actor->isAdmin() || ($actor->hasPermission($ability))) {
return true;
}
return false;
}
}

View File

@ -624,7 +624,7 @@ class User extends AbstractModel
* @param mixed $arguments
* @throws PermissionDeniedException
*/
public function assertCan($ability, $arguments = [])
public function assertCan($ability, $arguments = null)
{
$this->assertPermission(
$this->can($ability, $arguments)
@ -762,7 +762,7 @@ class User extends AbstractModel
* @param array|mixed $arguments
* @return bool
*/
public function can($ability, $arguments = [])
public function can($ability, $arguments = null)
{
return static::$gate->allows($this, $ability, $arguments);
}
@ -772,7 +772,7 @@ class User extends AbstractModel
* @param array|mixed $arguments
* @return bool
*/
public function cannot($ability, $arguments = [])
public function cannot($ability, $arguments = null)
{
return ! $this->can($ability, $arguments);
}

View File

@ -9,8 +9,14 @@
namespace Flarum\User;
use Flarum\Discussion\Access\DiscussionPolicy;
use Flarum\Discussion\Discussion;
use Flarum\Foundation\AbstractServiceProvider;
use Flarum\Foundation\ContainerUtil;
use Flarum\Group\Access\GroupPolicy;
use Flarum\Group\Group;
use Flarum\Post\Access\PostPolicy;
use Flarum\Post\Post;
use Flarum\Settings\SettingsRepositoryInterface;
use Flarum\User\Access\ScopeUserVisibility;
use Flarum\User\DisplayName\DriverInterface;
@ -36,6 +42,16 @@ class UserServiceProvider extends AbstractServiceProvider
$this->app->singleton('flarum.user.group_processors', function () {
return [];
});
$this->app->singleton('flarum.policies', function () {
return [
Access\AbstractPolicy::GLOBAL => [],
Discussion::class => [DiscussionPolicy::class],
Group::class => [GroupPolicy::class],
Post::class => [PostPolicy::class],
User::class => [Access\UserPolicy::class],
];
});
}
protected function registerDisplayNameDrivers()
@ -81,18 +97,17 @@ class UserServiceProvider extends AbstractServiceProvider
User::addGroupProcessor(ContainerUtil::wrapCallback($callback, $this->app));
}
User::setHasher($this->app->make('hash'));
User::setGate($this->app->make(Gate::class));
User::setDisplayNameDriver($this->app->make('flarum.user.display_name.driver'));
$events = $this->app->make('events');
User::setHasher($this->app->make('hash'));
User::setGate($this->app->makeWith(Access\Gate::class, ['policyClasses' => $this->app->make('flarum.policies')]));
User::setDisplayNameDriver($this->app->make('flarum.user.display_name.driver'));
$events->listen(Saving::class, SelfDemotionGuard::class);
$events->listen(Registered::class, AccountActivationMailer::class);
$events->listen(EmailChangeRequested::class, EmailConfirmationMailer::class);
$events->subscribe(UserMetadataUpdater::class);
$events->subscribe(UserPolicy::class);
User::registerPreference('discloseOnline', 'boolval', true);
User::registerPreference('indexProfile', 'boolval', true);

View File

@ -0,0 +1,287 @@
<?php
/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\Tests\integration\extenders;
use Carbon\Carbon;
use Flarum\Discussion\Discussion;
use Flarum\Extend;
use Flarum\Post\CommentPost;
use Flarum\Post\Post;
use Flarum\Tests\integration\BuildsHttpRequests;
use Flarum\Tests\integration\RetrievesAuthorizedUsers;
use Flarum\Tests\integration\TestCase;
use Flarum\User\Access\AbstractPolicy;
use Flarum\User\User;
class PolicyTest extends TestCase
{
use BuildsHttpRequests;
use RetrievesAuthorizedUsers;
// Request body to hide discussions sent in tests.
protected $hideQuery = ['authenticatedAs' => 2, 'json' => ['data' => ['attributes' => ['isHidden' => true]]]];
private function prepDb()
{
$this->prepareDatabase([
'users' => [
$this->adminUser(),
$this->normalUser(),
],
'discussions' => [
['id' => 1, 'title' => 'Unrelated Discussion', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'first_post_id' => 1, 'comment_count' => 1, 'is_private' => 0],
],
'posts' => [
['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>a normal reply - too-obscure</p></t>'],
]
]);
}
/**
* @test
*/
public function unrelated_user_cant_hide_discussion_by_default()
{
$this->prepDb();
$response = $this->send(
$this->request('PATCH', '/api/discussions/1', $this->hideQuery)
);
$this->assertEquals(403, $response->getStatusCode());
}
/**
* @test
*/
public function unrelated_user_can_hide_discussion_if_allowed()
{
$this->extend(
(new Extend\Policy())
->modelPolicy(Discussion::class, CustomPolicy::class)
);
$this->prepDb();
$response = $this->send(
$this->request('PATCH', '/api/discussions/1', $this->hideQuery)
);
$this->assertEquals(200, $response->getStatusCode());
}
/**
* @test
*/
public function unrelated_user_cant_hide_discussion_if_denied()
{
$this->extend(
(new Extend\Policy())
->modelPolicy(Discussion::class, DenyHidePolicy::class)
->modelPolicy(Discussion::class, CustomPolicy::class)
);
$this->prepDb();
$response = $this->send(
$this->request('PATCH', '/api/discussions/1', $this->hideQuery)
);
$this->assertEquals(403, $response->getStatusCode());
}
/**
* @test
*/
public function unrelated_user_can_hide_discussion_if_force_allowed()
{
$this->extend(
(new Extend\Policy())
->modelPolicy(Discussion::class, ForceAllowHidePolicy::class)
->modelPolicy(Discussion::class, DenyHidePolicy::class)
->modelPolicy(Discussion::class, CustomPolicy::class)
);
$this->prepDb();
$response = $this->send(
$this->request('PATCH', '/api/discussions/1', $this->hideQuery)
);
$this->assertEquals(200, $response->getStatusCode());
}
/**
* @test
*/
public function unrelated_user_cant_hide_discussion_if_force_denied()
{
$this->extend(
(new Extend\Policy())
->modelPolicy(Discussion::class, DenyHidePolicy::class)
->modelPolicy(Discussion::class, ForceDenyHidePolicy::class)
->modelPolicy(Discussion::class, CustomPolicy::class)
->modelPolicy(Discussion::class, ForceAllowHidePolicy::class)
);
$this->prepDb();
$response = $this->send(
$this->request('PATCH', '/api/discussions/1', $this->hideQuery)
);
$this->assertEquals(403, $response->getStatusCode());
}
/**
* @test
*/
public function regular_user_cant_start_discussions_by_default()
{
$this->prepDb();
$user = User::find(2);
$this->assertEquals(false, $user->can('startDiscussion'));
}
/**
* @test
*/
public function regular_user_can_start_discussions_if_granted_by_global_policy()
{
$this->extend(
(new Extend\Policy)
->globalPolicy(GlobalStartDiscussionPolicy::class)
);
$this->prepDb();
$user = User::find(2);
$this->assertEquals(true, $user->can('startDiscussion'));
}
/**
* @test
*/
public function global_policy_doesnt_apply_if_argument_provided()
{
$this->extend(
(new Extend\Policy)
->globalPolicy(GlobalStartDiscussionPolicy::class)
);
$this->prepDb();
$user = User::find(2);
$this->assertEquals(false, $user->can('startDiscussion', Discussion::find(1)));
}
/**
* @test
*/
public function unrelated_user_cant_hide_post_by_default()
{
$this->prepDb();
$user = User::find(2);
$this->assertEquals(false, $user->can('hide', Post::find(1)));
}
/**
* @test
*/
public function unrelated_user_can_hide_post_if_allowed()
{
$this->extend(
(new Extend\Policy)->modelPolicy(CommentPost::class, CommentPostChildClassPolicy::class)
);
$this->prepDb();
$user = User::find(2);
$this->assertEquals(true, $user->can('hide', Post::find(1)));
}
/**
* @test
*/
public function policies_are_inherited_to_child_classes()
{
$this->extend(
(new Extend\Policy)->modelPolicy(Post::class, PostParentClassPolicy::class),
(new Extend\Policy)->modelPolicy(CommentPost::class, CommentPostChildClassPolicy::class)
);
$this->prepDb();
$user = User::find(2);
$this->assertEquals(false, $user->can('hide', Post::find(1)));
}
}
class CustomPolicy extends AbstractPolicy
{
protected function hide(User $user, Discussion $discussion)
{
return $this->allow();
}
}
class DenyHidePolicy extends AbstractPolicy
{
protected function hide(User $user, Discussion $discussion)
{
return $this->deny();
}
}
class ForceAllowHidePolicy extends AbstractPolicy
{
protected function hide(User $user, Discussion $discussion)
{
return $this->forceAllow();
}
}
class ForceDenyHidePolicy extends AbstractPolicy
{
protected function hide(User $user, Discussion $discussion)
{
return $this->forceDeny();
}
}
class GlobalStartDiscussionPolicy extends AbstractPolicy
{
protected function startDiscussion(User $user)
{
return $this->allow();
}
}
class PostParentClassPolicy extends AbstractPolicy
{
protected function hide(User $user, Post $post)
{
return $this->deny();
}
}
class CommentPostChildClassPolicy extends AbstractPolicy
{
protected function hide(User $user, CommentPost $post)
{
return $this->allow();
}
}