Move Authorization checks to Service for Groups & Update tests

app/Api/v1/Controllers/GroupController.php

@@ -20,7 +20,7 @@ class GroupController extends Controller
      */
     public function index(Request $request)
     {
-        $groups = Groups::prependTheAllGroup($request->user()->groups()->withCount('twofaccounts')->get(), $request->user()->id);
+        $groups = Groups::getAll($request->user());
 
         return GroupResource::collection($groups);
     }
@@ -33,11 +33,9 @@ class GroupController extends Controller
      */
     public function store(GroupStoreRequest $request)
     {
-        $this->authorize('create', Group::class);
-
         $validated = $request->validated();
 
-        $group = $request->user()->groups()->create($validated);
+        $group = Groups::create($validated, $request->user());
 
         return (new GroupResource($group))
             ->response()
@@ -66,11 +64,9 @@ class GroupController extends Controller
      */
     public function update(GroupStoreRequest $request, Group $group)
     {
-        $this->authorize('update', $group);
-
         $validated = $request->validated();
 
-        Groups::update($group, $validated);
+        Groups::update($group, $validated, $request->user());
 
         return new GroupResource($group);
     }
@@ -84,11 +80,9 @@ class GroupController extends Controller
      */
     public function assignAccounts(GroupAssignRequest $request, Group $group)
     {
-        $this->authorize('update', $group);
-
         $validated = $request->validated();
 
-        Groups::assign($validated['ids'], $group);
+        Groups::assign($validated['ids'], $request->user(), $group);
 
         return new GroupResource($group);
     }
@@ -103,20 +97,19 @@ class GroupController extends Controller
     {
         $this->authorize('view', $group);
 
-        return new TwoFAccountCollection($group->twofaccounts());
+        return new TwoFAccountCollection($group->twofaccounts);
     }
 
     /**
      * Remove the specified resource from storage.
      *
      * @param  \App\Models\Group  $group
+     * @param  \Illuminate\Http\Request  $request
      * @return \Illuminate\Http\JsonResponse
      */
-    public function destroy(Group $group)
+    public function destroy(Group $group, Request $request)
     {
-        $this->authorize('delete', $group);
-
-        Groups::delete($group->id);
+        Groups::delete($group->id, $request->user());
 
         return response()->json(null, 204);
     }

app/Api/v1/Controllers/TwoFAccountController.php

@@ -73,7 +73,7 @@ class TwoFAccountController extends Controller
         $request->user()->twofaccounts()->save($twofaccount);
 
         // Possible group association
-        Groups::assign($twofaccount->id);
+        Groups::assign($twofaccount->id, $request->user());
 
         return (new TwoFAccountReadResource($twofaccount->refresh()))
             ->response()

app/Models/Group.php

@@ -78,4 +78,14 @@ class Group extends Model
     {
         return $this->hasMany(\App\Models\TwoFAccount::class);
     }
+
+   /**
+    * Get the user that owns the group.
+    *
+    * @return \Illuminate\Database\Eloquent\Relations\BelongsTo<\App\Models\User, \App\Models\Group>
+    */
+   public function user()
+   {
+       return $this->belongsTo(\App\Models\User::class);
+   }
 }

app/Models/TwoFAccount.php

@@ -178,6 +178,16 @@ class TwoFAccount extends Model implements Sortable
      */
     protected $generator = null;
 
+    /**
+     * Get the user that owns the twofaccount.
+     *
+     * @return \Illuminate\Database\Eloquent\Relations\BelongsTo<\App\Models\User, \App\Models\TwoFAccount>
+     */
+    public function user()
+    {
+        return $this->belongsTo(\App\Models\User::class);
+    }
+
     /**
      * Get legacy_uri attribute
      *

app/Services/GroupService.php

@@ -2,47 +2,47 @@
 
 namespace App\Services;
 
-use App\Facades\Settings;
 use App\Models\Group;
 use App\Models\TwoFAccount;
+use App\Models\User;
+use Illuminate\Auth\Access\AuthorizationException;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Support\Facades\Log;
 
 class GroupService
 {
     /**
-     * Prepends the pseudo group named 'All' to a group collection
+     * Returns all existing groups for the given user
      *
-     * @param  Collection<int, Group>  $groups
+     * @param  \App\Models\User  $user
      * @return Collection<int, Group>
      */
-    public static function prependTheAllGroup(Collection $groups, int $userId) : Collection
+    public static function getAll(User $user) : Collection
     {
-        $theAllGroup = new Group([
-            'name' => __('commons.all'),
-        ]);
-
-        $theAllGroup->id                 = 0;
-        $theAllGroup->twofaccounts_count = TwoFAccount::where('user_id', $userId)->count();
-
-        return $groups->prepend($theAllGroup);
+        return self::prependTheAllGroup($user->groups()->withCount('twofaccounts')->get(), $user->id);
     }
 
     /**
-     * Creates a group
+     * Creates a group for the given user
      *
      * @param  array  $data
+     * @param  \App\Models\User  $user
      * @return \App\Models\Group The created group
+     *
+     * @throws \Illuminate\Auth\Access\AuthorizationException
      */
-    public static function create(array $data) : Group
+    public static function create(array $data, User $user) : Group
     {
-        $group = Group::create([
+        if ($user->cannot('create', Group::class)) {
+            Log::notice(sprintf('User ID #%s cannot create groups', $user->id));
+            throw new AuthorizationException();
+        }
+
+        $group = $user->groups()->create([
             'name' => $data['name'],
         ]);
 
-        $group->save();
-
-        Log::info(sprintf('Group %s created', var_export($group->name, true)));
+        Log::info(sprintf('Group "%s" created for user ID #%s', var_export($group->name, true), $user->id));
 
         return $group;
     }
@@ -52,15 +52,23 @@ class GroupService
      *
      * @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
      */
-    public static function update(Group $group, array $data) : Group
+    public static function update(Group $group, array $data, User $user) : 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();
+        }
+
         $group->update([
             'name' => $data['name'],
         ]);
 
-        Log::info(sprintf('Group %s updated', var_export($group->name, true)));
+        Log::info(sprintf('Group "%s" updated by user ID #%s', var_export($group->name, true), $user->id));
 
         return $group;
     }
@@ -69,74 +77,113 @@ class GroupService
      * 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) : int
+    public static function delete($ids, User $user) : int
     {
-        $ids = is_array($ids) ? $ids : func_get_args();
+        $ids = is_array($ids) ? $ids : [$ids];
 
-        // A group is possibly set as the default group in Settings.
-        // In this case we reset the setting to "No group" (groupId = 0)
-        $defaultGroupId = Settings::get('defaultGroup');
+        $groups = Group::findMany($ids);
 
-        if (in_array($defaultGroupId, $ids)) {
-            Settings::set('defaultGroup', 0);
-        }
+        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();
+            }
 
-        // A group is also possibly set as the active group if the user
-        // configured 2FAuth to memorize the active group.
-        // In this case we reset the setting to the pseudo "All" group (groupId = 0)
-        $activeGroupId = Settings::get('activeGroup');
+            // 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();
+            }
 
-        if (in_array($activeGroupId, $ids)) {
-            Settings::set('activeGroup', 0);
-        }
+            // 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);
+            $deleted = Group::destroy($ids);
+            Log::info(sprintf('Groups IDs #%s deleted', implode(',#', $ids)));
 
-        Log::info(sprintf('Groups #%s deleted', implode(',#', $ids)));
+            return $deleted;
+        }
 
-        return $deleted;
+        return 0;
     }
 
     /**
-     * Assign one or more accounts to a group
+     * Assign one or more accounts to a user 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, Group $group = null) : void
+    public static function assign($ids, User $user, Group $group = null) : void
     {
         if (! $group) {
-            $group = self::defaultGroup();
+            $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();
+            }
         }
 
         if ($group) {
-            // saveMany() expect an iterable so we pass an array to
-            // find() to always obtain a list of TwoFAccount
-            if (! is_array($ids)) {
-                $ids = [$ids];
+            $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();
             }
-            $twofaccounts = TwoFAccount::find($ids);
 
             $group->twofaccounts()->saveMany($twofaccounts);
             $group->loadCount('twofaccounts');
 
-            Log::info(sprintf('Twofaccounts #%s assigned to groups %s', implode(',#', $ids), var_export($group->name, true)));
+            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');
         }
     }
 
     /**
-     * Determines the destination group
+     * Prepends the pseudo group named 'All' to a group collection
+     *
+     * @param  Collection<int, Group>  $groups
+     * @return Collection<int, Group>
+     */
+    private static function prependTheAllGroup(Collection $groups, int $userId) : Collection
+    {
+        $theAllGroup = new Group([
+            'name' => __('commons.all'),
+        ]);
+
+        $theAllGroup->id                 = 0;
+        $theAllGroup->twofaccounts_count = TwoFAccount::where('user_id', $userId)->count();
+
+        return $groups->prepend($theAllGroup);
+    }
+
+    /**
+     * Determines the default group of the given user
      *
+     * @param  \App\Models\User  $user
      * @return \App\Models\Group|null The group or null if it does not exist
      */
-    private static function defaultGroup()
+    private static function defaultGroup(User $user)
     {
-        $id = Settings::get('defaultGroup') === -1 ? (int) Settings::get('activeGroup') : (int) Settings::get('defaultGroup');
+        $id = $user->preferences['defaultGroup'] === -1 ? (int) $user->preferences['activeGroup'] : (int) $user->preferences['defaultGroup'];
 
         return Group::find($id);
     }

database/factories/TwoFAccountFactory.php

@@ -22,6 +22,7 @@ class TwoFAccountFactory extends Factory
         $secret = Base32::encodeUpper($this->faker->regexify('[A-Z0-9]{8}'));
     
         return [
+            'group_id' => null,
             'otp_type' => 'totp',
             'account' => $account,
             'service' => $service,

database/factories/UserFactory.php

@@ -26,9 +26,24 @@ class UserFactory extends Factory
             'email_verified_at' => now(),
             'password' => bcrypt(self::USER_PASSWORD),
             'remember_token' => Str::random(10),
+            'is_admin' => false,
         ];
     }
 
+    /**
+     * Indicate that the user is an administrator.
+     *
+     * @return \Illuminate\Database\Eloquent\Factories\Factory<\App\Models\User>
+     */
+    public function administrator()
+    {
+        return $this->state(function (array $attributes) {
+            return [
+                'is_admin' => true,
+            ];
+        });
+    }
+
     /**
      * Indicate that the model's email address should be unverified.
      *

tests/Api/v1/Controllers/GroupControllerTest.php

@@ -14,7 +14,7 @@ use Tests\FeatureTestCase;
 class GroupControllerTest extends FeatureTestCase
 {
     /**
-     * @var \App\Models\User
+     * @var \App\Models\User|\Illuminate\Contracts\Auth\Authenticatable
      */
     protected $user;
 
@@ -33,7 +33,7 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_index_returns_group_collection_with_pseudo_group()
     {
-        Group::factory()->count(3)->create();
+        Group::factory()->count(3)->for($this->user)->create();
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('GET', '/api/v1/groups')
@@ -86,7 +86,7 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_show_returns_group_resource()
     {
-        $group = Group::factory()->create([
+        $group = Group::factory()->for($this->user)->create([
             'name' => 'My group',
         ]);
 
@@ -117,7 +117,7 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_update_returns_updated_group_resource()
     {
-        $group = Group::factory()->create();
+        $group = Group::factory()->for($this->user)->create();
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('PUT', '/api/v1/groups/' . $group->id, [
@@ -150,7 +150,7 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_update_with_invalid_data_returns_validation_error()
     {
-        $group = Group::factory()->create();
+        $group = Group::factory()->for($this->user)->create();
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('PUT', '/api/v1/groups/' . $group->id, [
@@ -164,8 +164,8 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_assign_accounts_returns_updated_group_resource()
     {
-        $group    = Group::factory()->create();
-        $accounts = TwoFAccount::factory()->count(2)->create();
+        $group    = Group::factory()->for($this->user)->create();
+        $accounts = TwoFAccount::factory()->count(2)->for($this->user)->create();
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/groups/' . $group->id . '/assign', [
@@ -184,7 +184,7 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_assign_accounts_to_missing_group_returns_not_found()
     {
-        $accounts = TwoFAccount::factory()->count(2)->create();
+        $accounts = TwoFAccount::factory()->count(2)->for($this->user)->create();
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/groups/1000/assign', [
@@ -201,8 +201,8 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_assign_invalid_accounts_returns_validation_error()
     {
-        $group    = Group::factory()->create();
-        $accounts = TwoFAccount::factory()->count(2)->create();
+        $group    = Group::factory()->for($this->user)->create();
+        $accounts = TwoFAccount::factory()->count(2)->for($this->user)->create();
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/groups/' . $group->id . '/assign', [
@@ -216,8 +216,8 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_get_assigned_accounts_returns_twofaccounts_collection()
     {
-        $group    = Group::factory()->create();
-        $accounts = TwoFAccount::factory()->count(2)->create();
+        $group    = Group::factory()->for($this->user)->create();
+        $accounts = TwoFAccount::factory()->count(2)->for($this->user)->create();
 
         $assign = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/groups/' . $group->id . '/assign', [
@@ -248,8 +248,8 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_get_assigned_accounts_returns_twofaccounts_collection_with_secret()
     {
-        $group    = Group::factory()->create();
-        $accounts = TwoFAccount::factory()->count(2)->create();
+        $group    = Group::factory()->for($this->user)->create();
+        $accounts = TwoFAccount::factory()->count(2)->for($this->user)->create();
 
         $assign = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/groups/' . $group->id . '/assign', [
@@ -296,7 +296,7 @@ class GroupControllerTest extends FeatureTestCase
      */
     public function test_destroy_group_returns_success()
     {
-        $group = Group::factory()->create();
+        $group = Group::factory()->for($this->user)->create();
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('DELETE', '/api/v1/groups/' . $group->id)

tests/Feature/Services/GroupServiceTest.php

@@ -3,9 +3,12 @@
 namespace Tests\Feature\Services;
 
 use App\Facades\Groups;
-use App\Facades\Settings;
 use App\Models\Group;
 use App\Models\TwoFAccount;
+use App\Models\User;
+use App\Policies\GroupPolicy;
+use Illuminate\Auth\Access\AuthorizationException;
+use Mockery\MockInterface;
 use Tests\FeatureTestCase;
 
 /**
@@ -14,6 +17,16 @@ use Tests\FeatureTestCase;
  */
 class GroupServiceTest extends FeatureTestCase
 {
+    /**
+     * @var \App\Models\User|\Illuminate\Contracts\Auth\Authenticatable
+     */
+    protected $user;
+
+    /**
+     * @var \App\Models\User|\Illuminate\Contracts\Auth\Authenticatable
+     */
+    protected $admin;
+
     /**
      * App\Models\Group $groupOne, $groupTwo
      */
@@ -30,119 +43,107 @@ class GroupServiceTest extends FeatureTestCase
 
     private const NEW_GROUP_NAME = 'MyNewGroup';
 
-    private const TWOFACCOUNT_COUNT = 2;
-
-    private const ACCOUNT = 'account';
-
-    private const SERVICE = 'service';
-
-    private const SECRET = 'A4GRFHVVRBGY7UIW';
-
-    private const ALGORITHM_CUSTOM = 'sha256';
+    /**
+     * @test
+     */
+    public function setUp() : void
+    {
+        parent::setUp();
 
-    private const DIGITS_CUSTOM = 7;
+        $this->user  = User::factory()->create();
+        $this->admin = User::factory()->administrator()->create();
 
-    private const PERIOD_CUSTOM = 40;
+        $this->groupOne = Group::factory()->for($this->user)->create();
+        $this->groupTwo = Group::factory()->for($this->user)->create();
 
-    private const IMAGE = 'https%3A%2F%2Fen.opensuse.org%2Fimages%2F4%2F44%2FButton-filled-colour.png';
+        Group::factory()->count(3)->for($this->admin)->create();
 
-    private const ICON = 'test.png';
+        $this->twofaccountOne = TwoFAccount::factory()->for($this->user)->create([
+            'group_id' => $this->groupOne->id,
+        ]);
+        $this->twofaccountTwo = TwoFAccount::factory()->for($this->user)->create([
+            'group_id' => $this->groupTwo->id,
+        ]);
 
-    private const TOTP_FULL_CUSTOM_URI = 'otpauth://totp/' . self::SERVICE . ':' . self::ACCOUNT . '?secret=' . self::SECRET . '&issuer=' . self::SERVICE . '&digits=' . self::DIGITS_CUSTOM . '&period=' . self::PERIOD_CUSTOM . '&algorithm=' . self::ALGORITHM_CUSTOM . '&image=' . self::IMAGE;
+        TwoFAccount::factory()->for($this->admin)->create();
+    }
 
     /**
      * @test
      */
-    public function setUp() : void
+    public function test_getAll_returns_pseudo_group_on_top_of_user_groups_only()
     {
-        parent::setUp();
+        $groups = Groups::getAll($this->user);
 
-        $this->groupOne       = new Group;
-        $this->groupOne->name = 'MyGroupOne';
-        $this->groupOne->save();
-
-        $this->groupTwo       = new Group;
-        $this->groupTwo->name = 'MyGroupTwo';
-        $this->groupTwo->save();
-
-        $this->twofaccountOne             = new TwoFAccount;
-        $this->twofaccountOne->legacy_uri = self::TOTP_FULL_CUSTOM_URI;
-        $this->twofaccountOne->service    = self::SERVICE;
-        $this->twofaccountOne->account    = self::ACCOUNT;
-        $this->twofaccountOne->icon       = self::ICON;
-        $this->twofaccountOne->otp_type   = 'totp';
-        $this->twofaccountOne->secret     = self::SECRET;
-        $this->twofaccountOne->digits     = self::DIGITS_CUSTOM;
-        $this->twofaccountOne->algorithm  = self::ALGORITHM_CUSTOM;
-        $this->twofaccountOne->period     = self::PERIOD_CUSTOM;
-        $this->twofaccountOne->counter    = null;
-        $this->twofaccountOne->save();
-
-        $this->twofaccountTwo             = new TwoFAccount;
-        $this->twofaccountTwo->legacy_uri = self::TOTP_FULL_CUSTOM_URI;
-        $this->twofaccountTwo->service    = self::SERVICE;
-        $this->twofaccountTwo->account    = self::ACCOUNT;
-        $this->twofaccountTwo->icon       = self::ICON;
-        $this->twofaccountTwo->otp_type   = 'totp';
-        $this->twofaccountTwo->secret     = self::SECRET;
-        $this->twofaccountTwo->digits     = self::DIGITS_CUSTOM;
-        $this->twofaccountTwo->algorithm  = self::ALGORITHM_CUSTOM;
-        $this->twofaccountTwo->period     = self::PERIOD_CUSTOM;
-        $this->twofaccountTwo->counter    = null;
-        $this->twofaccountTwo->save();
+        $this->assertCount(3, $groups);
+        $this->assertEquals(0, $groups->first()->id);
+        $this->assertEquals(__('commons.all'), $groups->first()->name);
+        $this->assertEquals($this->groupOne->user_id, $groups[1]->user_id);
+        $this->assertEquals($this->groupTwo->user_id, $groups[2]->user_id);
     }
 
     /**
      * @test
      */
-    public function test_getAll_returns_a_collection()
+    public function test_getAll_returns_groups_with_count()
     {
-        $this->assertInstanceOf(\Illuminate\Database\Eloquent\Collection::class, Groups::getAll());
+        $groups = Groups::getAll($this->user);
+
+        $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_getAll_adds_pseudo_group_on_top_of_user_groups()
+    public function test_create_persists_and_returns_created_group()
     {
-        $groups = Groups::getAll();
+        $newGroup = Groups::create(['name' => self::NEW_GROUP_NAME], $this->user);
 
-        $this->assertEquals(0, $groups->first()->id);
-        $this->assertEquals(__('commons.all'), $groups->first()->name);
+        $this->assertDatabaseHas('groups', [
+            'name'    => self::NEW_GROUP_NAME,
+            'user_id' => $this->user->id,
+        ]);
+        $this->assertInstanceOf(Group::class, $newGroup);
+        $this->assertEquals(self::NEW_GROUP_NAME, $newGroup->name);
     }
 
     /**
      * @test
      */
-    public function test_getAll_returns_pseudo_group_with_all_twofaccounts_count()
+    public function test_create_authorization()
     {
-        $groups = Groups::getAll();
+        $this->mock(GroupPolicy::class, function (MockInterface $groupPolicy) {
+            $groupPolicy->shouldReceive('create')
+                ->andReturn(false);
+        });
+
+        $this->expectException(AuthorizationException::class);
 
-        $this->assertEquals(self::TWOFACCOUNT_COUNT, $groups->first()->twofaccounts_count);
+        Groups::create(['name' => 'lorem'], $this->user);
     }
 
     /**
      * @test
      */
-    public function test_create_persists_and_returns_created_group()
+    public function test_update_persists_and_returns_updated_group()
     {
-        $newGroup = Groups::create(['name' => self::NEW_GROUP_NAME]);
+        $this->groupOne = Groups::update($this->groupOne, ['name' => self::NEW_GROUP_NAME], $this->user);
 
         $this->assertDatabaseHas('groups', ['name' => self::NEW_GROUP_NAME]);
-        $this->assertInstanceOf(\App\Models\Group::class, $newGroup);
-        $this->assertEquals(self::NEW_GROUP_NAME, $newGroup->name);
+        $this->assertInstanceOf(Group::class, $this->groupOne);
+        $this->assertEquals(self::NEW_GROUP_NAME, $this->groupOne->name);
     }
 
     /**
      * @test
      */
-    public function test_update_persists_and_returns_updated_group()
+    public function test_update_fails_when_user_does_not_own_the_group()
     {
-        $this->groupOne = Groups::update($this->groupOne, ['name' => self::NEW_GROUP_NAME]);
+        $this->expectException(AuthorizationException::class);
 
-        $this->assertDatabaseHas('groups', ['name' => self::NEW_GROUP_NAME]);
-        $this->assertInstanceOf(\App\Models\Group::class, $this->groupOne);
-        $this->assertEquals(self::NEW_GROUP_NAME, $this->groupOne->name);
+        Groups::update($this->groupOne, ['name' => self::NEW_GROUP_NAME], $this->admin);
     }
 
     /**
@@ -150,7 +151,7 @@ class GroupServiceTest extends FeatureTestCase
      */
     public function test_delete_a_groupId_clear_db_and_returns_deleted_count()
     {
-        $deleted = Groups::delete($this->groupOne->id);
+        $deleted = Groups::delete($this->groupOne->id, $this->user);
 
         $this->assertDatabaseMissing('groups', ['id' => $this->groupOne->id]);
         $this->assertEquals(1, $deleted);
@@ -161,7 +162,7 @@ class GroupServiceTest extends FeatureTestCase
      */
     public function test_delete_an_array_of_ids_clear_db_and_returns_deleted_count()
     {
-        $deleted = Groups::delete([$this->groupOne->id, $this->groupTwo->id]);
+        $deleted = Groups::delete([$this->groupOne->id, $this->groupTwo->id], $this->user);
 
         $this->assertDatabaseMissing('groups', ['id' => $this->groupOne->id]);
         $this->assertDatabaseMissing('groups', ['id' => $this->groupTwo->id]);
@@ -171,32 +172,53 @@ class GroupServiceTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_delete_default_group_reset_defaultGroup_setting()
+    public function test_delete_missing_id_does_not_fail_and_returns_deleted_count()
     {
-        Settings::set('defaultGroup', $this->groupOne->id);
+        $this->assertDatabaseMissing('groups', ['id' => 1000]);
 
-        $deleted = Groups::delete($this->groupOne->id);
+        $deleted = Groups::delete([$this->groupOne->id, 1000], $this->user);
 
-        $this->assertDatabaseHas('options', [
-            'key'   => 'defaultGroup',
-            'value' => 0,
-        ]);
+        $this->assertDatabaseMissing('groups', ['id' => $this->groupOne->id]);
+        $this->assertEquals(1, $deleted);
     }
 
     /**
      * @test
      */
-    public function test_delete_active_group_reset_activeGroup_setting()
+    public function test_delete_default_group_reset_defaultGroup_preference()
     {
-        Settings::set('rememberActiveGroup', true);
-        Settings::set('activeGroup', $this->groupOne->id);
+        $this->user['preferences->defaultGroup'] = $this->groupOne->id;
+        $this->user->save();
 
-        $deleted = Groups::delete($this->groupOne->id);
+        Groups::delete($this->groupOne->id, $this->user);
 
-        $this->assertDatabaseHas('options', [
-            'key'   => 'activeGroup',
-            'value' => 0,
-        ]);
+        $this->user->refresh();
+        $this->assertEquals(0, $this->user->preferences['defaultGroup']);
+    }
+
+    /**
+     * @test
+     */
+    public function test_delete_active_group_reset_activeGroup_preference()
+    {
+        $this->user['preferences->rememberActiveGroup'] = true;
+        $this->user['preferences->activeGroup']         = $this->groupOne->id;
+        $this->user->save();
+
+        Groups::delete($this->groupOne->id, $this->user);
+
+        $this->user->refresh();
+        $this->assertEquals(0, $this->user->preferences['activeGroup']);
+    }
+
+    /**
+     * @test
+     */
+    public function test_delete_fails_when_user_does_not_own_one_of_the_groups()
+    {
+        $this->expectException(AuthorizationException::class);
+
+        Groups::delete($this->groupOne->id, $this->admin);
     }
 
     /**
@@ -204,11 +226,11 @@ class GroupServiceTest extends FeatureTestCase
      */
     public function test_assign_a_twofaccountid_to_a_specified_group_persists_the_relation()
     {
-        Groups::assign($this->twofaccountOne->id, $this->groupOne);
+        Groups::assign($this->twofaccountOne->id, $this->user, $this->groupTwo);
 
         $this->assertDatabaseHas('twofaccounts', [
             'id'       => $this->twofaccountOne->id,
-            'group_id' => $this->groupOne->id,
+            'group_id' => $this->groupTwo->id,
         ]);
     }
 
@@ -217,15 +239,15 @@ class GroupServiceTest extends FeatureTestCase
      */
     public function test_assign_multiple_twofaccountid_to_a_specified_group_persists_the_relation()
     {
-        Groups::assign([$this->twofaccountOne->id, $this->twofaccountTwo->id], $this->groupOne);
+        Groups::assign([$this->twofaccountOne->id, $this->twofaccountTwo->id], $this->user, $this->groupTwo);
 
         $this->assertDatabaseHas('twofaccounts', [
             'id'       => $this->twofaccountOne->id,
-            'group_id' => $this->groupOne->id,
+            'group_id' => $this->groupTwo->id,
         ]);
         $this->assertDatabaseHas('twofaccounts', [
             'id'       => $this->twofaccountTwo->id,
-            'group_id' => $this->groupOne->id,
+            'group_id' => $this->groupTwo->id,
         ]);
     }
 
@@ -234,9 +256,10 @@ class GroupServiceTest extends FeatureTestCase
      */
     public function test_assign_a_twofaccountid_to_no_group_assigns_to_default_group()
     {
-        Settings::set('defaultGroup', $this->groupTwo->id);
+        $this->user['preferences->defaultGroup'] = $this->groupTwo->id;
+        $this->user->save();
 
-        Groups::assign($this->twofaccountOne->id);
+        Groups::assign($this->twofaccountOne->id, $this->user);
 
         $this->assertDatabaseHas('twofaccounts', [
             'id'       => $this->twofaccountOne->id,
@@ -249,10 +272,11 @@ class GroupServiceTest extends FeatureTestCase
      */
     public function test_assign_a_twofaccountid_to_no_group_assigns_to_active_group()
     {
-        Settings::set('defaultGroup', -1);
-        Settings::set('activeGroup', $this->groupTwo->id);
+        $this->user['preferences->defaultGroup'] = -1;
+        $this->user['preferences->activeGroup']  = $this->groupTwo->id;
+        $this->user->save();
 
-        Groups::assign($this->twofaccountOne->id);
+        Groups::assign($this->twofaccountOne->id, $this->user);
 
         $this->assertDatabaseHas('twofaccounts', [
             'id'       => $this->twofaccountOne->id,
@@ -263,27 +287,39 @@ class GroupServiceTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_assign_a_twofaccountid_to_missing_active_group_does_not_fails()
+    public function test_assign_a_twofaccountid_to_missing_active_group_returns_not_found()
     {
-        Settings::set('defaultGroup', -1);
-        Settings::set('activeGroup', 100000);
+        $orginalGroup = $this->twofaccountOne->group_id;
+
+        $this->user['preferences->defaultGroup'] = -1;
+        $this->user['preferences->activeGroup']  = 1000;
+        $this->user->save();
 
-        Groups::assign($this->twofaccountOne->id);
+        Groups::assign($this->twofaccountOne->id, $this->user);
 
         $this->assertDatabaseHas('twofaccounts', [
             'id'       => $this->twofaccountOne->id,
-            'group_id' => null,
+            'group_id' => $orginalGroup,
         ]);
     }
 
     /**
      * @test
      */
-    public function test_getAccounts_returns_accounts()
+    public function test_assign_fails_when_user_does_not_own_the_group()
+    {
+        $this->expectException(AuthorizationException::class);
+
+        Groups::assign($this->twofaccountOne->id, $this->user, $this->admin->groups()->first());
+    }
+
+    /**
+     * @test
+     */
+    public function test_assign_fails_when_user_does_not_own_one_of_the_accounts()
     {
-        Groups::assign([$this->twofaccountOne->id, $this->twofaccountTwo->id], $this->groupOne);
-        $accounts = Groups::getAccounts($this->groupOne);
+        $this->expectException(AuthorizationException::class);
 
-        $this->assertEquals(2, $accounts->count());
+        Groups::assign([$this->twofaccountOne->id, $this->admin->twofaccounts()->first()->id], $this->user, $this->groupTwo);
     }
 }

tests/Unit/Api/v1/Controllers/GroupControllerTest.php

@@ -3,13 +3,16 @@
 namespace Tests\Unit\Api\v1\Controllers;
 
 use App\Api\v1\Controllers\GroupController;
+use App\Api\v1\Requests\GroupAssignRequest;
+use App\Api\v1\Requests\GroupStoreRequest;
+use App\Api\v1\Resources\GroupResource;
+use App\Api\v1\Resources\TwoFAccountReadResource;
 use App\Facades\Groups;
 use App\Models\Group;
-use App\Models\TwoFAccount;
-use App\Services\SettingService;
+use App\Models\User;
 use Illuminate\Foundation\Testing\WithoutMiddleware;
+use Illuminate\Http\Request;
 use Mockery;
-use Mockery\MockInterface;
 use Tests\TestCase;
 
 /**
@@ -29,11 +32,20 @@ class GroupControllerTest extends TestCase
      */
     protected $groupStoreRequest;
 
+    /**
+     * @var \Illuminate\Http\Request mocked request
+     */
+    protected $request;
+
     public function setUp() : void
     {
         parent::setUp();
 
-        $this->groupStoreRequest = Mockery::mock('App\Api\v1\Requests\GroupStoreRequest');
+        $this->groupStoreRequest = Mockery::mock(GroupStoreRequest::class);
+        $this->request           = Mockery::mock(Request::class);
+
+        $this->request->shouldReceive('user')
+            ->andReturn(new User());
 
         $this->controller = new GroupController();
     }
@@ -49,9 +61,9 @@ class GroupControllerTest extends TestCase
             ->once()
             ->andReturn($groups);
 
-        $response = $this->controller->index();
+        $response = $this->controller->index($this->request);
 
-        $this->assertContainsOnlyInstancesOf('App\Api\v1\Resources\GroupResource', $response->collection);
+        $this->assertContainsOnlyInstancesOf(GroupResource::class, $response->collection);
     }
 
     /**
@@ -61,9 +73,11 @@ class GroupControllerTest extends TestCase
     {
         $group = Group::factory()->make();
 
-        $this->groupStoreRequest->shouldReceive('validated')
-            ->once()
-            ->andReturn(['name' => $group->name]);
+        $this->groupStoreRequest->shouldReceive([
+            'validated' => ['name' => $group->name],
+            'user'      => new User(),
+        ])
+            ->once();
 
         Groups::shouldReceive('create')
             ->once()
@@ -71,7 +85,8 @@ class GroupControllerTest extends TestCase
 
         $response = $this->controller->store($this->groupStoreRequest);
 
-        $this->assertInstanceOf('App\Models\Group', $response->original);
+        $this->assertInstanceOf(Group::class, $response->original);
+        // $this->assertInstanceOf(GroupResource::class, $response);
     }
 
     /**
@@ -83,7 +98,7 @@ class GroupControllerTest extends TestCase
 
         $response = $this->controller->show($group);
 
-        $this->assertInstanceOf('App\Api\v1\Resources\GroupResource', $response);
+        $this->assertInstanceOf(GroupResource::class, $response);
     }
 
     /**
@@ -93,9 +108,11 @@ class GroupControllerTest extends TestCase
     {
         $group = Group::factory()->make();
 
-        $this->groupStoreRequest->shouldReceive('validated')
-            ->once()
-            ->andReturn(['name' => $group->name]);
+        $this->groupStoreRequest->shouldReceive([
+            'validated' => ['name' => $group->name],
+            'user'      => new User(),
+        ])
+            ->once();
 
         Groups::shouldReceive('update')
             ->once()
@@ -103,7 +120,7 @@ class GroupControllerTest extends TestCase
 
         $response = $this->controller->update($this->groupStoreRequest, $group);
 
-        $this->assertInstanceOf('App\Api\v1\Resources\GroupResource', $response);
+        $this->assertInstanceOf(GroupResource::class, $response);
     }
 
     /**
@@ -112,19 +129,22 @@ class GroupControllerTest extends TestCase
     public function test_assignAccounts_returns_api_resource_assigned_using_groupService()
     {
         $group              = Group::factory()->make();
-        $groupAssignRequest = Mockery::mock('App\Api\v1\Requests\GroupAssignRequest');
+        $groupAssignRequest = Mockery::mock(GroupAssignRequest::class);
+        $user               = new User();
 
-        $groupAssignRequest->shouldReceive('validated')
-            ->once()
-            ->andReturn(['ids' => $group->id]);
+        $groupAssignRequest->shouldReceive([
+            'validated' => ['ids' => $group->id],
+            'user'      => $user,
+        ])
+            ->once();
 
         Groups::shouldReceive('assign')
-            ->with($group->id, $group)
+            ->with($group->id, $user, $group)
             ->once();
 
         $response = $this->controller->assignAccounts($groupAssignRequest, $group);
 
-        $this->assertInstanceOf('App\Api\v1\Resources\GroupResource', $response);
+        $this->assertInstanceOf(GroupResource::class, $response);
     }
 
     /**
@@ -134,21 +154,9 @@ class GroupControllerTest extends TestCase
     {
         $group = Group::factory()->make();
 
-        $settingService = $this->mock(SettingService::class, function (MockInterface $settingService) {
-            $settingService->shouldReceive('get')
-                ->andReturn(false);
-        });
-
-        $twofaccounts = TwoFAccount::factory()->count(3)->make();
+        $response = $this->controller->accounts($group, $this->request);
 
-        Groups::shouldReceive('getAccounts')
-            ->with($group)
-            ->once()
-            ->andReturn($twofaccounts);
-
-        $response = $this->controller->accounts($group);
-        // TwoFAccountCollection
-        $this->assertContainsOnlyInstancesOf('App\Api\v1\Resources\TwoFAccountReadResource', $response->collection);
+        $this->assertContainsOnlyInstancesOf(TwoFAccountReadResource::class, $response->collection);
     }
 
     /**
@@ -156,13 +164,15 @@ class GroupControllerTest extends TestCase
      */
     public function test_destroy_uses_group_service()
     {
-        $group = Group::factory()->make();
+        $group     = Group::factory()->make();
+        $group->id = 0;
 
         Groups::shouldReceive('delete')
             ->once()
-            ->with($group->id);
+            ->with($group->id, $this->request->user())
+            ->andReturn(0);
 
-        $response = $this->controller->destroy($group);
+        $response = $this->controller->destroy($group, $this->request);
 
         $this->assertInstanceOf('Illuminate\Http\JsonResponse', $response);
     }