Refactor groups service and controller for better authorization handling

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);
     }

app/Events/GroupDeleted.php

@@ -0,0 +1,29 @@
+<?php
+
+namespace App\Events;
+
+use App\Models\Group;
+use Illuminate\Broadcasting\InteractsWithSockets;
+use Illuminate\Foundation\Events\Dispatchable;
+use Illuminate\Queue\SerializesModels;
+
+class GroupDeleted
+{
+    use Dispatchable, InteractsWithSockets, SerializesModels;
+
+    /**
+     * @var \App\Models\Group
+     */
+    public $group;
+
+    /**
+     * Create a new event instance.
+     *
+     * @param  \App\Models\Group  $group
+     * @return void
+     */
+    public function __construct(Group $group)
+    {
+        $this->group = $group;
+    }
+}

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()

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));
     }
 }

app/Listeners/ResetUsersPreference.php

@@ -0,0 +1,48 @@
+<?php
+
+namespace App\Listeners;
+
+use App\Events\GroupDeleted;
+use App\Models\User;
+use Illuminate\Support\Facades\Log;
+
+class ResetUsersPreference
+{
+    /**
+     * Create the event listener.
+     *
+     * @return void
+     */
+    public function __construct()
+    {
+        //
+    }
+
+    /**
+     * Handle the event.
+     *
+     * @param  GroupDeleted  $event
+     * @return void
+     */
+    public function handle(GroupDeleted $event)
+    {
+        // a group is possibly set as the default group or the active group for some users.
+        // In this case, after the group has been deleted, we must reset:
+        //      - the 'defaultGroup' preference to "No group" (groupId = 0)
+        //      - the 'activeGroup' preference to the pseudo "All" group (groupId = 0)
+        foreach (User::all() as $user) {
+            if ($user->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));
+            }
+        }
+    }
+}

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
         });
     }

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,
         ],

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
+     * @var  \App\Models\User|null
-     *
-     * @param  \App\Models\User  $user
-     * @return Collection<int, Group>
      */
-    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<int, Group>|Group
+     * 
+     * @throws \Illuminate\Database\Eloquent\ModelNotFoundException<\App\Models\Group>
+     */
+    public function get(mixed $ids)
+    {
+        /**
+         * @var Collection<int, Group>|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<int, Group>
+     */
+    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<int, \App\Models\TwoFAccount>
+     */
+    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)) {
+        if (! is_null($this->user)) {
-            Log::notice(sprintf('User ID #%s cannot create groups', $user->id));
+            // Authorization check
-            throw new AuthorizationException();
+            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)) {
+        if (! is_null($this->user)) {
-            Log::notice(sprintf('User ID #%s cannot update group "%s"', $user->id, var_export($group->name, true)));
+            // Authorization check
-            throw new AuthorizationException();
+            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])) {
+            if (! is_null($this->user)) {
-                Log::notice(sprintf('User ID #%s cannot delete all groups in IDs #%s', $user->id, implode(',', $ids)));
+                // Authorization check
-                throw new AuthorizationException();
+                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.
+            return Group::destroy($ids);
-            // 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 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) {
+        $ids = is_array($ids) ? $ids : [$ids];
-            $group = self::defaultGroup($user);
+        $twofaccounts = TwoFAccount::findOrFail($ids);
-        } else {
+
-            if ($user->cannot('update', $group)) {
+        if (! is_null($this->user)) {
-                Log::notice(sprintf('User ID #%s cannot update group "%s"', $user->id, var_export($group->name, true)));
+            $group = $group ?? self::defaultGroup($this->user);
-                throw new AuthorizationException();
+
+            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)));
             }
         }
-
+        else if ($group) {
-        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();
-            }
-
             $group->twofaccounts()->saveMany($twofaccounts);
             $group->loadCount('twofaccounts');
 
-            Log::info(sprintf('Twofaccounts IDS #%s assigned to groups "%s"', implode(',', $ids), var_export($group->name, true)));
+            Log::info(sprintf('Twofaccounts IDs #%s assigned to group %s (id #%s)', implode(',', $ids), var_export($group->name, true), $group->id));
-        } else {
+        }
-            Log::info('Cannot find a group to assign the TwoFAccounts to');
+        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<int, Group>  $groups
      * @return Collection<int, Group>
      */
-    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);
     }

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());
     }
 }

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],
+                'validated' => $validated,
-            'user'      => new User(),
+                'user'      => new User(),
-        ])
+        ]);
-            ->once();
 
-        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],
+            'validated' => $validated,
-            'user'      => $user,
+            'user'      => new User(),
-        ])
+        ]);
-            ->once();
 
-        Groups::shouldReceive('assign')
+        Groups::shouldReceive('for')
-            ->with($group->id, $user, $group)
+            ->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())
+            ->andReturnSelf()
-            ->andReturn(0);
+            ->shouldReceive('delete')
+            ->with($group->id)
+            ->once()
+            ->andReturn(1);
 
         $response = $this->controller->destroy($group, $this->request);