Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.x: add ability to add multiple optional parameters to table scopes #226

Merged
merged 4 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions docs/en/3-0-migration-guide.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
3.0 Migration Guide
###################

Authorization 3.0 contains new features and a few breaking changes.

Breaking Changes
================

The following interfaces now have appropriate parameter and return types added:

- ``AuthorizationServiceInterface``
- ``IdentityInterface``
- ``BeforePolicyInterface``
- ``RequestPolicyInterface.php``
- ``ResolverInterface``

Multiple optional arguments for ``applyScope``
----------------------------------------------

``IdentityInterface::applyScope`` as well as ``AuthorizationServiceInterface::applyScope``
allow multiple optional arguments to be added.

Removed methods
---------------

- ``AuthorizationService::resultTypeCheck`` - has been replaced with an ``assert()`` call

4 changes: 2 additions & 2 deletions src/AuthorizationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,13 @@ protected function performCheck(?IdentityInterface $user, string $action, mixed
/**
* @inheritDoc
*/
public function applyScope(?IdentityInterface $user, string $action, $resource): mixed
public function applyScope(?IdentityInterface $user, string $action, mixed $resource, mixed ...$optionalArgs): mixed
{
$this->authorizationChecked = true;
$policy = $this->resolver->getPolicy($resource);
$handler = $this->getScopeHandler($policy, $action);

return $handler($user, $resource);
return $handler($user, $resource, ...$optionalArgs);
}

/**
Expand Down
8 changes: 7 additions & 1 deletion src/AuthorizationServiceInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,15 @@ public function canResult(?IdentityInterface $user, string $action, mixed $resou
* @param \Authorization\IdentityInterface|null $user The user to check permissions for.
* @param string $action The action/operation being performed.
* @param mixed $resource The resource being operated on.
* @param mixed $optionalArgs Multiple additional arguments which are passed to the scope
* @return mixed The modified resource.
*/
public function applyScope(?IdentityInterface $user, string $action, mixed $resource): mixed;
public function applyScope(
?IdentityInterface $user,
string $action,
mixed $resource,
mixed ...$optionalArgs
): mixed;

/**
* Return a boolean based on whether or not this object
Expand Down
5 changes: 3 additions & 2 deletions src/Controller/Component/AuthorizationComponent.php
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,10 @@ protected function performCheck(
*
* @param mixed $resource The resource to apply a scope to.
* @param string|null $action The action to apply a scope for.
* @param mixed $optionalArgs Multiple additional arguments which are passed to the scope
* @return mixed
*/
public function applyScope(mixed $resource, ?string $action = null): mixed
public function applyScope(mixed $resource, ?string $action = null, mixed ...$optionalArgs): mixed
{
$request = $this->getController()->getRequest();
if ($action === null) {
Expand All @@ -164,7 +165,7 @@ public function applyScope(mixed $resource, ?string $action = null): mixed
throw new MissingIdentityException('Identity must exist for applyScope() call.');
}

return $identity->applyScope($action, $resource);
return $identity->applyScope($action, $resource, ...$optionalArgs);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/IdentityDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ public function canResult(string $action, $resource): ResultInterface
/**
* @inheritDoc
*/
public function applyScope(string $action, $resource): mixed
public function applyScope(string $action, mixed $resource, mixed ...$optionalArgs): mixed
{
return $this->authorization->applyScope($this, $action, $resource);
return $this->authorization->applyScope($this, $action, $resource, ...$optionalArgs);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/IdentityInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ public function canResult(string $action, mixed $resource): ResultInterface;
*
* @param string $action The action/operation being performed.
* @param mixed $resource The resource being operated on.
* @param mixed $optionalArgs Multiple additional arguments which are passed to the scope
* @return mixed The modified resource.
*/
public function applyScope(string $action, mixed $resource): mixed;
public function applyScope(string $action, mixed $resource, mixed ...$optionalArgs): mixed;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be a breaking change as user-land code will have implemented this interface. We should add this to the documentation/migration guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. I was just curious if you guys even like this idea or not 😄
The first version of the 3.0 migration guide has been added with the major changes I have seen via looking at 2.x...3.x-scope-params


/**
* Get the decorated identity
Expand Down
30 changes: 30 additions & 0 deletions tests/TestCase/AuthorizationServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@
use Authorization\Policy\BeforePolicyInterface;
use Authorization\Policy\Exception\MissingMethodException;
use Authorization\Policy\MapResolver;
use Authorization\Policy\OrmResolver;
use Authorization\Policy\Result;
use Authorization\Policy\ResultInterface;
use Cake\Datasource\QueryInterface;
use Cake\TestSuite\TestCase;
use TestApp\Model\Entity\Article;
use TestApp\Model\Table\ArticlesTable;
use TestApp\Policy\ArticlePolicy;
use TestApp\Policy\MagicCallPolicy;

Expand Down Expand Up @@ -173,6 +176,33 @@ public function testApplyScopeMethodMissing()
$result = $service->applyScope($user, 'nope', $article);
}

public function testApplyScopeAdditionalArguments()
{
$service = new AuthorizationService(new OrmResolver());
$user = new IdentityDecorator($service, [
'id' => 9,
'role' => 'admin',
]);

$articles = new ArticlesTable();
$query = $this->createMock(QueryInterface::class);
$query->method('getRepository')
->willReturn($articles);

$query->expects($this->once())
->method('where')
->with([
'identity_id' => 9,
'firstArg' => 'first argument',
'secondArg' => false,
])
->willReturn($query);
$result = $service->applyScope($user, 'additionalArguments', $query, 'first argument', false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will these changes also enable the use of named parameters? eg. applyScope($user, category: $category)

Copy link
Contributor Author

@LordSimal LordSimal Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a test which proves it works 😉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay!


$this->assertInstanceOf(QueryInterface::class, $result);
$this->assertSame($query, $result);
}

public function testBeforeFalse()
{
$entity = new Article();
Expand Down
22 changes: 22 additions & 0 deletions tests/TestCase/Controller/Component/AuthorizationComponentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,28 @@ public function testApplyScopExplicitAction()
$this->assertSame($query, $result);
}

public function testApplyScopeAdditionalArguments()
{
$articles = new ArticlesTable();
$query = $this->createMock(QueryInterface::class);
$query->method('getRepository')
->willReturn($articles);

$query->expects($this->once())
->method('where')
->with([
'identity_id' => 1,
'firstArg' => 'first argument',
'secondArg' => false,
])
->willReturn($query);

$result = $this->Auth->applyScope($query, 'additionalArguments', 'first argument', false);

$this->assertInstanceOf(QueryInterface::class, $result);
$this->assertSame($query, $result);
}

public function testAuthorizeSuccessCheckExplicitAction()
{
$article = new Article(['user_id' => 1]);
Expand Down
13 changes: 13 additions & 0 deletions tests/TestCase/IdentityDecoratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,19 @@ public function testApplyScopeDelegation()
$this->assertTrue($identity->applyScope('update', $resource));
}

public function testApplyScopeAdditionalParams()
{
$resource = new stdClass();
$auth = $this->createMock(AuthorizationServiceInterface::class);
$identity = new IdentityDecorator($auth, ['id' => 1]);

$auth->expects($this->once())
->method('applyScope')
->with($identity, 'update', $resource, 'first argument', false)
->will($this->returnValue(true));
$this->assertTrue($identity->applyScope('update', $resource, 'first argument', false));
}

public function testCall()
{
$data = new Article(['id' => 1]);
Expand Down
9 changes: 9 additions & 0 deletions tests/test_app/TestApp/Policy/ArticlesTablePolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,13 @@ public function scopeModify(IdentityInterface $user, QueryInterface $query)
'identity_id' => $user['id'],
]);
}

public function scopeAdditionalArguments(IdentityInterface $user, QueryInterface $query, $firstArg, $secondArg)
{
return $query->where([
'identity_id' => $user['id'],
'firstArg' => $firstArg,
'secondArg' => $secondArg,
]);
}
}