Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
23 changes: 10 additions & 13 deletions src/AuthorizationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,12 @@ protected function performCheck(
$handler = $this->getCanHandler($policy, $action);
$result = $handler($user, $resource, ...$optionalArgs);

assert(
is_bool($result) || $result instanceof ResultInterface,
new Exception(sprintf(
if (!is_bool($result) && !$result instanceof ResultInterface) {
throw new Exception(sprintf(
'Authorization check method must return `%s` or `bool`.',
ResultInterface::class,
)),
);
));
}

return $result;
}
Expand Down Expand Up @@ -138,10 +137,9 @@ protected function getCanHandler(mixed $policy, string $action): Closure
{
$method = 'can' . ucfirst($action);

assert(
method_exists($policy, $method) || method_exists($policy, '__call'),
new MissingMethodException([$method, $action, $policy::class]),
);
if (!method_exists($policy, $method) && !method_exists($policy, '__call')) {
throw new MissingMethodException([$method, $action, $policy::class]);
}

/** @phpstan-ignore callable.nonCallable */
return [$policy, $method](...);
Expand All @@ -159,10 +157,9 @@ protected function getScopeHandler(mixed $policy, string $action): Closure
{
$method = 'scope' . ucfirst($action);

assert(
method_exists($policy, $method) || method_exists($policy, '__call'),
new MissingMethodException([$method, $action, $policy::class]),
);
if (!method_exists($policy, $method) && !method_exists($policy, '__call')) {
throw new MissingMethodException([$method, $action, $policy::class]);
}

/** @phpstan-ignore callable.nonCallable */
return [$policy, $method](...);
Expand Down
2 changes: 1 addition & 1 deletion src/Middleware/RequestAuthorizationMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
$service = $this->getServiceFromRequest($request);
$identity = $request->getAttribute($this->getConfig('identityAttribute'));

$result = $service->canResult($identity, $this->getConfig('method'), $request);
try {
$result = $service->canResult($identity, $this->getConfig('method'), $request);
if (!$result->getStatus()) {
throw new ForbiddenException($result, [$this->getConfig('method'), $request->getRequestTarget()]);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Middleware/UnauthorizedHandler/CakeRedirectHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function __construct()
protected function getUrl(ServerRequestInterface $request, array $options): string
{
$url = $options['url'];
if ($options['queryParam'] !== null) {
if ($options['queryParam'] !== null && $request->getMethod() === 'GET') {
$uri = $request->getUri();
$redirect = $uri->getPath();
if ($uri->getQuery()) {
Expand Down
19 changes: 19 additions & 0 deletions tests/TestCase/AuthorizationServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
namespace Authorization\Test\TestCase;

use Authorization\AuthorizationService;
use Authorization\Exception\Exception;
use Authorization\IdentityDecorator;
use Authorization\IdentityInterface;
use Authorization\Policy\BeforePolicyInterface;
Expand Down Expand Up @@ -539,4 +540,22 @@ public function testMissingMethod(): void

$service->can($user, 'disable', $entity);
}

public function testCanThrowsWhenPolicyReturnsInvalidType(): void
{
$resolver = new MapResolver([
Article::class => ArticlePolicy::class,
]);
$service = new AuthorizationService($resolver);
$user = new IdentityDecorator($service, [
'role' => 'admin',
]);

$this->expectException(Exception::class);
$this->expectExceptionMessage(
'Authorization check method must return `Authorization\Policy\ResultInterface` or `bool`.',
);

$service->can($user, 'invalidReturnType', new Article());
}
}
28 changes: 28 additions & 0 deletions tests/TestCase/Middleware/RequestAuthorizationMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,32 @@ public function testUnauthorizedHandlerSuppress(): void

$this->assertSame(200, $result->getStatusCode());
}

public function testPolicyExceptionRoutedThroughUnauthorizedHandler(): void
{
$request = (new ServerRequest([
'url' => '/articles/index',
]))
->withParam('action', 'index')
->withParam('controller', 'Articles');

$handler = new TestRequestHandler();

$resolver = new MapResolver([
ServerRequest::class => new RequestPolicy(),
]);

$authService = new AuthorizationService($resolver);
$request = $request->withAttribute('authorization', $authService);

// `doesNotExist` triggers MissingMethodException inside canResult();
// the middleware should route it through the configured handler instead of letting it bubble.
$middleware = new RequestAuthorizationMiddleware([
'method' => 'doesNotExist',
'unauthorizedHandler' => 'Suppress',
]);
$result = $middleware->process($request, $handler);

$this->assertSame(200, $result->getStatusCode());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Cake\Http\ServerRequestFactory;
use Cake\Routing\Router;
use Cake\TestSuite\TestCase;
use PHPUnit\Framework\Attributes\DataProvider;

class CakeRedirectHandlerTest extends TestCase
{
Expand Down Expand Up @@ -150,4 +151,39 @@ public function testHandleRedirectWithBasePath(): void
$response->getHeaderLine('Location'),
);
}

public static function httpMethodProvider(): array
{
return [
['POST'],
['PUT'],
['DELETE'],
['PATCH'],
['OPTIONS'],
['HEAD'],
];
}

#[DataProvider('httpMethodProvider')]
public function testHandleRedirectionIgnoreNonIdempotentMethods(string $method): void
{
$handler = new CakeRedirectHandler();

$exception = new Exception();
$request = ServerRequestFactory::fromGlobals(
[
'REQUEST_METHOD' => $method,
'REQUEST_URI' => '/admin/dashboard',
],
);

$response = $handler->handle($exception, $request, [
'exceptions' => [
Exception::class,
],
]);

$this->assertSame(302, $response->getStatusCode());
$this->assertSame('/login', $response->getHeaderLine('Location'));
}
}
9 changes: 9 additions & 0 deletions tests/test_app/TestApp/Policy/ArticlePolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,13 @@ public function canWithInjectedService($user, Article $article)
{
return $this->service->serviceLogic();
}

/**
* Returns an invalid type (not bool, not ResultInterface) to exercise the
* defensive type guard in AuthorizationService::performCheck().
*/
public function canInvalidReturnType($user, Article $article): string
{
return 'not a bool nor a Result';
}
}
Loading