From 3c368f0f64e8ffd2d5be279fcf458ef982593738 Mon Sep 17 00:00:00 2001 From: Dzianis Kotau Date: Sat, 27 May 2023 17:11:41 +0300 Subject: [PATCH] Permissions implementation (#16) --- phpunit.xml.dist | 4 -- src/Enums/Permission.php | 13 ----- .../Concerns/WithMoonShineAuthorization.php | 2 +- src/Services/Contracts/BeImpersonable.php | 14 ++++++ src/Services/Contracts/Impersonable.php | 14 ++++++ src/Services/ImpersonateManager.php | 12 ++--- tests/Feature/Actions/EnterActionTest.php | 34 +++++++++++-- tests/Feature/Http/ImpersonateEnterTest.php | 36 ++++++++++++++ .../Factories/MoonshineUserFactory.php | 7 +++ .../Stubs/Database/Factories/UserFactory.php | 7 +++ tests/Stubs/Models/MoonshineUser.php | 8 ++- tests/Stubs/Models/User.php | 9 +++- tests/Unit/PermissionTest.php | 49 +++++++++++++++++++ 13 files changed, 176 insertions(+), 33 deletions(-) delete mode 100644 src/Enums/Permission.php create mode 100644 src/Services/Contracts/BeImpersonable.php create mode 100644 src/Services/Contracts/Impersonable.php create mode 100644 tests/Unit/PermissionTest.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index f92d9ab..0fb1753 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -24,10 +24,6 @@ ./src - - ./src/Rules/CanImpersonate.php - ./src/Rules/CanBeImpersonated.php - diff --git a/src/Enums/Permission.php b/src/Enums/Permission.php deleted file mode 100644 index bff4e93..0000000 --- a/src/Enums/Permission.php +++ /dev/null @@ -1,13 +0,0 @@ - - */ -enum Permission: string -{ - case IMPERSONATE = 'impersonate'; -} diff --git a/src/Http/Concerns/WithMoonShineAuthorization.php b/src/Http/Concerns/WithMoonShineAuthorization.php index 5b04a65..a562af4 100644 --- a/src/Http/Concerns/WithMoonShineAuthorization.php +++ b/src/Http/Concerns/WithMoonShineAuthorization.php @@ -15,6 +15,6 @@ trait WithMoonShineAuthorization { public function authorize(): bool { - return MoonShineAuth::guard()->check(); // TODO: check 'impersonate' permission as well + return MoonShineAuth::guard()->check(); } } diff --git a/src/Services/Contracts/BeImpersonable.php b/src/Services/Contracts/BeImpersonable.php new file mode 100644 index 0000000..9897711 --- /dev/null +++ b/src/Services/Contracts/BeImpersonable.php @@ -0,0 +1,14 @@ + + */ +interface BeImpersonable +{ + /** + * True if user can be impersonated, false otherwise + */ + public function canBeImpersonated(): bool; +} diff --git a/src/Services/Contracts/Impersonable.php b/src/Services/Contracts/Impersonable.php new file mode 100644 index 0000000..6001d9a --- /dev/null +++ b/src/Services/Contracts/Impersonable.php @@ -0,0 +1,14 @@ + + */ +interface Impersonable +{ + /** + * True if user can impersonate, false otherwise + */ + public function canImpersonate(): bool; +} diff --git a/src/Services/ImpersonateManager.php b/src/Services/ImpersonateManager.php index b999b3c..864990e 100644 --- a/src/Services/ImpersonateManager.php +++ b/src/Services/ImpersonateManager.php @@ -5,6 +5,8 @@ namespace Jampire\MoonshineImpersonate\Services; use Illuminate\Contracts\Auth\Authenticatable; +use Jampire\MoonshineImpersonate\Services\Contracts\BeImpersonable; +use Jampire\MoonshineImpersonate\Services\Contracts\Impersonable; use Jampire\MoonshineImpersonate\Support\Settings; /** @@ -53,11 +55,9 @@ public function canEnter(Authenticatable $userToImpersonate): bool return false; } - // @codeCoverageIgnoreStart if (!$this->canImpersonate()) { return false; } - // @codeCoverageIgnoreEnd return $this->canBeImpersonated($userToImpersonate); } @@ -78,16 +78,12 @@ public function isImpersonating(): bool public function canImpersonate(): bool { - // TODO: implement Permission::IMPERSONATE permission - - return true; + return !$this->moonshineUser instanceof Impersonable || $this->moonshineUser->canImpersonate(); } public function canBeImpersonated(Authenticatable $userToImpersonate): bool { - // TODO: implement which users are allowed to be impersonated - - return $userToImpersonate->getAuthIdentifier() > 0; + return !$userToImpersonate instanceof BeImpersonable || $userToImpersonate->canBeImpersonated(); } public function saveAuthInSession(Authenticatable $user): void diff --git a/tests/Feature/Actions/EnterActionTest.php b/tests/Feature/Actions/EnterActionTest.php index beb64bc..93e7e54 100644 --- a/tests/Feature/Actions/EnterActionTest.php +++ b/tests/Feature/Actions/EnterActionTest.php @@ -17,6 +17,22 @@ enableMoonShineGuard(); }); +test('enter action validation works correctly', function (): void { + Event::fake(); + + $user = User::factory()->create(); + $moonShineUser = MoonshineUser::factory()->create(); + actingAs($moonShineUser, Settings::moonShineGuard()); + + $action = app(EnterAction::class); + + expect($action->execute($user->getKey(), true)) + ->toBeTrue() + ->and($action->execute($user->getKey(), true)) + ->toBeFalse() + ; +}); + it('cannot execute enter action if user does not found', function (): void { $moonShineUser = MoonshineUser::factory()->create(); actingAs($moonShineUser, Settings::moonShineGuard()); @@ -28,18 +44,26 @@ ; }); -test('enter action validation works correctly', function (): void { - Event::fake(); +test('enter action cannot be executed on non-impersonated user', function (): void { + $user = User::factory()->notImpersonated()->create(); + $moonShineUser = MoonshineUser::factory()->create(); + actingAs($moonShineUser, Settings::moonShineGuard()); + + $action = app(EnterAction::class); + expect($action->execute($user->getKey(), true)) + ->toBeFalse() + ; +}); + +test('enter action cannot be executed by admin with no permission', function (): void { $user = User::factory()->create(); - $moonShineUser = MoonshineUser::factory()->create(); + $moonShineUser = MoonshineUser::factory()->cannotImpersonate()->create(); actingAs($moonShineUser, Settings::moonShineGuard()); $action = app(EnterAction::class); expect($action->execute($user->getKey(), true)) - ->toBeTrue() - ->and($action->execute($user->getKey(), true)) ->toBeFalse() ; }); diff --git a/tests/Feature/Http/ImpersonateEnterTest.php b/tests/Feature/Http/ImpersonateEnterTest.php index 2ae05c3..9d60ecd 100644 --- a/tests/Feature/Http/ImpersonateEnterTest.php +++ b/tests/Feature/Http/ImpersonateEnterTest.php @@ -111,3 +111,39 @@ 'id' => trans_impersonate('validation.enter.is_impersonating'), ]); }); + +it('cannot impersonate non-impersonated user', function (): void { + $user = User::factory()->notImpersonated()->create(); + $moonShineUser = MoonshineUser::factory()->create(); + + actingAs($moonShineUser, Settings::moonShineGuard()); + $response = post(route_impersonate('enter'), [ + 'id' => $user->id, + ]); + + $response->assertSessionHasErrors([ + 'id' => trans_impersonate('validation.enter.cannot_be_impersonated'), + ]); + + expect(session()->get(config_impersonate('key'))) + ->toBeEmpty() + ; +}); + +test('admin cannot impersonate with no permissions', function (): void { + $user = User::factory()->create(); + $moonShineUser = MoonshineUser::factory()->cannotImpersonate()->create(); + + actingAs($moonShineUser, Settings::moonShineGuard()); + $response = post(route_impersonate('enter'), [ + 'id' => $user->id, + ]); + + $response->assertSessionHasErrors([ + 'id' => trans_impersonate('validation.enter.cannot_impersonate'), + ]); + + expect(session()->get(config_impersonate('key'))) + ->toBeEmpty() + ; +}); diff --git a/tests/Stubs/Database/Factories/MoonshineUserFactory.php b/tests/Stubs/Database/Factories/MoonshineUserFactory.php index 2fdf620..86959f7 100644 --- a/tests/Stubs/Database/Factories/MoonshineUserFactory.php +++ b/tests/Stubs/Database/Factories/MoonshineUserFactory.php @@ -31,4 +31,11 @@ public function definition(): array 'remember_token' => Str::random(10), ]; } + + public function cannotImpersonate(): self + { + return $this->state(fn (): array => [ + 'moonshine_user_role_id' => 2, + ]); + } } diff --git a/tests/Stubs/Database/Factories/UserFactory.php b/tests/Stubs/Database/Factories/UserFactory.php index 044c64f..71675ca 100644 --- a/tests/Stubs/Database/Factories/UserFactory.php +++ b/tests/Stubs/Database/Factories/UserFactory.php @@ -31,4 +31,11 @@ public function definition(): array 'remember_token' => Str::random(10), ]; } + + public function notImpersonated(): self + { + return $this->state(fn (): array => [ + 'name' => fake()->name().' - Non-Impersonable', + ]); + } } diff --git a/tests/Stubs/Models/MoonshineUser.php b/tests/Stubs/Models/MoonshineUser.php index 248d176..4c29bdb 100644 --- a/tests/Stubs/Models/MoonshineUser.php +++ b/tests/Stubs/Models/MoonshineUser.php @@ -4,6 +4,7 @@ namespace Jampire\MoonshineImpersonate\Tests\Stubs\Models; +use Jampire\MoonshineImpersonate\Services\Contracts\Impersonable; use Jampire\MoonshineImpersonate\Tests\Stubs\Database\Factories\MoonshineUserFactory; use MoonShine\Models\MoonshineUser as BaseUser; @@ -12,7 +13,7 @@ * * @author Dzianis Kotau */ -class MoonshineUser extends BaseUser +class MoonshineUser extends BaseUser implements Impersonable { protected $table = 'moonshine_users'; @@ -20,4 +21,9 @@ protected static function newFactory(): MoonshineUserFactory { return MoonshineUserFactory::new(); } + + public function canImpersonate(): bool + { + return $this->moonshine_user_role_id === 1; + } } diff --git a/tests/Stubs/Models/User.php b/tests/Stubs/Models/User.php index 254ce6b..9be670b 100644 --- a/tests/Stubs/Models/User.php +++ b/tests/Stubs/Models/User.php @@ -6,6 +6,8 @@ use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Foundation\Auth\User as BaseUser; +use Illuminate\Support\Str; +use Jampire\MoonshineImpersonate\Services\Contracts\BeImpersonable; use Jampire\MoonshineImpersonate\Tests\Stubs\Database\Factories\UserFactory; use MoonShine\Traits\Models\HasMoonShineChangeLog; @@ -14,7 +16,7 @@ * * @author Dzianis Kotau */ -class User extends BaseUser +class User extends BaseUser implements BeImpersonable { use HasFactory; use HasMoonShineChangeLog; @@ -25,4 +27,9 @@ protected static function newFactory(): UserFactory { return UserFactory::new(); } + + public function canBeImpersonated(): bool + { + return !Str::endsWith($this->name, 'Non-Impersonable'); + } } diff --git a/tests/Unit/PermissionTest.php b/tests/Unit/PermissionTest.php new file mode 100644 index 0000000..93b5fd4 --- /dev/null +++ b/tests/Unit/PermissionTest.php @@ -0,0 +1,49 @@ +create(); + + $manager = new ImpersonateManager($moonShineUser); + + expect($manager->canImpersonate()) + ->toBeTrue() + ; +}); + +it('can be impersonated', function (): void { + $user = User::factory()->create(); + $moonShineUser = MoonshineUser::factory()->create(); + + $manager = new ImpersonateManager($moonShineUser); + + expect($manager->canBeImpersonated($user)) + ->toBeTrue() + ; +}); + +it('cannot impersonate with no permissions', function (): void { + $moonShineUser = MoonshineUser::factory()->cannotImpersonate()->create(); + + $manager = new ImpersonateManager($moonShineUser); + + expect($manager->canImpersonate()) + ->toBeFalse() + ; +}); + +it('cannot be impersonated with no permissions', function (): void { + $user = User::factory()->notImpersonated()->create(); + $moonShineUser = MoonshineUser::factory()->create(); + + $manager = new ImpersonateManager($moonShineUser); + + expect($manager->canBeImpersonated($user)) + ->toBeFalse() + ; +});