From 35834aa1bf186e2651ca4e9d92c2204fd1fe8b16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Nilsved?= Date: Mon, 29 Oct 2018 14:29:52 +0100 Subject: [PATCH 01/10] Format code --- src/Concerns/AuthorizesWithAbility.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Concerns/AuthorizesWithAbility.php b/src/Concerns/AuthorizesWithAbility.php index 332be4e0..4852fc16 100644 --- a/src/Concerns/AuthorizesWithAbility.php +++ b/src/Concerns/AuthorizesWithAbility.php @@ -34,6 +34,7 @@ public function registerAbilityForAuthorization(string $policyOrGate, $arguments public function registerGuardForAuthorization(Guard $guard): AuthorizesWithAbilityContract { $this->authorizesWithAbilityGuard = $guard; + return $this; } From afae667861dff20a5cc0bc2454bf348eec997377 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Nilsved?= Date: Mon, 29 Oct 2018 14:43:01 +0100 Subject: [PATCH 02/10] Add SerializesModels from Laravel queues It will serialize a single argument, but unfortunately not if the arguments is an array. --- src/Concerns/AuthorizesWithAbility.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Concerns/AuthorizesWithAbility.php b/src/Concerns/AuthorizesWithAbility.php index 4852fc16..74b7757f 100644 --- a/src/Concerns/AuthorizesWithAbility.php +++ b/src/Concerns/AuthorizesWithAbility.php @@ -4,10 +4,13 @@ use Illuminate\Contracts\Auth\Access\Authorizable; use Illuminate\Contracts\Auth\Guard; +use Illuminate\Queue\SerializesModels; use Kontenta\Kontour\Contracts\AuthorizesWithAbility as AuthorizesWithAbilityContract; trait AuthorizesWithAbility { + use SerializesModels; //If $authorizesWithAbilityArguments is just a single Eloquent model, this will serialize it. (But not if it's an array unfortunately) + private $authorizesWithAbilityPolicyOrGate; private $authorizesWithAbilityArguments; private $authorizesWithAbilityGuard; From e592a4df2bbcd52c1e9fb1e9e0902a8af11cafd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Nilsved?= Date: Mon, 29 Oct 2018 14:46:27 +0100 Subject: [PATCH 03/10] Add test for AuthorizesWithAbility --- tests/Feature/AuthorizesWithAbilityTest.php | 31 +++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/Feature/AuthorizesWithAbilityTest.php diff --git a/tests/Feature/AuthorizesWithAbilityTest.php b/tests/Feature/AuthorizesWithAbilityTest.php new file mode 100644 index 00000000..32aa0723 --- /dev/null +++ b/tests/Feature/AuthorizesWithAbilityTest.php @@ -0,0 +1,31 @@ +prepareDatabase(); + $this->user = factory(User::class)->create(); + } + + public function test_can_be_serialized_and_deserialized() { + $link = AdminLink::create('Reset password', 'http://test.com'); + + $serializedLink = serialize($link); + $unserializedLink = unserialize($serializedLink); + + $this->assertEquals($link, $unserializedLink, "Unserialization did not produce the orginal object structure"); + } +} From 9583c55e0a40258ddbab711106c540e5c8d4d7b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Nilsved?= Date: Mon, 29 Oct 2018 15:07:25 +0100 Subject: [PATCH 04/10] Resolve the Guard from name --- src/Concerns/AuthorizesWithAbility.php | 14 ++++++++++---- src/Contracts/AuthorizesWithAbility.php | 4 ++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Concerns/AuthorizesWithAbility.php b/src/Concerns/AuthorizesWithAbility.php index 74b7757f..e2b5b718 100644 --- a/src/Concerns/AuthorizesWithAbility.php +++ b/src/Concerns/AuthorizesWithAbility.php @@ -5,6 +5,7 @@ use Illuminate\Contracts\Auth\Access\Authorizable; use Illuminate\Contracts\Auth\Guard; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Facades\Auth; use Kontenta\Kontour\Contracts\AuthorizesWithAbility as AuthorizesWithAbilityContract; trait AuthorizesWithAbility @@ -31,10 +32,10 @@ public function registerAbilityForAuthorization(string $policyOrGate, $arguments /** * Register a guard to be used for the authorization - * @param Guard $guard + * @param string $guard * @return $this */ - public function registerGuardForAuthorization(Guard $guard): AuthorizesWithAbilityContract + public function registerGuardForAuthorization(string $guard): AuthorizesWithAbilityContract { $this->authorizesWithAbilityGuard = $guard; @@ -48,8 +49,13 @@ public function registerGuardForAuthorization(Guard $guard): AuthorizesWithAbili */ public function isAuthorized(Authorizable $user = null): bool { - if ($this->authorizesWithAbilityGuard) { - $user = $this->authorizesWithAbilityGuard->user(); + try { + if ($this->authorizesWithAbilityGuard) { + $user = Auth::guard($this->authorizesWithAbilityGuard)->user(); + } + } catch (\Exception $e) { + // Something is wrong with the guard... perhaps it no longer exists? + return false; } if (!$user) { diff --git a/src/Contracts/AuthorizesWithAbility.php b/src/Contracts/AuthorizesWithAbility.php index 714c58ba..83415859 100644 --- a/src/Contracts/AuthorizesWithAbility.php +++ b/src/Contracts/AuthorizesWithAbility.php @@ -16,8 +16,8 @@ public function registerAbilityForAuthorization(string $policyOrGate, $arguments /** * Register a guard to be used for the authorization - * @param Guard $guard + * @param string $guard * @return $this */ - public function registerGuardForAuthorization(Guard $guard): AuthorizesWithAbility; + public function registerGuardForAuthorization(string $guard): AuthorizesWithAbility; } From c66f0d7103c9d39e016ee80887b5baf8aa72f145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Nilsved?= Date: Mon, 29 Oct 2018 15:09:24 +0100 Subject: [PATCH 05/10] Check that user is Authorizable --- src/Concerns/AuthorizesWithAbility.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Concerns/AuthorizesWithAbility.php b/src/Concerns/AuthorizesWithAbility.php index e2b5b718..a9f1c9ac 100644 --- a/src/Concerns/AuthorizesWithAbility.php +++ b/src/Concerns/AuthorizesWithAbility.php @@ -58,7 +58,7 @@ public function isAuthorized(Authorizable $user = null): bool return false; } - if (!$user) { + if (!$user instanceof Authorizable) { return false; } From ff8ec6d91fe4b413c2f05085ef774126122df303 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Nilsved?= Date: Mon, 29 Oct 2018 15:18:41 +0100 Subject: [PATCH 06/10] Add basic Gate tests --- tests/Feature/AuthorizesWithAbilityTest.php | 23 +++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/Feature/AuthorizesWithAbilityTest.php b/tests/Feature/AuthorizesWithAbilityTest.php index 32aa0723..6ddbb5f1 100644 --- a/tests/Feature/AuthorizesWithAbilityTest.php +++ b/tests/Feature/AuthorizesWithAbilityTest.php @@ -2,9 +2,10 @@ namespace Kontenta\Kontour\Tests\Feature; +use Illuminate\Support\Facades\Gate; use Kontenta\Kontour\AdminLink; -use Kontenta\Kontour\Tests\IntegrationTest; use Kontenta\Kontour\Tests\Feature\Fakes\User; +use Kontenta\Kontour\Tests\IntegrationTest; class AuthorizesWithAbilityTest extends IntegrationTest { @@ -20,7 +21,25 @@ public function setUp() $this->user = factory(User::class)->create(); } - public function test_can_be_serialized_and_deserialized() { + public function test_authorizing_with_gate() + { + Gate::define('testGate', function ($user) { + return true; + }); + $link = AdminLink::create('Reset password', 'http://test.com')->registerAbilityForAuthorization('testGate'); + + $this->assertTrue($link->isAuthorized($this->user)); + } + + public function test_not_authorized_without_user() + { + $link = AdminLink::create('Reset password', 'http://test.com')->registerAbilityForAuthorization('testGate'); + + $this->assertFalse($link->isAuthorized()); + } + + public function test_can_be_serialized_and_deserialized() + { $link = AdminLink::create('Reset password', 'http://test.com'); $serializedLink = serialize($link); From 72e71586b948b80252a0eec48f621417657e7e0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Nilsved?= Date: Mon, 29 Oct 2018 15:36:06 +0100 Subject: [PATCH 07/10] Define the test gate in setup for reuse in multiple tests --- tests/Feature/AuthorizesWithAbilityTest.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/Feature/AuthorizesWithAbilityTest.php b/tests/Feature/AuthorizesWithAbilityTest.php index 6ddbb5f1..02bdff3d 100644 --- a/tests/Feature/AuthorizesWithAbilityTest.php +++ b/tests/Feature/AuthorizesWithAbilityTest.php @@ -19,13 +19,14 @@ public function setUp() parent::setUp(); $this->prepareDatabase(); $this->user = factory(User::class)->create(); - } - public function test_authorizing_with_gate() - { Gate::define('testGate', function ($user) { return true; }); + } + + public function test_authorizing_with_gate() + { $link = AdminLink::create('Reset password', 'http://test.com')->registerAbilityForAuthorization('testGate'); $this->assertTrue($link->isAuthorized($this->user)); From f38e04f6549dacd35819ece3898c7337240ad93d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Nilsved?= Date: Mon, 29 Oct 2018 16:01:45 +0100 Subject: [PATCH 08/10] Don't expand arguments to "can" method --- src/Concerns/AuthorizesWithAbility.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Concerns/AuthorizesWithAbility.php b/src/Concerns/AuthorizesWithAbility.php index a9f1c9ac..d923c77d 100644 --- a/src/Concerns/AuthorizesWithAbility.php +++ b/src/Concerns/AuthorizesWithAbility.php @@ -62,6 +62,6 @@ public function isAuthorized(Authorizable $user = null): bool return false; } - return $user->can($this->authorizesWithAbilityPolicyOrGate, ...$this->authorizesWithAbilityArguments); + return $user->can($this->authorizesWithAbilityPolicyOrGate, $this->authorizesWithAbilityArguments); } } From a7c57feca947c5ae6f289a833733051e3dc28446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Nilsved?= Date: Mon, 29 Oct 2018 16:04:06 +0100 Subject: [PATCH 09/10] Extend testing to all relevant cases --- tests/Feature/AuthorizesWithAbilityTest.php | 22 +++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/tests/Feature/AuthorizesWithAbilityTest.php b/tests/Feature/AuthorizesWithAbilityTest.php index 02bdff3d..9165c640 100644 --- a/tests/Feature/AuthorizesWithAbilityTest.php +++ b/tests/Feature/AuthorizesWithAbilityTest.php @@ -20,32 +20,46 @@ public function setUp() $this->prepareDatabase(); $this->user = factory(User::class)->create(); - Gate::define('testGate', function ($user) { - return true; + Gate::define('testGate', function ($user, $argument) { + return $user->id == $argument->id; }); } public function test_authorizing_with_gate() { - $link = AdminLink::create('Reset password', 'http://test.com')->registerAbilityForAuthorization('testGate'); + $link = AdminLink::create('Reset password', 'http://test.com'); + $link->registerAbilityForAuthorization('testGate', $this->user); $this->assertTrue($link->isAuthorized($this->user)); } public function test_not_authorized_without_user() { - $link = AdminLink::create('Reset password', 'http://test.com')->registerAbilityForAuthorization('testGate'); + $link = AdminLink::create('Reset password', 'http://test.com'); + $link->registerAbilityForAuthorization('testGate', $this->user); $this->assertFalse($link->isAuthorized()); } + public function test_not_authorized_with_non_existing_guard() + { + $link = AdminLink::create('Reset password', 'http://test.com'); + $link->registerAbilityForAuthorization('testGate', $this->user); + $link->registerGuardForAuthorization('non.existing.guard'); + + $this->assertFalse($link->isAuthorized($this->user)); + } + public function test_can_be_serialized_and_deserialized() { $link = AdminLink::create('Reset password', 'http://test.com'); + $link->registerAbilityForAuthorization('testGate', $this->user); $serializedLink = serialize($link); + $link->__wakeup(); // Restore any serialized models on the original object $unserializedLink = unserialize($serializedLink); + $this->assertTrue($unserializedLink->isAuthorized($this->user)); $this->assertEquals($link, $unserializedLink, "Unserialization did not produce the orginal object structure"); } } From 9bb0436b4de83972c170967fcdadd86b60cda627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Nilsved?= Date: Mon, 29 Oct 2018 16:14:15 +0100 Subject: [PATCH 10/10] Improve argument and property names --- src/Concerns/AuthorizesWithAbility.php | 14 +++++++------- src/Contracts/AuthorizesWithAbility.php | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Concerns/AuthorizesWithAbility.php b/src/Concerns/AuthorizesWithAbility.php index d923c77d..5ebbe428 100644 --- a/src/Concerns/AuthorizesWithAbility.php +++ b/src/Concerns/AuthorizesWithAbility.php @@ -12,19 +12,19 @@ trait AuthorizesWithAbility { use SerializesModels; //If $authorizesWithAbilityArguments is just a single Eloquent model, this will serialize it. (But not if it's an array unfortunately) - private $authorizesWithAbilityPolicyOrGate; + private $authorizesWithAbilityName; private $authorizesWithAbilityArguments; private $authorizesWithAbilityGuard; - /** + /** * Register a policy or gate to be used for the authorization - * @param string $policyOrGate - * @param $arguments + * @param string $ability name from a Gate/Policy + * @param array|mixed $arguments for the ability check, typically a model instance * @return $this */ - public function registerAbilityForAuthorization(string $policyOrGate, $arguments = []): AuthorizesWithAbilityContract + public function registerAbilityForAuthorization(string $ability, $arguments = []): AuthorizesWithAbilityContract { - $this->authorizesWithAbilityPolicyOrGate = $policyOrGate; + $this->authorizesWithAbilityName = $ability; $this->authorizesWithAbilityArguments = $arguments; return $this; @@ -62,6 +62,6 @@ public function isAuthorized(Authorizable $user = null): bool return false; } - return $user->can($this->authorizesWithAbilityPolicyOrGate, $this->authorizesWithAbilityArguments); + return $user->can($this->authorizesWithAbilityName, $this->authorizesWithAbilityArguments); } } diff --git a/src/Contracts/AuthorizesWithAbility.php b/src/Contracts/AuthorizesWithAbility.php index 83415859..5f536a50 100644 --- a/src/Contracts/AuthorizesWithAbility.php +++ b/src/Contracts/AuthorizesWithAbility.php @@ -8,11 +8,11 @@ interface AuthorizesWithAbility extends Authorizes { /** * Register a policy or gate to be used for the authorization - * @param string $policyOrGate - * @param $arguments + * @param string $ability name from a Gate/Policy + * @param array|mixed $arguments for the ability check, typically a model instance * @return $this */ - public function registerAbilityForAuthorization(string $policyOrGate, $arguments = []): AuthorizesWithAbility; + public function registerAbilityForAuthorization(string $ability, $arguments = []): AuthorizesWithAbility; /** * Register a guard to be used for the authorization