From e5292bb6bbda9070030f39b06bd123326537f8b0 Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Fri, 26 Nov 2021 17:54:21 -0300 Subject: [PATCH 01/14] Adds on-demand gate authorization. --- src/Illuminate/Auth/Access/Gate.php | 62 ++++++++- src/Illuminate/Support/Facades/Gate.php | 2 + tests/Auth/AuthAccessGateTest.php | 170 ++++++++++++++++++++++++ 3 files changed, 233 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index 75eba1dcea8f..26eb752ce9ab 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -313,7 +313,7 @@ public function none($abilities, $arguments = []) /** * Determine if the given ability should be granted for the current user. * - * @param string $ability + * @param \Closure|string $ability * @param array|mixed $arguments * @return \Illuminate\Auth\Access\Response * @@ -771,6 +771,66 @@ public function forUser($user) ); } + /** + * Throws an authorization exception if the condition or callback is falsy. + * + * @param \Closure|bool $condition + * @param string|null $message + * @param string|null $code + * @param bool $allow + * @return \Illuminate\Auth\Access\Response + * + * @throws \Illuminate\Auth\Access\AuthorizationException + */ + public function permit($condition, $message = null, $code = null, $allow = true) + { + $response = value($condition, $this->resolveUser()); + + if (! $response instanceof Response) { + $response = new Response($allow === (bool) $response, $message, $code); + } + + return $response->authorize(); + } + + /** + * Throws an authorization exception if the condition or callback is truthy. + * + * @param \Closure|bool $condition + * @param string|null $message + * @param string|null $code + * @return \Illuminate\Auth\Access\Response + * + * @throws \Illuminate\Auth\Access\AuthorizationException + */ + public function forbid($condition, $message = null, $code = null) + { + return $this->permit($condition, $message, $code, false); + } + + /** + * Evaluates an on-demand condition and returns a Response. + * + * @param \Closure|bool $condition + * @param array $arguments + * @param string|null $message + * @param mixed $code + * @param bool $straight + * @return \Illuminate\Auth\Access\Response + * + * @throws \Illuminate\Auth\Access\AuthorizationException + */ + protected function inspectOnDemand($condition, $arguments, $message, $code, $straight) + { + $condition = value($condition, $this->resolveUser(), ...$arguments); + + if (! $condition instanceof Response) { + $condition = new Response($straight == $condition, $message, $code); + } + + return $condition->authorize(); + } + /** * Resolve the user from the user resolver. * diff --git a/src/Illuminate/Support/Facades/Gate.php b/src/Illuminate/Support/Facades/Gate.php index 21355f2008a6..c61de4284ffd 100644 --- a/src/Illuminate/Support/Facades/Gate.php +++ b/src/Illuminate/Support/Facades/Gate.php @@ -8,6 +8,8 @@ * @method static \Illuminate\Auth\Access\Gate guessPolicyNamesUsing(callable $callback) * @method static \Illuminate\Auth\Access\Response authorize(string $ability, array|mixed $arguments = []) * @method static \Illuminate\Auth\Access\Response inspect(string $ability, array|mixed $arguments = []) + * @method static \Illuminate\Auth\Access\Response permit(\Closure|bool $condition, string|null $message = null, mixed $code = null) + * @method static \Illuminate\Auth\Access\Response forbid(\Closure|bool $condition, string|null $message = null, mixed $code = null) * @method static \Illuminate\Contracts\Auth\Access\Gate after(callable $callback) * @method static \Illuminate\Contracts\Auth\Access\Gate before(callable $callback) * @method static \Illuminate\Contracts\Auth\Access\Gate define(string $ability, callable|string $callback) diff --git a/tests/Auth/AuthAccessGateTest.php b/tests/Auth/AuthAccessGateTest.php index c9c0d7d08ef2..adce3d436231 100644 --- a/tests/Auth/AuthAccessGateTest.php +++ b/tests/Auth/AuthAccessGateTest.php @@ -689,6 +689,176 @@ public function testAuthorizeReturnsAnAllowedResponseForATruthyReturn() $this->assertNull($response->message()); } + public function testPermitAuthorizesTrue() + { + $response = $this->getBasicGate()->permit(true); + + $this->assertTrue($response->allowed()); + } + + public function testPermitAuthorizesTruthy() + { + $response = $this->getBasicGate()->permit('truthy'); + + $this->assertTrue($response->allowed()); + } + + public function testPermitAuthorizesCallbackTrue() + { + $response = $this->getBasicGate()->permit(function ($user) { + $this->assertSame(1, $user->id); + + return true; + }, 'foo', 'bar'); + + $this->assertTrue($response->allowed()); + $this->assertSame('foo', $response->message()); + $this->assertSame('bar', $response->code()); + } + + public function testPermitAuthorizesResponseAllowed() + { + $response = $this->getBasicGate()->permit(Response::allow('foo', 'bar')); + + $this->assertTrue($response->allowed()); + $this->assertSame('foo', $response->message()); + $this->assertSame('bar', $response->code()); + } + + public function testPermitAuthorizesCallbackResponseAllowed() + { + $response = $this->getBasicGate()->permit(function () { + return Response::allow('quz', 'qux'); + }, 'foo', 'bar'); + + $this->assertTrue($response->allowed()); + $this->assertSame('quz', $response->message()); + $this->assertSame('qux', $response->code()); + } + + public function testPermitThrowsExceptionWhenFalse() + { + $this->expectException(AuthorizationException::class); + + $this->getBasicGate()->permit(false); + } + + public function testPermitThrowsExceptionWhenCallbackFalse() + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('foo'); + $this->expectExceptionCode('bar'); + + $this->getBasicGate()->permit(function () { + return false; + }, 'foo', 'bar'); + } + + public function testPermitThrowsExceptionWhenResponseDenied() + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('foo'); + $this->expectExceptionCode('bar'); + + $this->getBasicGate()->permit(Response::deny('foo', 'bar')); + } + + public function testPermitThrowsExceptionWhenCallbackResponseDenied() + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('quz'); + $this->expectExceptionCode('qux'); + + $this->getBasicGate()->permit(function () { + return Response::deny('quz', 'qux'); + }, 'foo', 'bar'); + } + + public function testForbidAuthorizesFalse() + { + $response = $this->getBasicGate()->forbid(false); + + $this->assertTrue($response->allowed()); + } + + public function testForbidAuthorizesFalsy() + { + $response = $this->getBasicGate()->forbid(0); + + $this->assertTrue($response->allowed()); + } + + public function testForbidAuthorizesCallbackFalse() + { + $response = $this->getBasicGate()->forbid(function ($user) { + $this->assertSame(1, $user->id); + + return false; + }, 'foo', 'bar'); + + $this->assertTrue($response->allowed()); + $this->assertSame('foo', $response->message()); + $this->assertSame('bar', $response->code()); + } + + public function testForbidAuthorizesResponseAllowed() + { + $response = $this->getBasicGate()->forbid(Response::allow('foo', 'bar')); + + $this->assertTrue($response->allowed()); + $this->assertSame('foo', $response->message()); + $this->assertSame('bar', $response->code()); + } + + public function testForbidAuthorizesCallbackResponseAllowed() + { + $response = $this->getBasicGate()->forbid(function () { + return Response::allow('quz', 'qux'); + }, 'foo', 'bar'); + + $this->assertTrue($response->allowed()); + $this->assertSame('quz', $response->message()); + $this->assertSame('qux', $response->code()); + } + + public function testForbidThrowsExceptionWhenTrue() + { + $this->expectException(AuthorizationException::class); + + $this->getBasicGate()->forbid(true); + } + + public function testForbidThrowsExceptionWhenCallbackTrue() + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('foo'); + $this->expectExceptionCode('bar'); + + $this->getBasicGate()->forbid(function () { + return true; + }, 'foo', 'bar'); + } + + public function testForbidThrowsExceptionWhenResponseDenied() + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('foo'); + $this->expectExceptionCode('bar'); + + $this->getBasicGate()->forbid(Response::deny('foo', 'bar')); + } + + public function testForbidThrowsExceptionWhenCallbackResponseDenied() + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('quz'); + $this->expectExceptionCode('qux'); + + $this->getBasicGate()->forbid(function () { + return Response::deny('quz', 'qux'); + }, 'foo', 'bar'); + } + protected function getBasicGate($isAdmin = false) { return new Gate(new Container, function () use ($isAdmin) { From 769ee3064157594501cf10b67f1d8cccc720629d Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Fri, 26 Nov 2021 17:56:59 -0300 Subject: [PATCH 02/14] Removes lingering PHPDoc written by mistake. --- src/Illuminate/Auth/Access/Gate.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index 26eb752ce9ab..45896959ea12 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -313,7 +313,7 @@ public function none($abilities, $arguments = []) /** * Determine if the given ability should be granted for the current user. * - * @param \Closure|string $ability + * @param string $ability * @param array|mixed $arguments * @return \Illuminate\Auth\Access\Response * From 4812fb7b03c429c251fd5fde422718c9eb7894d6 Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Fri, 26 Nov 2021 18:03:09 -0300 Subject: [PATCH 03/14] Style changes. --- src/Illuminate/Auth/Access/Gate.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index 45896959ea12..c40e5223d507 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -774,7 +774,7 @@ public function forUser($user) /** * Throws an authorization exception if the condition or callback is falsy. * - * @param \Closure|bool $condition + * @param \Closure|bool $condition * @param string|null $message * @param string|null $code * @param bool $allow @@ -796,7 +796,7 @@ public function permit($condition, $message = null, $code = null, $allow = true) /** * Throws an authorization exception if the condition or callback is truthy. * - * @param \Closure|bool $condition + * @param \Closure|bool $condition * @param string|null $message * @param string|null $code * @return \Illuminate\Auth\Access\Response From d109aca2e143f6879c3dbd99dd1f3b3411d9ad85 Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Fri, 26 Nov 2021 18:05:49 -0300 Subject: [PATCH 04/14] Removes unused function. --- src/Illuminate/Auth/Access/Gate.php | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index c40e5223d507..c75e998ddb2f 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -808,29 +808,6 @@ public function forbid($condition, $message = null, $code = null) return $this->permit($condition, $message, $code, false); } - /** - * Evaluates an on-demand condition and returns a Response. - * - * @param \Closure|bool $condition - * @param array $arguments - * @param string|null $message - * @param mixed $code - * @param bool $straight - * @return \Illuminate\Auth\Access\Response - * - * @throws \Illuminate\Auth\Access\AuthorizationException - */ - protected function inspectOnDemand($condition, $arguments, $message, $code, $straight) - { - $condition = value($condition, $this->resolveUser(), ...$arguments); - - if (! $condition instanceof Response) { - $condition = new Response($straight == $condition, $message, $code); - } - - return $condition->authorize(); - } - /** * Resolve the user from the user resolver. * From 04714a5a112fd3ae397294a4ee4f50f06276f292 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Wed, 1 Dec 2021 08:03:13 -0600 Subject: [PATCH 05/14] formatting --- src/Illuminate/Auth/Access/Gate.php | 79 +++++++++++++------------ src/Illuminate/Support/Facades/Gate.php | 4 +- tests/Auth/AuthAccessGateTest.php | 36 +++++------ 3 files changed, 62 insertions(+), 57 deletions(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index c75e998ddb2f..484451661c4d 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -117,6 +117,48 @@ public function has($ability) return true; } + /** + * Perform an on-demand authorization check. Throw an authorization exception if the condition or callback is false. + * + * @param \Closure|bool $condition + * @param string|null $message + * @param string|null $code + * @return \Illuminate\Auth\Access\Response + * + * @throws \Illuminate\Auth\Access\AuthorizationException + */ + public function allowIf($condition, $message = null, $code = null) + { + $response = value($condition, $this->resolveUser()); + + if (! $response instanceof Response) { + $response = new Response((bool) $response === true, $message, $code); + } + + return $response->authorize(); + } + + /** + * Perform an on-demand authorization check. Throw an authorization exception if the condition or callback is true. + * + * @param \Closure|bool $condition + * @param string|null $message + * @param string|null $code + * @return \Illuminate\Auth\Access\Response + * + * @throws \Illuminate\Auth\Access\AuthorizationException + */ + public function denyIf($condition, $message = null, $code = null) + { + $response = value($condition, $this->resolveUser()); + + if (! $response instanceof Response) { + $response = new Response((bool) $response === false, $message, $code); + } + + return $response->authorize(); + } + /** * Define a new ability. * @@ -771,43 +813,6 @@ public function forUser($user) ); } - /** - * Throws an authorization exception if the condition or callback is falsy. - * - * @param \Closure|bool $condition - * @param string|null $message - * @param string|null $code - * @param bool $allow - * @return \Illuminate\Auth\Access\Response - * - * @throws \Illuminate\Auth\Access\AuthorizationException - */ - public function permit($condition, $message = null, $code = null, $allow = true) - { - $response = value($condition, $this->resolveUser()); - - if (! $response instanceof Response) { - $response = new Response($allow === (bool) $response, $message, $code); - } - - return $response->authorize(); - } - - /** - * Throws an authorization exception if the condition or callback is truthy. - * - * @param \Closure|bool $condition - * @param string|null $message - * @param string|null $code - * @return \Illuminate\Auth\Access\Response - * - * @throws \Illuminate\Auth\Access\AuthorizationException - */ - public function forbid($condition, $message = null, $code = null) - { - return $this->permit($condition, $message, $code, false); - } - /** * Resolve the user from the user resolver. * diff --git a/src/Illuminate/Support/Facades/Gate.php b/src/Illuminate/Support/Facades/Gate.php index c61de4284ffd..49d8b66d6a7f 100644 --- a/src/Illuminate/Support/Facades/Gate.php +++ b/src/Illuminate/Support/Facades/Gate.php @@ -8,8 +8,8 @@ * @method static \Illuminate\Auth\Access\Gate guessPolicyNamesUsing(callable $callback) * @method static \Illuminate\Auth\Access\Response authorize(string $ability, array|mixed $arguments = []) * @method static \Illuminate\Auth\Access\Response inspect(string $ability, array|mixed $arguments = []) - * @method static \Illuminate\Auth\Access\Response permit(\Closure|bool $condition, string|null $message = null, mixed $code = null) - * @method static \Illuminate\Auth\Access\Response forbid(\Closure|bool $condition, string|null $message = null, mixed $code = null) + * @method static \Illuminate\Auth\Access\Response allowIf(\Closure|bool $condition, string|null $message = null, mixed $code = null) + * @method static \Illuminate\Auth\Access\Response denyIf(\Closure|bool $condition, string|null $message = null, mixed $code = null) * @method static \Illuminate\Contracts\Auth\Access\Gate after(callable $callback) * @method static \Illuminate\Contracts\Auth\Access\Gate before(callable $callback) * @method static \Illuminate\Contracts\Auth\Access\Gate define(string $ability, callable|string $callback) diff --git a/tests/Auth/AuthAccessGateTest.php b/tests/Auth/AuthAccessGateTest.php index adce3d436231..8dddba49d756 100644 --- a/tests/Auth/AuthAccessGateTest.php +++ b/tests/Auth/AuthAccessGateTest.php @@ -691,21 +691,21 @@ public function testAuthorizeReturnsAnAllowedResponseForATruthyReturn() public function testPermitAuthorizesTrue() { - $response = $this->getBasicGate()->permit(true); + $response = $this->getBasicGate()->allowIf(true); $this->assertTrue($response->allowed()); } public function testPermitAuthorizesTruthy() { - $response = $this->getBasicGate()->permit('truthy'); + $response = $this->getBasicGate()->allowIf('truthy'); $this->assertTrue($response->allowed()); } public function testPermitAuthorizesCallbackTrue() { - $response = $this->getBasicGate()->permit(function ($user) { + $response = $this->getBasicGate()->allowIf(function ($user) { $this->assertSame(1, $user->id); return true; @@ -718,7 +718,7 @@ public function testPermitAuthorizesCallbackTrue() public function testPermitAuthorizesResponseAllowed() { - $response = $this->getBasicGate()->permit(Response::allow('foo', 'bar')); + $response = $this->getBasicGate()->allowIf(Response::allow('foo', 'bar')); $this->assertTrue($response->allowed()); $this->assertSame('foo', $response->message()); @@ -727,7 +727,7 @@ public function testPermitAuthorizesResponseAllowed() public function testPermitAuthorizesCallbackResponseAllowed() { - $response = $this->getBasicGate()->permit(function () { + $response = $this->getBasicGate()->allowIf(function () { return Response::allow('quz', 'qux'); }, 'foo', 'bar'); @@ -740,7 +740,7 @@ public function testPermitThrowsExceptionWhenFalse() { $this->expectException(AuthorizationException::class); - $this->getBasicGate()->permit(false); + $this->getBasicGate()->allowIf(false); } public function testPermitThrowsExceptionWhenCallbackFalse() @@ -749,7 +749,7 @@ public function testPermitThrowsExceptionWhenCallbackFalse() $this->expectExceptionMessage('foo'); $this->expectExceptionCode('bar'); - $this->getBasicGate()->permit(function () { + $this->getBasicGate()->allowIf(function () { return false; }, 'foo', 'bar'); } @@ -760,7 +760,7 @@ public function testPermitThrowsExceptionWhenResponseDenied() $this->expectExceptionMessage('foo'); $this->expectExceptionCode('bar'); - $this->getBasicGate()->permit(Response::deny('foo', 'bar')); + $this->getBasicGate()->allowIf(Response::deny('foo', 'bar')); } public function testPermitThrowsExceptionWhenCallbackResponseDenied() @@ -769,28 +769,28 @@ public function testPermitThrowsExceptionWhenCallbackResponseDenied() $this->expectExceptionMessage('quz'); $this->expectExceptionCode('qux'); - $this->getBasicGate()->permit(function () { + $this->getBasicGate()->allowIf(function () { return Response::deny('quz', 'qux'); }, 'foo', 'bar'); } public function testForbidAuthorizesFalse() { - $response = $this->getBasicGate()->forbid(false); + $response = $this->getBasicGate()->denyIf(false); $this->assertTrue($response->allowed()); } public function testForbidAuthorizesFalsy() { - $response = $this->getBasicGate()->forbid(0); + $response = $this->getBasicGate()->denyIf(0); $this->assertTrue($response->allowed()); } public function testForbidAuthorizesCallbackFalse() { - $response = $this->getBasicGate()->forbid(function ($user) { + $response = $this->getBasicGate()->denyIf(function ($user) { $this->assertSame(1, $user->id); return false; @@ -803,7 +803,7 @@ public function testForbidAuthorizesCallbackFalse() public function testForbidAuthorizesResponseAllowed() { - $response = $this->getBasicGate()->forbid(Response::allow('foo', 'bar')); + $response = $this->getBasicGate()->denyIf(Response::allow('foo', 'bar')); $this->assertTrue($response->allowed()); $this->assertSame('foo', $response->message()); @@ -812,7 +812,7 @@ public function testForbidAuthorizesResponseAllowed() public function testForbidAuthorizesCallbackResponseAllowed() { - $response = $this->getBasicGate()->forbid(function () { + $response = $this->getBasicGate()->denyIf(function () { return Response::allow('quz', 'qux'); }, 'foo', 'bar'); @@ -825,7 +825,7 @@ public function testForbidThrowsExceptionWhenTrue() { $this->expectException(AuthorizationException::class); - $this->getBasicGate()->forbid(true); + $this->getBasicGate()->denyIf(true); } public function testForbidThrowsExceptionWhenCallbackTrue() @@ -834,7 +834,7 @@ public function testForbidThrowsExceptionWhenCallbackTrue() $this->expectExceptionMessage('foo'); $this->expectExceptionCode('bar'); - $this->getBasicGate()->forbid(function () { + $this->getBasicGate()->denyIf(function () { return true; }, 'foo', 'bar'); } @@ -845,7 +845,7 @@ public function testForbidThrowsExceptionWhenResponseDenied() $this->expectExceptionMessage('foo'); $this->expectExceptionCode('bar'); - $this->getBasicGate()->forbid(Response::deny('foo', 'bar')); + $this->getBasicGate()->denyIf(Response::deny('foo', 'bar')); } public function testForbidThrowsExceptionWhenCallbackResponseDenied() @@ -854,7 +854,7 @@ public function testForbidThrowsExceptionWhenCallbackResponseDenied() $this->expectExceptionMessage('quz'); $this->expectExceptionCode('qux'); - $this->getBasicGate()->forbid(function () { + $this->getBasicGate()->denyIf(function () { return Response::deny('quz', 'qux'); }, 'foo', 'bar'); } From 0f4ff8bc971df62e214a571278c9eaa2cb75f956 Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Wed, 1 Dec 2021 15:59:15 -0300 Subject: [PATCH 06/14] Added exception when auth user is required and is guest. --- src/Illuminate/Auth/Access/Gate.php | 25 ++++--- tests/Auth/AuthAccessGateTest.php | 102 +++++++++++++++++++++++----- 2 files changed, 99 insertions(+), 28 deletions(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index 484451661c4d..d6ba36f1d831 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -2,6 +2,7 @@ namespace Illuminate\Auth\Access; +use Closure; use Exception; use Illuminate\Contracts\Auth\Access\Gate as GateContract; use Illuminate\Contracts\Container\Container; @@ -123,16 +124,26 @@ public function has($ability) * @param \Closure|bool $condition * @param string|null $message * @param string|null $code + * @param bool $allow * @return \Illuminate\Auth\Access\Response * * @throws \Illuminate\Auth\Access\AuthorizationException */ - public function allowIf($condition, $message = null, $code = null) + public function allowIf($condition, $message = null, $code = null, $allow = true) { - $response = value($condition, $this->resolveUser()); + $user = $this->resolveUser(); + + // When the developer issues a callback that expects the authenticated user, we + // will preemptively fail this check if there is none. This is accomplished by + // just negating the flag for authorization and respecting this fail message. + if (! $user && $condition instanceof Closure && ! $this->callbackAllowsGuests($condition)) { + $condition = ! $allow; + } + + $response = value($condition, $user); if (! $response instanceof Response) { - $response = new Response((bool) $response === true, $message, $code); + $response = new Response((bool) $response === $allow, $message, $code); } return $response->authorize(); @@ -150,13 +161,7 @@ public function allowIf($condition, $message = null, $code = null) */ public function denyIf($condition, $message = null, $code = null) { - $response = value($condition, $this->resolveUser()); - - if (! $response instanceof Response) { - $response = new Response((bool) $response === false, $message, $code); - } - - return $response->authorize(); + return $this->allowIf($condition, $message, $code, false); } /** diff --git a/tests/Auth/AuthAccessGateTest.php b/tests/Auth/AuthAccessGateTest.php index 8dddba49d756..79ad8645a195 100644 --- a/tests/Auth/AuthAccessGateTest.php +++ b/tests/Auth/AuthAccessGateTest.php @@ -689,21 +689,21 @@ public function testAuthorizeReturnsAnAllowedResponseForATruthyReturn() $this->assertNull($response->message()); } - public function testPermitAuthorizesTrue() + public function testAllowIfAuthorizesTrue() { $response = $this->getBasicGate()->allowIf(true); $this->assertTrue($response->allowed()); } - public function testPermitAuthorizesTruthy() + public function testAllowIfAuthorizesTruthy() { $response = $this->getBasicGate()->allowIf('truthy'); $this->assertTrue($response->allowed()); } - public function testPermitAuthorizesCallbackTrue() + public function testAllowIfAuthorizesCallbackTrue() { $response = $this->getBasicGate()->allowIf(function ($user) { $this->assertSame(1, $user->id); @@ -716,7 +716,7 @@ public function testPermitAuthorizesCallbackTrue() $this->assertSame('bar', $response->code()); } - public function testPermitAuthorizesResponseAllowed() + public function testAllowIfAuthorizesResponseAllowed() { $response = $this->getBasicGate()->allowIf(Response::allow('foo', 'bar')); @@ -725,7 +725,7 @@ public function testPermitAuthorizesResponseAllowed() $this->assertSame('bar', $response->code()); } - public function testPermitAuthorizesCallbackResponseAllowed() + public function testAllowIfAuthorizesCallbackResponseAllowed() { $response = $this->getBasicGate()->allowIf(function () { return Response::allow('quz', 'qux'); @@ -736,14 +736,34 @@ public function testPermitAuthorizesCallbackResponseAllowed() $this->assertSame('qux', $response->code()); } - public function testPermitThrowsExceptionWhenFalse() + public function testAllowsIfCallbackAcceptsGuestsWhenAuthenticated() + { + $response = $this->getBasicGate()->allowIf(function (stdClass $user = null) { + return $user !== null; + }); + + $this->assertTrue($response->allowed()); + } + + public function testAllowIfCallbackAcceptsGuestsWhenUnauthenticated() + { + $gate = $this->getBasicGate()->forUser(null); + + $response = $gate->allowIf(function (stdClass $user = null) { + return $user === null; + }); + + $this->assertTrue($response->allowed()); + } + + public function testAllowIfThrowsExceptionWhenFalse() { $this->expectException(AuthorizationException::class); $this->getBasicGate()->allowIf(false); } - public function testPermitThrowsExceptionWhenCallbackFalse() + public function testAllowIfThrowsExceptionWhenCallbackFalse() { $this->expectException(AuthorizationException::class); $this->expectExceptionMessage('foo'); @@ -754,7 +774,7 @@ public function testPermitThrowsExceptionWhenCallbackFalse() }, 'foo', 'bar'); } - public function testPermitThrowsExceptionWhenResponseDenied() + public function testAllowIfThrowsExceptionWhenResponseDenied() { $this->expectException(AuthorizationException::class); $this->expectExceptionMessage('foo'); @@ -763,7 +783,7 @@ public function testPermitThrowsExceptionWhenResponseDenied() $this->getBasicGate()->allowIf(Response::deny('foo', 'bar')); } - public function testPermitThrowsExceptionWhenCallbackResponseDenied() + public function testAllowIfThrowsExceptionWhenCallbackResponseDenied() { $this->expectException(AuthorizationException::class); $this->expectExceptionMessage('quz'); @@ -774,21 +794,34 @@ public function testPermitThrowsExceptionWhenCallbackResponseDenied() }, 'foo', 'bar'); } - public function testForbidAuthorizesFalse() + public function testAllowIfThrowsExceptionIfAuthUserExpectedWhenGuest() + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('foo'); + $this->expectExceptionCode('bar'); + + $gate = $this->getBasicGate()->forUser(null); + + $gate->allowIf(function (stdClass $user) { + return true; + }, 'foo', 'bar'); + } + + public function testDenyIfAuthorizesFalse() { $response = $this->getBasicGate()->denyIf(false); $this->assertTrue($response->allowed()); } - public function testForbidAuthorizesFalsy() + public function testDenyIfAuthorizesFalsy() { $response = $this->getBasicGate()->denyIf(0); $this->assertTrue($response->allowed()); } - public function testForbidAuthorizesCallbackFalse() + public function testDenyIfAuthorizesCallbackFalse() { $response = $this->getBasicGate()->denyIf(function ($user) { $this->assertSame(1, $user->id); @@ -801,7 +834,7 @@ public function testForbidAuthorizesCallbackFalse() $this->assertSame('bar', $response->code()); } - public function testForbidAuthorizesResponseAllowed() + public function testDenyIfAuthorizesResponseAllowed() { $response = $this->getBasicGate()->denyIf(Response::allow('foo', 'bar')); @@ -810,7 +843,7 @@ public function testForbidAuthorizesResponseAllowed() $this->assertSame('bar', $response->code()); } - public function testForbidAuthorizesCallbackResponseAllowed() + public function testDenyIfAuthorizesCallbackResponseAllowed() { $response = $this->getBasicGate()->denyIf(function () { return Response::allow('quz', 'qux'); @@ -821,14 +854,34 @@ public function testForbidAuthorizesCallbackResponseAllowed() $this->assertSame('qux', $response->code()); } - public function testForbidThrowsExceptionWhenTrue() + public function testDenyIfCallbackAcceptsGuestsWhenAuthenticated() + { + $response = $this->getBasicGate()->denyIf(function (stdClass $user = null) { + return $user === null; + }); + + $this->assertTrue($response->allowed()); + } + + public function testDenyIfCallbackAcceptsGuestsWhenUnauthenticated() + { + $gate = $this->getBasicGate()->forUser(null); + + $response = $gate->denyIf(function (stdClass $user = null) { + return $user !== null; + }); + + $this->assertTrue($response->allowed()); + } + + public function testDenyIfThrowsExceptionWhenTrue() { $this->expectException(AuthorizationException::class); $this->getBasicGate()->denyIf(true); } - public function testForbidThrowsExceptionWhenCallbackTrue() + public function testDenyIfThrowsExceptionWhenCallbackTrue() { $this->expectException(AuthorizationException::class); $this->expectExceptionMessage('foo'); @@ -839,7 +892,7 @@ public function testForbidThrowsExceptionWhenCallbackTrue() }, 'foo', 'bar'); } - public function testForbidThrowsExceptionWhenResponseDenied() + public function testDenyIfThrowsExceptionWhenResponseDenied() { $this->expectException(AuthorizationException::class); $this->expectExceptionMessage('foo'); @@ -848,7 +901,7 @@ public function testForbidThrowsExceptionWhenResponseDenied() $this->getBasicGate()->denyIf(Response::deny('foo', 'bar')); } - public function testForbidThrowsExceptionWhenCallbackResponseDenied() + public function testDenyIfThrowsExceptionWhenCallbackResponseDenied() { $this->expectException(AuthorizationException::class); $this->expectExceptionMessage('quz'); @@ -859,6 +912,19 @@ public function testForbidThrowsExceptionWhenCallbackResponseDenied() }, 'foo', 'bar'); } + public function testDenyIfThrowsExceptionIfAuthUserExpectedWhenGuest() + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('foo'); + $this->expectExceptionCode('bar'); + + $gate = $this->getBasicGate()->forUser(null); + + $gate->denyIf(function (stdClass $user) { + return false; + }, 'foo', 'bar'); + } + protected function getBasicGate($isAdmin = false) { return new Gate(new Container, function () use ($isAdmin) { From 836905640c44034a68ad81ccdb79b1b46ac118de Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Wed, 1 Dec 2021 16:10:55 -0300 Subject: [PATCH 07/14] Minor line style fix. --- src/Illuminate/Auth/Access/Gate.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index d6ba36f1d831..42aafa87462e 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -135,7 +135,7 @@ public function allowIf($condition, $message = null, $code = null, $allow = true // When the developer issues a callback that expects the authenticated user, we // will preemptively fail this check if there is none. This is accomplished by - // just negating the flag for authorization and respecting this fail message. + // just negating the authorization flag, and reuse the same message and code. if (! $user && $condition instanceof Closure && ! $this->callbackAllowsGuests($condition)) { $condition = ! $allow; } From e3aa7f4c4b2285c5afd2469667865191813ac24c Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Wed, 1 Dec 2021 18:29:35 -0300 Subject: [PATCH 08/14] Use `canBeCalledWithUser()` instead. --- src/Illuminate/Auth/Access/Gate.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index 42aafa87462e..c997705dc005 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -136,7 +136,7 @@ public function allowIf($condition, $message = null, $code = null, $allow = true // When the developer issues a callback that expects the authenticated user, we // will preemptively fail this check if there is none. This is accomplished by // just negating the authorization flag, and reuse the same message and code. - if (! $user && $condition instanceof Closure && ! $this->callbackAllowsGuests($condition)) { + if ($condition instanceof Closure && ! $this->canBeCalledWithUser($user, $condition)) { $condition = ! $allow; } From 6cc5f9b3b427809d108c42bbce3d8bab4f6fd939 Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Wed, 1 Dec 2021 18:29:49 -0300 Subject: [PATCH 09/14] Additional tests. --- tests/Auth/AuthAccessGateTest.php | 40 +++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/Auth/AuthAccessGateTest.php b/tests/Auth/AuthAccessGateTest.php index 79ad8645a195..4c68e2f00974 100644 --- a/tests/Auth/AuthAccessGateTest.php +++ b/tests/Auth/AuthAccessGateTest.php @@ -703,6 +703,13 @@ public function testAllowIfAuthorizesTruthy() $this->assertTrue($response->allowed()); } + public function testAllowIfAuthorizesIfGuest() + { + $response = $this->getBasicGate()->forUser(null)->allowIf(true); + + $this->assertTrue($response->allowed()); + } + public function testAllowIfAuthorizesCallbackTrue() { $response = $this->getBasicGate()->allowIf(function ($user) { @@ -794,6 +801,19 @@ public function testAllowIfThrowsExceptionWhenCallbackResponseDenied() }, 'foo', 'bar'); } + public function testAllowIfThrowsExceptionIfUnauthenticated() + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('foo'); + $this->expectExceptionCode('bar'); + + $gate = $this->getBasicGate()->forUser(null); + + $gate->allowIf(function () { + return true; + }, 'foo', 'bar'); + } + public function testAllowIfThrowsExceptionIfAuthUserExpectedWhenGuest() { $this->expectException(AuthorizationException::class); @@ -821,6 +841,13 @@ public function testDenyIfAuthorizesFalsy() $this->assertTrue($response->allowed()); } + public function testDenyIfAuthorizesIfGuest() + { + $response = $this->getBasicGate()->forUser(null)->denyIf(false); + + $this->assertTrue($response->allowed()); + } + public function testDenyIfAuthorizesCallbackFalse() { $response = $this->getBasicGate()->denyIf(function ($user) { @@ -912,6 +939,19 @@ public function testDenyIfThrowsExceptionWhenCallbackResponseDenied() }, 'foo', 'bar'); } + public function testDenyIfThrowsExceptionIfUnauthenticated() + { + $this->expectException(AuthorizationException::class); + $this->expectExceptionMessage('foo'); + $this->expectExceptionCode('bar'); + + $gate = $this->getBasicGate()->forUser(null); + + $gate->denyIf(function () { + return false; + }, 'foo', 'bar'); + } + public function testDenyIfThrowsExceptionIfAuthUserExpectedWhenGuest() { $this->expectException(AuthorizationException::class); From 25c754a667820c52f447d84c5b10e63aa343c256 Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Thu, 2 Dec 2021 12:28:24 -0300 Subject: [PATCH 10/14] Moved logic to shared method. --- src/Illuminate/Auth/Access/Gate.php | 48 +++++++++++++++++++---------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index c997705dc005..47ef58c5cdf0 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -124,12 +124,41 @@ public function has($ability) * @param \Closure|bool $condition * @param string|null $message * @param string|null $code - * @param bool $allow * @return \Illuminate\Auth\Access\Response * * @throws \Illuminate\Auth\Access\AuthorizationException */ - public function allowIf($condition, $message = null, $code = null, $allow = true) + public function allowIf($condition, $message = null, $code = null) + { + return $this->onDemand($condition, $message, $code, true); + } + + /** + * Perform an on-demand authorization check. Throw an authorization exception if the condition or callback is true. + * + * @param \Closure|bool $condition + * @param string|null $message + * @param string|null $code + * @return \Illuminate\Auth\Access\Response + * + * @throws \Illuminate\Auth\Access\AuthorizationException + */ + public function denyIf($condition, $message = null, $code = null) + { + return $this->onDemand($condition, $message, $code, false); + } + + /** + * Authorize a given condition or callback. + * + * @param \Closure|bool $condition + * @param string|null $message + * @param string|null $code + * @param bool $allow + * @return \Illuminate\Auth\Access\Response + * @throws \Illuminate\Auth\Access\AuthorizationException + */ + protected function onDemand($condition, $message, $code, $allow) { $user = $this->resolveUser(); @@ -149,21 +178,6 @@ public function allowIf($condition, $message = null, $code = null, $allow = true return $response->authorize(); } - /** - * Perform an on-demand authorization check. Throw an authorization exception if the condition or callback is true. - * - * @param \Closure|bool $condition - * @param string|null $message - * @param string|null $code - * @return \Illuminate\Auth\Access\Response - * - * @throws \Illuminate\Auth\Access\AuthorizationException - */ - public function denyIf($condition, $message = null, $code = null) - { - return $this->allowIf($condition, $message, $code, false); - } - /** * Define a new ability. * From 76a7966715a338bb739f568bcc110ee1baeb2ece Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Thu, 2 Dec 2021 12:39:14 -0300 Subject: [PATCH 11/14] Removed `value` helper. --- src/Illuminate/Auth/Access/Gate.php | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index 47ef58c5cdf0..c17ba5e23bc1 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -151,7 +151,7 @@ public function denyIf($condition, $message = null, $code = null) /** * Authorize a given condition or callback. * - * @param \Closure|bool $condition + * @param \Illuminate\Auth\Access\Response|\Closure|bool $condition * @param string|null $message * @param string|null $code * @param bool $allow @@ -165,17 +165,15 @@ protected function onDemand($condition, $message, $code, $allow) // When the developer issues a callback that expects the authenticated user, we // will preemptively fail this check if there is none. This is accomplished by // just negating the authorization flag, and reuse the same message and code. - if ($condition instanceof Closure && ! $this->canBeCalledWithUser($user, $condition)) { - $condition = ! $allow; + if ($condition instanceof Closure) { + $condition = $this->canBeCalledWithUser($user, $condition) ? $condition($user) : ! $allow; } - $response = value($condition, $user); - - if (! $response instanceof Response) { - $response = new Response((bool) $response === $allow, $message, $code); + if (! $condition instanceof Response) { + $condition = new Response((bool) $condition === $allow, $message, $code); } - return $response->authorize(); + return $condition->authorize(); } /** From 5c775c6a17cf58d95e997ac54ece884af96079a7 Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Thu, 2 Dec 2021 12:40:27 -0300 Subject: [PATCH 12/14] Adds support for `Response` object. Minor formatting. --- src/Illuminate/Auth/Access/Gate.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index c17ba5e23bc1..152fc437b5e5 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -121,7 +121,7 @@ public function has($ability) /** * Perform an on-demand authorization check. Throw an authorization exception if the condition or callback is false. * - * @param \Closure|bool $condition + * @param \Illuminate\Auth\Access\Response|\Closure|bool $condition * @param string|null $message * @param string|null $code * @return \Illuminate\Auth\Access\Response @@ -136,7 +136,7 @@ public function allowIf($condition, $message = null, $code = null) /** * Perform an on-demand authorization check. Throw an authorization exception if the condition or callback is true. * - * @param \Closure|bool $condition + * @param \Illuminate\Auth\Access\Response|\Closure|bool $condition * @param string|null $message * @param string|null $code * @return \Illuminate\Auth\Access\Response @@ -166,7 +166,9 @@ protected function onDemand($condition, $message, $code, $allow) // will preemptively fail this check if there is none. This is accomplished by // just negating the authorization flag, and reuse the same message and code. if ($condition instanceof Closure) { - $condition = $this->canBeCalledWithUser($user, $condition) ? $condition($user) : ! $allow; + $condition = $this->canBeCalledWithUser($user, $condition) + ? $condition($user) + : ! $allow; } if (! $condition instanceof Response) { From a29c757a360ddaa4ac7ec9a5a39c52b904942cf2 Mon Sep 17 00:00:00 2001 From: DarkGhosthunter Date: Thu, 2 Dec 2021 12:46:51 -0300 Subject: [PATCH 13/14] Style changes. --- src/Illuminate/Auth/Access/Gate.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index 152fc437b5e5..9fa274836fb7 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -154,8 +154,9 @@ public function denyIf($condition, $message = null, $code = null) * @param \Illuminate\Auth\Access\Response|\Closure|bool $condition * @param string|null $message * @param string|null $code - * @param bool $allow + * @param bool $allow * @return \Illuminate\Auth\Access\Response + * * @throws \Illuminate\Auth\Access\AuthorizationException */ protected function onDemand($condition, $message, $code, $allow) From 446118feb5d8690c48d7cb74d3fefba19eedb7ed Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 2 Dec 2021 15:09:07 -0600 Subject: [PATCH 14/14] formatting --- src/Illuminate/Auth/Access/Gate.php | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/Illuminate/Auth/Access/Gate.php b/src/Illuminate/Auth/Access/Gate.php index 9fa274836fb7..fe8d93fcb4ec 100644 --- a/src/Illuminate/Auth/Access/Gate.php +++ b/src/Illuminate/Auth/Access/Gate.php @@ -130,7 +130,7 @@ public function has($ability) */ public function allowIf($condition, $message = null, $code = null) { - return $this->onDemand($condition, $message, $code, true); + return $this->authorizeOnDemand($condition, $message, $code, true); } /** @@ -145,7 +145,7 @@ public function allowIf($condition, $message = null, $code = null) */ public function denyIf($condition, $message = null, $code = null) { - return $this->onDemand($condition, $message, $code, false); + return $this->authorizeOnDemand($condition, $message, $code, false); } /** @@ -154,29 +154,26 @@ public function denyIf($condition, $message = null, $code = null) * @param \Illuminate\Auth\Access\Response|\Closure|bool $condition * @param string|null $message * @param string|null $code - * @param bool $allow + * @param bool $allowWhenResponseIs * @return \Illuminate\Auth\Access\Response * * @throws \Illuminate\Auth\Access\AuthorizationException */ - protected function onDemand($condition, $message, $code, $allow) + protected function authorizeOnDemand($condition, $message, $code, $allowWhenResponseIs) { $user = $this->resolveUser(); - // When the developer issues a callback that expects the authenticated user, we - // will preemptively fail this check if there is none. This is accomplished by - // just negating the authorization flag, and reuse the same message and code. if ($condition instanceof Closure) { - $condition = $this->canBeCalledWithUser($user, $condition) - ? $condition($user) - : ! $allow; - } - - if (! $condition instanceof Response) { - $condition = new Response((bool) $condition === $allow, $message, $code); + $response = $this->canBeCalledWithUser($user, $condition) + ? $condition($user) + : new Response(false, $message, $code); + } else { + $response = $condition; } - return $condition->authorize(); + return with($response instanceof Response ? $response : new Response( + (bool) $response === $allowWhenResponseIs, $message, $code + ))->authorize(); } /**