diff --git a/app/Api/v1/Controllers/GroupController.php b/app/Api/v1/Controllers/GroupController.php index eb977016..c72c2c0b 100644 --- a/app/Api/v1/Controllers/GroupController.php +++ b/app/Api/v1/Controllers/GroupController.php @@ -20,7 +20,7 @@ class GroupController extends Controller */ public function index(Request $request) { - $groups = Groups::getAll($request->user()); + $groups = Groups::for($request->user())->withTheAllGroup()->all(); return GroupResource::collection($groups); } @@ -35,7 +35,7 @@ public function store(GroupStoreRequest $request) { $validated = $request->validated(); - $group = Groups::create($validated, $request->user()); + $group = Groups::for($request->user())->create($validated); return (new GroupResource($group)) ->response() @@ -46,11 +46,12 @@ public function store(GroupStoreRequest $request) * Display the specified resource. * * @param \App\Models\Group $group + * @param \Illuminate\Http\Request $request * @return \App\Api\v1\Resources\GroupResource */ - public function show(Group $group) + public function show(Group $group, Request $request) { - $this->authorize('view', $group); + $group = Groups::for($request->user())->get($group->id); return new GroupResource($group); } @@ -66,7 +67,7 @@ public function update(GroupStoreRequest $request, Group $group) { $validated = $request->validated(); - Groups::update($group, $validated, $request->user()); + Groups::for($request->user())->update($group, $validated); return new GroupResource($group); } @@ -82,7 +83,7 @@ public function assignAccounts(GroupAssignRequest $request, Group $group) { $validated = $request->validated(); - Groups::assign($validated['ids'], $request->user(), $group); + Groups::for($request->user())->assign($validated['ids'], $group); return new GroupResource($group); } @@ -91,13 +92,14 @@ public function assignAccounts(GroupAssignRequest $request, Group $group) * Get accounts assigned to the group * * @param \App\Models\Group $group + * @param \Illuminate\Http\Request $request * @return \App\Api\v1\Resources\TwoFAccountCollection */ - public function accounts(Group $group) + public function accounts(Group $group, Request $request) { - $this->authorize('view', $group); + $groups = Groups::for($request->user())->accounts($group); - return new TwoFAccountCollection($group->twofaccounts); + return new TwoFAccountCollection($groups); } /** @@ -109,7 +111,7 @@ public function accounts(Group $group) */ public function destroy(Group $group, Request $request) { - Groups::delete($group->id, $request->user()); + Groups::for($request->user())->delete($group->id); return response()->json(null, 204); } diff --git a/app/Events/GroupDeleted.php b/app/Events/GroupDeleted.php new file mode 100644 index 00000000..31210cd7 --- /dev/null +++ b/app/Events/GroupDeleted.php @@ -0,0 +1,29 @@ +group = $group; + } +} diff --git a/app/Facades/Groups.php b/app/Facades/Groups.php index 6616668e..dceaa8f1 100644 --- a/app/Facades/Groups.php +++ b/app/Facades/Groups.php @@ -5,6 +5,9 @@ use App\Services\GroupService; use Illuminate\Support\Facades\Facade; +/** + * @see \App\Services\GroupService + */ class Groups extends Facade { protected static function getFacadeAccessor() diff --git a/app/Listeners/DissociateTwofaccountFromGroup.php b/app/Listeners/DissociateTwofaccountFromGroup.php index a9ab9d9b..08daa788 100644 --- a/app/Listeners/DissociateTwofaccountFromGroup.php +++ b/app/Listeners/DissociateTwofaccountFromGroup.php @@ -31,6 +31,6 @@ public function handle(GroupDeleting $event) ['group_id' => null] ); - Log::info(sprintf('TwoFAccounts dissociated from group #%d', $event->group->id)); + Log::info(sprintf('TwoFAccounts dissociated from group %s (id #%d)', var_export($event->group->name, true), $event->group->id)); } } diff --git a/app/Listeners/ResetUsersPreference.php b/app/Listeners/ResetUsersPreference.php new file mode 100644 index 00000000..a28924d0 --- /dev/null +++ b/app/Listeners/ResetUsersPreference.php @@ -0,0 +1,48 @@ +preferences['defaultGroup'] == $event->group->id) { + $user['preferences->defaultGroup'] = 0; + } + + if ($user->preferences['activeGroup'] == $event->group->id) { + $user['preferences->activeGroup'] = 0; + } + + if ($user->isDirty()) { + $user->save(); + Log::info(sprintf('Group %s (id #%d) removed from user %s (id #%d) preferences', var_export($event->group->name, true), $event->group->id, var_export($user->name, true), $user->id)); + } + } + } +} diff --git a/app/Models/Group.php b/app/Models/Group.php index 243ba24e..f57038ad 100644 --- a/app/Models/Group.php +++ b/app/Models/Group.php @@ -2,6 +2,7 @@ namespace App\Models; +use App\Events\GroupDeleted; use App\Events\GroupDeleting; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; @@ -51,6 +52,7 @@ class Group extends Model */ protected $dispatchesEvents = [ 'deleting' => GroupDeleting::class, + 'deleted' => GroupDeleted::class, ]; /** @@ -62,9 +64,19 @@ protected static function boot() { parent::boot(); + static::created(function (object $model) { + // @codeCoverageIgnoreStart + Log::info(sprintf('Group %s (id #%d) created ', var_export($model->name, true), $model->id)); + // @codeCoverageIgnoreEnd + }); + static::updated(function (object $model) { + // @codeCoverageIgnoreStart + Log::info(sprintf('Group %s (id #%d) updated ', var_export($model->name, true), $model->id)); + // @codeCoverageIgnoreEnd + }); static::deleted(function (object $model) { // @codeCoverageIgnoreStart - Log::info(sprintf('Group "%s" deleted', var_export($model->name, true))); + Log::info(sprintf('Group %s (id #%d) deleted ', var_export($model->name, true), $model->id)); // @codeCoverageIgnoreEnd }); } diff --git a/app/Providers/EventServiceProvider.php b/app/Providers/EventServiceProvider.php index 84ae555e..ee2039ba 100644 --- a/app/Providers/EventServiceProvider.php +++ b/app/Providers/EventServiceProvider.php @@ -3,11 +3,13 @@ namespace App\Providers; use App\Events\GroupDeleting; +use App\Events\GroupDeleted; use App\Events\ScanForNewReleaseCalled; use App\Events\TwoFAccountDeleted; use App\Listeners\CleanIconStorage; use App\Listeners\DissociateTwofaccountFromGroup; use App\Listeners\ReleaseRadar; +use App\Listeners\ResetUsersPreference; use Illuminate\Auth\Events\Registered; use Illuminate\Auth\Listeners\SendEmailVerificationNotification; use Illuminate\Foundation\Support\Providers\EventServiceProvider as ServiceProvider; @@ -29,6 +31,9 @@ class EventServiceProvider extends ServiceProvider GroupDeleting::class => [ DissociateTwofaccountFromGroup::class, ], + GroupDeleted::class => [ + ResetUsersPreference::class, + ], ScanForNewReleaseCalled::class => [ ReleaseRadar::class, ], diff --git a/app/Services/GroupService.php b/app/Services/GroupService.php index 5173fc88..5ab58011 100644 --- a/app/Services/GroupService.php +++ b/app/Services/GroupService.php @@ -6,154 +6,268 @@ use App\Models\TwoFAccount; use App\Models\User; use Illuminate\Auth\Access\AuthorizationException; +use Illuminate\Contracts\Support\Arrayable; use Illuminate\Database\Eloquent\Collection; use Illuminate\Support\Facades\Log; class GroupService { /** - * Returns all existing groups for the given user - * - * @param \App\Models\User $user - * @return Collection + * @var \App\Models\User|null */ - public static function getAll(User $user) : Collection + protected $user; + + /** + * @var bool + */ + protected $withTheAllGroup = false; + + + /** + * Create a new Group service instance. + * + * @return void + */ + public function __construct() { - return self::prependTheAllGroup($user->groups()->withCount('twofaccounts')->get(), $user->id); + $this->user = null; } /** - * Creates a group for the given user + * Sets the user on behalf of whom the service act + * + * @param \App\Models\User $user + * @return self + */ + public function for(User $user) + { + $this->user = $user; + + return $this; + } + + /** + * Sets the service to return group collections prepended with the 'All' pseudo group + * + * @return self + */ + public function withTheAllGroup() + { + $this->withTheAllGroup = true; + + return $this; + } + + /** + * Get one or multiple group by their primary keys + * + * @param int|array $ids + * @return Collection|Group + * + * @throws \Illuminate\Database\Eloquent\ModelNotFoundException<\App\Models\Group> + */ + public function get(mixed $ids) + { + /** + * @var Collection|Group + */ + $groups = Group::withCount('twofaccounts')->findOrFail($ids); + + if ($groups instanceof Collection) { + if (! is_null($this->user)) { + // Authorization check + if ($this->user->cannot('viewEach', [(new Group), $groups])) { + Log::notice(sprintf('User ID #%s cannot view all groups in IDs #%s', $this->user->id, implode(',', $ids))); + throw new AuthorizationException(); + } + } + } + else { + if (! is_null($this->user)) { + // Authorization check + if ($this->user->cannot('view', $groups)) { + Log::notice(sprintf('User ID #%s cannot view group %s (#%s)', $this->user->id, var_export($groups->name, true), $groups->id)); + throw new AuthorizationException(); + } + } + } + + return $groups; + } + + /** + * Returns all existing groups preprended with the 'All' group for the given user + * + * @return Collection + */ + public function all() : Collection + { + $groups = ! is_null($this->user) + ? $this->user->groups()->withCount('twofaccounts')->get() + : Group::withCount('twofaccounts')->get(); + + return $this->withTheAllGroup + ? self::prependTheAllGroup($groups) + : $groups; + } + + /** + * Returns all accounts of the group + * + * @param \App\Models\Group $group + * @return Collection + */ + public function accounts(Group $group) : Collection + { + if (! is_null($this->user)) { + // Authorization check + if ($this->user->cannot('view', $group)) { + Log::notice(sprintf('User ID #%s cannot view group ID #%s', $this->user->id, $group->id)); + throw new AuthorizationException(); + } + } + + return $group->twofaccounts; + } + + /** + * Creates a group * * @param array $data - * @param \App\Models\User $user * @return \App\Models\Group The created group * * @throws \Illuminate\Auth\Access\AuthorizationException + * @throws \Exception */ - public static function create(array $data, User $user) : Group + public function create(array $data) : Group { - if ($user->cannot('create', Group::class)) { - Log::notice(sprintf('User ID #%s cannot create groups', $user->id)); - throw new AuthorizationException(); + if (! is_null($this->user)) { + // Authorization check + if ($this->user->cannot('create', Group::class)) { + Log::notice(sprintf('User ID #%s cannot create groups', $this->user->id)); + throw new AuthorizationException(); + } + + $group = $this->user->groups()->create([ + 'name' => $data['name'], + ]); + + Log::info(sprintf('Group %s created for user ID #%s', var_export($group->name, true), $this->user->id)); + + return $group; + } + else { + throw new \Exception('Cannot create a group without a user'); } - - $group = $user->groups()->create([ - 'name' => $data['name'], - ]); - - Log::info(sprintf('Group "%s" created for user ID #%s', var_export($group->name, true), $user->id)); - - return $group; } /** - * Updates a group using a list of parameters + * Updates a group using a list of values * * @param \App\Models\Group $group The group * @param array $data The parameters - * @param \App\Models\User $user * @return \App\Models\Group The updated group * + * * @throws \Illuminate\Auth\Access\AuthorizationException + * @throws \Exception */ - public static function update(Group $group, array $data, User $user) : Group + public function update(Group $group, array $data) : Group { - if ($user->cannot('update', $group)) { - Log::notice(sprintf('User ID #%s cannot update group "%s"', $user->id, var_export($group->name, true))); - throw new AuthorizationException(); + if (! is_null($this->user)) { + // Authorization check + if ($this->user->cannot('update', $group)) { + Log::notice(sprintf('User ID #%s cannot update group %s', $this->user->id, var_export($group->name, true))); + throw new AuthorizationException(); + } + + $group->update([ + 'name' => $data['name'], + ]); + + Log::info(sprintf('Group %s updated by user ID #%s', var_export($group->name, true), $this->user->id)); + + return $group; + } + else { + throw new \Exception('Cannot update a group without a user'); } - - $group->update([ - 'name' => $data['name'], - ]); - - Log::info(sprintf('Group "%s" updated by user ID #%s', var_export($group->name, true), $user->id)); - - return $group; } /** * Deletes one or more groups * * @param int|array $ids group ids to delete - * @param \App\Models\User $user * @return int The number of deleted */ - public static function delete($ids, User $user) : int + public function delete($ids) : int { $ids = is_array($ids) ? $ids : [$ids]; - $groups = Group::findMany($ids); if ($groups->count() > 0) { - if ($user->cannot('deleteEach', [$groups[0], $groups])) { - Log::notice(sprintf('User ID #%s cannot delete all groups in IDs #%s', $user->id, implode(',', $ids))); - throw new AuthorizationException(); + if (! is_null($this->user)) { + // Authorization check + if ($this->user->cannot('deleteEach', [$groups[0], $groups])) { + Log::notice(sprintf('User ID #%s cannot delete all groups in IDs #%s', $this->user->id, implode(',', $ids))); + throw new AuthorizationException(); + } } - // One of the groups is possibly set as the default group of the given user. - // In this case we reset the preference to "No group" (groupId = 0) - if (in_array($user->preferences['defaultGroup'], $ids)) { - $user['preferences->defaultGroup'] = 0; - $user->save(); - } - - // One of the groups is also possibly set as the active group if the user - // configured 2FAuth to memorize the active group. - // In this case we reset the preference to the pseudo "All" group (groupId = 0) - if (in_array($user->preferences['activeGroup'], $ids)) { - $user['preferences->activeGroup'] = 0; - $user->save(); - } - - $deleted = Group::destroy($ids); - Log::info(sprintf('Groups IDs #%s deleted', implode(',#', $ids))); - - return $deleted; + return Group::destroy($ids); } return 0; } /** - * Assign one or more accounts to a user group + * Assign one or more accounts to a group * * @param array|int $ids accounts ids to assign - * @param \App\Models\User $user * @param \App\Models\Group $group The target group * @return void * * @throws \Illuminate\Auth\Access\AuthorizationException * @throws \Illuminate\Database\Eloquent\ModelNotFoundException<\App\Models\TwoFAccount> */ - public static function assign($ids, User $user, Group $group = null) : void + public function assign($ids, Group $group = null) : void { - if (! $group) { - $group = self::defaultGroup($user); - } else { - if ($user->cannot('update', $group)) { - Log::notice(sprintf('User ID #%s cannot update group "%s"', $user->id, var_export($group->name, true))); - throw new AuthorizationException(); + $ids = is_array($ids) ? $ids : [$ids]; + $twofaccounts = TwoFAccount::findOrFail($ids); + + if (! is_null($this->user)) { + $group = $group ?? self::defaultGroup($this->user); + + if ($group) { + // Authorization check on group + if ($this->user->cannot('update', $group)) { + Log::notice(sprintf('User ID #%s cannot assign twofaccounts to group ID #%s', $this->user->id, $group->id)); + throw new AuthorizationException(); + } + + // Authorization check on twofaccounts + if ($this->user->cannot('updateEach', [$twofaccounts[0], $twofaccounts])) { + Log::notice(sprintf('User ID #%s cannot assign twofaccounts IDs #%s to a group', $this->user->id, implode(',', $ids))); + throw new AuthorizationException(); + } + + $group->twofaccounts()->saveMany($twofaccounts); + $group->loadCount('twofaccounts'); + + Log::info(sprintf('Twofaccounts IDs #%s assigned to group %s (id #%s)', implode(',', $ids), var_export($group->name, true), $group->id)); + } else { + Log::info(sprintf('Cannot find a group to assign the TwoFAccounts IDs #%s to', implode(',', $ids))); } } - - if ($group) { - $ids = is_array($ids) ? $ids : [$ids]; - - $twofaccounts = TwoFAccount::findOrFail($ids); - - if ($user->cannot('updateEach', [$twofaccounts[0], $twofaccounts])) { - Log::notice(sprintf('User ID #%s cannot assign twofaccounts %s to group "%s"', $user->id, implode(',', $ids), var_export($group->name, true))); - throw new AuthorizationException(); - } - + else if ($group) { $group->twofaccounts()->saveMany($twofaccounts); $group->loadCount('twofaccounts'); - Log::info(sprintf('Twofaccounts IDS #%s assigned to groups "%s"', implode(',', $ids), var_export($group->name, true))); - } else { - Log::info('Cannot find a group to assign the TwoFAccounts to'); + Log::info(sprintf('Twofaccounts IDs #%s assigned to group %s (id #%s)', implode(',', $ids), var_export($group->name, true), $group->id)); + } + else + { + Log::info(sprintf('No group to assign the TwoFAccounts IDs #%s to', implode(',', $ids))); } } @@ -163,14 +277,16 @@ public static function assign($ids, User $user, Group $group = null) : void * @param Collection $groups * @return Collection */ - private static function prependTheAllGroup(Collection $groups, int $userId) : Collection + private function prependTheAllGroup(Collection $groups) : Collection { $theAllGroup = new Group([ 'name' => __('commons.all'), ]); $theAllGroup->id = 0; - $theAllGroup->twofaccounts_count = TwoFAccount::where('user_id', $userId)->count(); + $theAllGroup->twofaccounts_count = is_null($this->user) + ? TwoFAccount::count() + : TwoFAccount::where('user_id', $this->user->id)->count(); return $groups->prepend($theAllGroup); } diff --git a/tests/Feature/Services/GroupServiceTest.php b/tests/Feature/Services/GroupServiceTest.php index 60d8a09c..fb14f1be 100644 --- a/tests/Feature/Services/GroupServiceTest.php +++ b/tests/Feature/Services/GroupServiceTest.php @@ -8,6 +8,7 @@ use App\Models\User; use App\Policies\GroupPolicy; use Illuminate\Auth\Access\AuthorizationException; +use Illuminate\Database\Eloquent\Collection; use Mockery\MockInterface; use Tests\FeatureTestCase; @@ -25,22 +26,26 @@ class GroupServiceTest extends FeatureTestCase /** * @var \App\Models\User|\Illuminate\Contracts\Auth\Authenticatable */ - protected $admin; + protected $otherUser; /** - * App\Models\Group $groupOne, $groupTwo + * App\Models\Group $groupOne, $groupTwo, $groupThree */ protected $groupOne; protected $groupTwo; + protected $groupThree; + /** - * App\Models\Group $twofaccountOne, $twofaccountTwo + * App\Models\Group $twofaccountOne, $twofaccountTwo, $twofaccountThree */ protected $twofaccountOne; protected $twofaccountTwo; + protected $twofaccountThree; + private const NEW_GROUP_NAME = 'MyNewGroup'; /** @@ -51,12 +56,13 @@ public function setUp() : void parent::setUp(); $this->user = User::factory()->create(); - $this->admin = User::factory()->administrator()->create(); + $this->otherUser = User::factory()->create(); $this->groupOne = Group::factory()->for($this->user)->create(); $this->groupTwo = Group::factory()->for($this->user)->create(); + $this->groupThree = Group::factory()->for($this->otherUser)->create(); - Group::factory()->count(3)->for($this->admin)->create(); + Group::factory()->count(2)->for($this->otherUser)->create(); $this->twofaccountOne = TwoFAccount::factory()->for($this->user)->create([ 'group_id' => $this->groupOne->id, @@ -65,15 +71,104 @@ public function setUp() : void 'group_id' => $this->groupTwo->id, ]); - TwoFAccount::factory()->for($this->admin)->create(); + $this->twofaccountThree = TwoFAccount::factory()->for($this->otherUser)->create([ + 'group_id' => $this->groupThree->id, + ]); + TwoFAccount::factory()->for($this->otherUser)->create([ + 'group_id' => $this->groupThree->id, + ]); } /** * @test */ - public function test_getAll_returns_pseudo_group_on_top_of_user_groups_only() + public function test_get_a_user_group_returns_a_group() { - $groups = Groups::getAll($this->user); + $group = Groups::for($this->user)->get($this->twofaccountOne->id); + + $this->assertInstanceOf(Group::class, $group); + $this->assertEquals($this->twofaccountOne->id, $group->id); + } + + /** + * @test + */ + public function test_get_multiple_user_group_returns_a_group_collection() + { + $groups = Groups::for($this->user)->get([$this->twofaccountOne->id, $this->twofaccountTwo->id]); + + $this->assertInstanceOf(Collection::class, $groups); + $this->assertCount(2, $groups); + $this->assertEquals($this->twofaccountOne->id, $groups[0]->id); + $this->assertEquals($this->twofaccountTwo->id, $groups[1]->id); + } + + /** + * @test + */ + public function test_get_a_missing_group_returns_not_found() + { + $this->expectException(\Exception::class); + + $group = Groups::get(1000); + } + + /** + * @test + */ + public function test_get_a_list_of_groups_with_a_missing_group_returns_not_found() + { + $this->expectException(\Exception::class); + + $group = Groups::get([$this->twofaccountOne->id, 1000]); + } + + /** + * @test + */ + public function test_user_authorization_to_get() + { + $this->expectException(AuthorizationException::class); + + Groups::for($this->otherUser)->get($this->twofaccountOne->id); + } + + /** + * @test + */ + public function test_user_authorization_to_multiple_get() + { + $this->expectException(AuthorizationException::class); + + Groups::for($this->otherUser)->get([$this->twofaccountOne->id, $this->twofaccountThree->id]); + } + + /** + * @test + */ + public function test_all_returns_user_groups_only() + { + $groups = Groups::for($this->user)->all(); + + $this->assertCount(2, $groups); + } + + /** + * @test + */ + public function test_all_returns_all_groups() + { + $groups = Groups::all(); + + $this->assertCount(5, $groups); + } + + /** + * @test + */ + public function test_withTheAllGroup_returns_pseudo_group_on_top_of_groups() + { + $groups = Groups::for($this->user)->withTheAllGroup()->all(); $this->assertCount(3, $groups); $this->assertEquals(0, $groups->first()->id); @@ -85,21 +180,34 @@ public function test_getAll_returns_pseudo_group_on_top_of_user_groups_only() /** * @test */ - public function test_getAll_returns_groups_with_count() + public function test_withTheAllGroup_returns_user_groups_with_count() { - $groups = Groups::getAll($this->user); + $groups = Groups::for($this->user)->withTheAllGroup()->all(); $this->assertEquals(2, $groups->first()->twofaccounts_count); $this->assertEquals(1, $groups[1]->twofaccounts_count); $this->assertEquals(1, $groups[2]->twofaccounts_count); } + /** + * @test + */ + public function test_withTheAllGroup_returns_all_groups_with_count() + { + $groups = Groups::withTheAllGroup()->all(); + + $this->assertEquals(4, $groups->first()->twofaccounts_count); + $this->assertEquals(1, $groups[1]->twofaccounts_count); + $this->assertEquals(1, $groups[2]->twofaccounts_count); + $this->assertEquals(2, $groups[3]->twofaccounts_count); + } + /** * @test */ public function test_create_persists_and_returns_created_group() { - $newGroup = Groups::create(['name' => self::NEW_GROUP_NAME], $this->user); + $newGroup = Groups::for($this->user)->create(['name' => self::NEW_GROUP_NAME]); $this->assertDatabaseHas('groups', [ 'name' => self::NEW_GROUP_NAME, @@ -112,7 +220,7 @@ public function test_create_persists_and_returns_created_group() /** * @test */ - public function test_create_authorization() + public function test_user_authorization_to_create() { $this->mock(GroupPolicy::class, function (MockInterface $groupPolicy) { $groupPolicy->shouldReceive('create') @@ -121,6 +229,16 @@ public function test_create_authorization() $this->expectException(AuthorizationException::class); + Groups::for($this->user)->create(['name' => 'lorem'], $this->user); + } + + /** + * @test + */ + public function test_create_without_user_fails() + { + $this->expectException(\Exception::class); + Groups::create(['name' => 'lorem'], $this->user); } @@ -129,7 +247,7 @@ public function test_create_authorization() */ public function test_update_persists_and_returns_updated_group() { - $this->groupOne = Groups::update($this->groupOne, ['name' => self::NEW_GROUP_NAME], $this->user); + $this->groupOne = Groups::for($this->user)->update($this->groupOne, ['name' => self::NEW_GROUP_NAME]); $this->assertDatabaseHas('groups', ['name' => self::NEW_GROUP_NAME]); $this->assertInstanceOf(Group::class, $this->groupOne); @@ -139,19 +257,29 @@ public function test_update_persists_and_returns_updated_group() /** * @test */ - public function test_update_fails_when_user_does_not_own_the_group() + public function test_user_authorization_to_update() { $this->expectException(AuthorizationException::class); - Groups::update($this->groupOne, ['name' => self::NEW_GROUP_NAME], $this->admin); + Groups::for($this->otherUser)->update($this->groupOne, ['name' => self::NEW_GROUP_NAME]); } /** * @test */ - public function test_delete_a_groupId_clear_db_and_returns_deleted_count() + public function test_update_without_user_fails() { - $deleted = Groups::delete($this->groupOne->id, $this->user); + $this->expectException(\Exception::class); + + Groups::update($this->groupOne, ['name' => self::NEW_GROUP_NAME]); + } + + /** + * @test + */ + public function test_delete_a_user_group_clears_db_and_returns_deleted_count() + { + $deleted = Groups::for($this->user)->delete($this->groupOne->id); $this->assertDatabaseMissing('groups', ['id' => $this->groupOne->id]); $this->assertEquals(1, $deleted); @@ -160,9 +288,9 @@ public function test_delete_a_groupId_clear_db_and_returns_deleted_count() /** * @test */ - public function test_delete_an_array_of_ids_clear_db_and_returns_deleted_count() + public function test_delete_multiple_user_groups_clears_db_and_returns_deleted_count() { - $deleted = Groups::delete([$this->groupOne->id, $this->groupTwo->id], $this->user); + $deleted = Groups::for($this->user)->delete([$this->groupOne->id, $this->groupTwo->id]); $this->assertDatabaseMissing('groups', ['id' => $this->groupOne->id]); $this->assertDatabaseMissing('groups', ['id' => $this->groupTwo->id]); @@ -172,11 +300,11 @@ public function test_delete_an_array_of_ids_clear_db_and_returns_deleted_count() /** * @test */ - public function test_delete_missing_id_does_not_fail_and_returns_deleted_count() + public function test_delete_missing_group_does_not_fail_and_returns_deleted_count() { $this->assertDatabaseMissing('groups', ['id' => 1000]); - $deleted = Groups::delete([$this->groupOne->id, 1000], $this->user); + $deleted = Groups::delete([$this->groupOne->id, 1000]); $this->assertDatabaseMissing('groups', ['id' => $this->groupOne->id]); $this->assertEquals(1, $deleted); @@ -185,12 +313,12 @@ public function test_delete_missing_id_does_not_fail_and_returns_deleted_count() /** * @test */ - public function test_delete_default_group_reset_defaultGroup_preference() + public function test_delete_default_group_resets_defaultGroup_preference() { $this->user['preferences->defaultGroup'] = $this->groupOne->id; $this->user->save(); - Groups::delete($this->groupOne->id, $this->user); + Groups::delete($this->groupOne->id); $this->user->refresh(); $this->assertEquals(0, $this->user->preferences['defaultGroup']); @@ -199,7 +327,7 @@ public function test_delete_default_group_reset_defaultGroup_preference() /** * @test */ - public function test_delete_active_group_reset_activeGroup_preference() + public function test_delete_active_group_resets_activeGroup_preference() { $this->user['preferences->rememberActiveGroup'] = true; $this->user['preferences->activeGroup'] = $this->groupOne->id; @@ -214,19 +342,19 @@ public function test_delete_active_group_reset_activeGroup_preference() /** * @test */ - public function test_delete_fails_when_user_does_not_own_one_of_the_groups() + public function test_user_authorization_to_delete() { $this->expectException(AuthorizationException::class); - Groups::delete($this->groupOne->id, $this->admin); + Groups::for($this->otherUser)->delete($this->groupOne->id); } /** * @test */ - public function test_assign_a_twofaccountid_to_a_specified_group_persists_the_relation() + public function test_assign_a_twofaccount_to_a_group_persists_the_relation() { - Groups::assign($this->twofaccountOne->id, $this->user, $this->groupTwo); + Groups::assign($this->twofaccountOne->id, $this->groupTwo); $this->assertDatabaseHas('twofaccounts', [ 'id' => $this->twofaccountOne->id, @@ -237,9 +365,22 @@ public function test_assign_a_twofaccountid_to_a_specified_group_persists_the_re /** * @test */ - public function test_assign_multiple_twofaccountid_to_a_specified_group_persists_the_relation() + public function test_assign_a_twofaccount_to_a_user_group_persists_the_relation() { - Groups::assign([$this->twofaccountOne->id, $this->twofaccountTwo->id], $this->user, $this->groupTwo); + Groups::for($this->user)->assign($this->twofaccountOne->id, $this->groupTwo); + + $this->assertDatabaseHas('twofaccounts', [ + 'id' => $this->twofaccountOne->id, + 'group_id' => $this->groupTwo->id, + ]); + } + + /** + * @test + */ + public function test_assign_multiple_twofaccounts_to_a_user_group_persists_the_relation() + { + Groups::for($this->user)->assign([$this->twofaccountOne->id, $this->twofaccountTwo->id], $this->groupTwo); $this->assertDatabaseHas('twofaccounts', [ 'id' => $this->twofaccountOne->id, @@ -254,12 +395,12 @@ public function test_assign_multiple_twofaccountid_to_a_specified_group_persists /** * @test */ - public function test_assign_a_twofaccountid_to_no_group_assigns_to_default_group() + public function test_assign_a_twofaccount_to_no_group_assigns_to_default_group() { $this->user['preferences->defaultGroup'] = $this->groupTwo->id; $this->user->save(); - Groups::assign($this->twofaccountOne->id, $this->user); + Groups::for($this->user)->assign($this->twofaccountOne->id); $this->assertDatabaseHas('twofaccounts', [ 'id' => $this->twofaccountOne->id, @@ -270,13 +411,13 @@ public function test_assign_a_twofaccountid_to_no_group_assigns_to_default_group /** * @test */ - public function test_assign_a_twofaccountid_to_no_group_assigns_to_active_group() + public function test_assign_a_twofaccount_to_no_group_assigns_to_active_group() { $this->user['preferences->defaultGroup'] = -1; $this->user['preferences->activeGroup'] = $this->groupTwo->id; $this->user->save(); - Groups::assign($this->twofaccountOne->id, $this->user); + Groups::for($this->user)->assign($this->twofaccountOne->id); $this->assertDatabaseHas('twofaccounts', [ 'id' => $this->twofaccountOne->id, @@ -287,7 +428,7 @@ public function test_assign_a_twofaccountid_to_no_group_assigns_to_active_group( /** * @test */ - public function test_assign_a_twofaccountid_to_missing_active_group_returns_not_found() + public function test_assign_a_twofaccount_to_missing_active_group_returns_not_found() { $orginalGroup = $this->twofaccountOne->group_id; @@ -295,7 +436,7 @@ public function test_assign_a_twofaccountid_to_missing_active_group_returns_not_ $this->user['preferences->activeGroup'] = 1000; $this->user->save(); - Groups::assign($this->twofaccountOne->id, $this->user); + Groups::for($this->user)->assign($this->twofaccountOne->id); $this->assertDatabaseHas('twofaccounts', [ 'id' => $this->twofaccountOne->id, @@ -306,20 +447,40 @@ public function test_assign_a_twofaccountid_to_missing_active_group_returns_not_ /** * @test */ - public function test_assign_fails_when_user_does_not_own_the_group() + public function test_user_authorization_to_assign_to_group() { $this->expectException(AuthorizationException::class); - Groups::assign($this->twofaccountOne->id, $this->user, $this->admin->groups()->first()); + Groups::for($this->otherUser)->assign($this->twofaccountOne->id, $this->otherUser->groups()->first()); } /** * @test */ - public function test_assign_fails_when_user_does_not_own_one_of_the_accounts() + public function test_user_authorization_to_assign_multiple_to_group() { $this->expectException(AuthorizationException::class); - Groups::assign([$this->twofaccountOne->id, $this->admin->twofaccounts()->first()->id], $this->user, $this->groupTwo); + Groups::for($this->otherUser)->assign([$this->twofaccountOne->id, $this->otherUser->twofaccounts()->first()->id], $this->groupTwo); + } + + /** + * @test + */ + public function test_user_authorization_to_assign_an_account() + { + $this->expectException(AuthorizationException::class); + + Groups::for($this->user)->assign($this->twofaccountThree->id, $this->user->groups()->first()); + } + + /** + * @test + */ + public function test_user_authorization_to_assign_multiple_accounts() + { + $this->expectException(AuthorizationException::class); + + Groups::for($this->user)->assign([$this->twofaccountOne->id, $this->twofaccountThree->id], $this->user->groups()->first()); } } diff --git a/tests/Unit/Api/v1/Controllers/GroupControllerTest.php b/tests/Unit/Api/v1/Controllers/GroupControllerTest.php index b4308720..4c85443a 100644 --- a/tests/Unit/Api/v1/Controllers/GroupControllerTest.php +++ b/tests/Unit/Api/v1/Controllers/GroupControllerTest.php @@ -9,6 +9,7 @@ use App\Api\v1\Resources\TwoFAccountReadResource; use App\Facades\Groups; use App\Models\Group; +use App\Models\TwoFAccount; use App\Models\User; use Illuminate\Foundation\Testing\WithoutMiddleware; use Illuminate\Http\Request; @@ -32,6 +33,11 @@ class GroupControllerTest extends TestCase */ protected $groupStoreRequest; + /** + * @var \App\Models\User mocked user + */ + protected $user; + /** * @var \Illuminate\Http\Request mocked request */ @@ -57,7 +63,14 @@ public function test_index_returns_api_resources_using_groupService() { $groups = Group::factory()->count(3)->make(); - Groups::shouldReceive('getAll') + Groups::shouldReceive('for') + ->with($this->request->user()) + ->once() + ->andReturnSelf() + ->shouldReceive('withTheAllGroup') + ->once() + ->andReturnSelf() + ->shouldReceive('all') ->once() ->andReturn($groups); @@ -72,14 +85,19 @@ public function test_index_returns_api_resources_using_groupService() public function test_store_returns_api_resource_stored_using_groupService() { $group = Group::factory()->make(); + $validated = ['name' => $group->name]; $this->groupStoreRequest->shouldReceive([ - 'validated' => ['name' => $group->name], + 'validated' => $validated, 'user' => new User(), - ]) - ->once(); + ]); - Groups::shouldReceive('create') + Groups::shouldReceive('for') + ->with($this->groupStoreRequest->user()) + ->once() + ->andReturnSelf() + ->shouldReceive('create') + ->with($validated) ->once() ->andReturn($group); @@ -92,11 +110,20 @@ public function test_store_returns_api_resource_stored_using_groupService() /** * @test */ - public function test_show_returns_api_resource() + public function test_show_returns_api_resource_using_groupService() { $group = Group::factory()->make(); - $response = $this->controller->show($group); + Groups::shouldReceive('for') + ->with($this->request->user()) + ->once() + ->andReturnSelf() + ->shouldReceive('get') + ->with($group->id) + ->once() + ->andReturn($group); + + $response = $this->controller->show($group, $this->request); $this->assertInstanceOf(GroupResource::class, $response); } @@ -107,14 +134,19 @@ public function test_show_returns_api_resource() public function test_update_returns_api_resource_updated_using_groupService() { $group = Group::factory()->make(); + $validated = ['name' => $group->name]; $this->groupStoreRequest->shouldReceive([ - 'validated' => ['name' => $group->name], - 'user' => new User(), - ]) - ->once(); + 'validated' => $validated, + 'user' => new User(), + ]); - Groups::shouldReceive('update') + Groups::shouldReceive('for') + ->with($this->groupStoreRequest->user()) + ->once() + ->andReturnSelf() + ->shouldReceive('update') + ->with($group, $validated) ->once() ->andReturn($group); @@ -130,16 +162,19 @@ public function test_assignAccounts_returns_api_resource_assigned_using_groupSer { $group = Group::factory()->make(); $groupAssignRequest = Mockery::mock(GroupAssignRequest::class); - $user = new User(); + $validated = ['ids' => $group->id]; $groupAssignRequest->shouldReceive([ - 'validated' => ['ids' => $group->id], - 'user' => $user, - ]) - ->once(); + 'validated' => $validated, + 'user' => new User(), + ]); - Groups::shouldReceive('assign') - ->with($group->id, $user, $group) + Groups::shouldReceive('for') + ->with($groupAssignRequest->user()) + ->once() + ->andReturnSelf() + ->shouldReceive('assign') + ->with($group->id, $group) ->once(); $response = $this->controller->assignAccounts($groupAssignRequest, $group); @@ -154,6 +189,23 @@ public function test_accounts_returns_api_resources_fetched_using_groupService() { $group = Group::factory()->make(); + $groupAccounts = new TwoFAccount(); + $groupAccounts = $groupAccounts->newCollection( + array( + new TwoFAccount(), + new TwoFAccount() + ) + ); + + Groups::shouldReceive('for') + ->with($this->request->user()) + ->once() + ->andReturnSelf() + ->shouldReceive('accounts') + ->with($group) + ->once() + ->andReturn($groupAccounts); + $response = $this->controller->accounts($group, $this->request); $this->assertContainsOnlyInstancesOf(TwoFAccountReadResource::class, $response->collection); @@ -167,10 +219,14 @@ public function test_destroy_uses_group_service() $group = Group::factory()->make(); $group->id = 0; - Groups::shouldReceive('delete') + Groups::shouldReceive('for') + ->with($this->request->user()) ->once() - ->with($group->id, $this->request->user()) - ->andReturn(0); + ->andReturnSelf() + ->shouldReceive('delete') + ->with($group->id) + ->once() + ->andReturn(1); $response = $this->controller->destroy($group, $this->request);