Update Authorizations, Logs and Tests for TwoFAccounts management

app/Api/v1/Controllers/TwoFAccountController.php

@@ -133,7 +133,7 @@ class TwoFAccountController extends Controller
         $validated = $request->validated();
 
         $twofaccounts = TwoFAccount::whereIn('id', $validated['orderedIds'])->get();
-        $this->authorize('updateEach', [$twofaccounts[0], $twofaccounts]);
+        $this->authorize('updateEach', [new TwoFAccount(), $twofaccounts]);
 
         TwoFAccount::setNewOrder($validated['orderedIds']);
 
@@ -172,7 +172,7 @@ class TwoFAccountController extends Controller
         }
 
         $twofaccounts = TwoFAccounts::export($validated['ids']);
-        $this->authorize('viewEach', [$twofaccounts[0], $twofaccounts]);
+        $this->authorize('viewEach', [new TwoFAccount(), $twofaccounts]);
 
         return new TwoFAccountExportCollection($twofaccounts);
     }
@@ -250,7 +250,7 @@ class TwoFAccountController extends Controller
         $ids          = Helpers::commaSeparatedToArray($validated['ids']);
         $twofaccounts = TwoFAccount::whereIn('id', $ids)->get();
 
-        $this->authorize('updateEach', [$twofaccounts[0], $twofaccounts]);
+        $this->authorize('updateEach', [new TwoFAccount(), $twofaccounts]);
 
         TwoFAccounts::withdraw($ids);
 
@@ -267,7 +267,7 @@ class TwoFAccountController extends Controller
     {
         $this->authorize('delete', $twofaccount);
 
-        TwoFAccounts::delete($twofaccount->id);
+        $twofaccount->delete();
 
         return response()->json(null, 204);
     }
@@ -292,7 +292,7 @@ class TwoFAccountController extends Controller
         $ids          = Helpers::commaSeparatedToArray($validated['ids']);
         $twofaccounts = TwoFAccount::whereIn('id', $ids)->get();
 
-        $this->authorize('deleteEach', [$twofaccounts[0], $twofaccounts]);
+        $this->authorize('deleteEach', [new TwoFAccount(), $twofaccounts]);
 
         TwoFAccounts::delete($validated['ids']);
 

app/Models/TwoFAccount.php

@@ -177,9 +177,15 @@ class TwoFAccount extends Model implements Sortable
             }
         });
 
-        // static::deleted(function ($model) {
+        static::created(function (object $model) {
-        //     Log::info(sprintf('TwoFAccount #%d deleted', $model->id));
+            Log::info(sprintf('TwoFAccount ID #%d created for user ID #%s', $model->id, $model->user_id));
-        // });
+        });
+        static::updated(function (object $model) {
+            Log::info(sprintf('TwoFAccount ID #%d updated by user ID #%s', $model->id, $model->user_id));
+        });
+        static::deleted(function (object $model) {
+            Log::info(sprintf('TwoFAccount ID #%d deleted ', $model->id));
+        });
     }
 
     /**
@@ -408,7 +414,9 @@ class TwoFAccount extends Model implements Sortable
             $this->enforceAsSteam();
         }
 
-        if (! $this->icon && Auth::user()->preferences['getOfficialIcons'] && ! $skipIconFetching) {
+        $user = is_null($this->user) ? Auth::user() : $this->user;
+
+        if (! $this->icon && $user->preferences['getOfficialIcons'] && ! $skipIconFetching) {
             $this->icon = $this->getDefaultIcon();
         }
 
@@ -460,7 +468,9 @@ class TwoFAccount extends Model implements Sortable
             self::setIcon($this->generator->getParameter('image'));
         }
 
-        if (! $this->icon && Auth::user()->preferences['getOfficialIcons'] && ! $skipIconFetching) {
+        $user = is_null($this->user) ? Auth::user() : $this->user;
+        
+        if (! $this->icon && $user->preferences['getOfficialIcons'] && ! $skipIconFetching) {
             $this->icon = $this->getDefaultIcon();
         }
 
@@ -697,8 +707,9 @@ class TwoFAccount extends Model implements Sortable
     private function getDefaultIcon()
     {
         $logoService = App::make(LogoService::class);
+        $user = is_null($this->user) ? Auth::user() : $this->user;
 
-        return Auth::user()->preferences['getOfficialIcons'] ? $logoService->getIcon($this->service) : null;
+        return $user->preferences['getOfficialIcons'] ? $logoService->getIcon($this->service) : null;
     }
 
     /**

app/Policies/TwoFAccountPolicy.php

@@ -5,6 +5,7 @@ namespace App\Policies;
 use App\Models\TwoFAccount;
 use App\Models\User;
 use Illuminate\Auth\Access\HandlesAuthorization;
+use Illuminate\Support\Facades\Log;
 
 class TwoFAccountPolicy
 {
@@ -30,7 +31,13 @@ class TwoFAccountPolicy
      */
     public function view(User $user, TwoFAccount $twofaccount)
     {
-        return $this->isOwnerOf($user, $twofaccount);
+        $can = $this->isOwnerOf($user, $twofaccount);
+
+        if (! $can) {
+            Log::notice(sprintf('User ID #%s cannot view twofaccount ID #%s', $user->id, $twofaccount->id));
+        }
+
+        return $can;
     }
 
     /**
@@ -43,7 +50,16 @@ class TwoFAccountPolicy
      */
     public function viewEach(User $user, TwoFAccount $twofaccount, $twofaccounts)
     {
-        return $this->isOwnerOfEach($user, $twofaccounts);
+        $can = $this->isOwnerOfEach($user, $twofaccounts);
+
+        if (! $can) {
+            $ids = $twofaccounts->map(function ($twofaccount, $key) {
+                return $twofaccount->id;
+            });
+            Log::notice(sprintf('User ID #%s cannot view all twofaccounts in IDs #%s', $user->id, implode(',', $ids->toArray())));
+        }
+
+        return $can;
     }
 
     /**
@@ -54,6 +70,8 @@ class TwoFAccountPolicy
      */
     public function create(User $user)
     {
+        // Log::notice(sprintf('User ID #%s cannot create twofaccounts', $user->id));
+
         return true;
     }
 
@@ -66,7 +84,13 @@ class TwoFAccountPolicy
      */
     public function update(User $user, TwoFAccount $twofaccount)
     {
-        return $this->isOwnerOf($user, $twofaccount);
+        $can = $this->isOwnerOf($user, $twofaccount);
+
+        if (! $can) {
+            Log::notice(sprintf('User ID #%s cannot update twofaccount ID #%s', $user->id, $twofaccount->id));
+        }
+
+        return $can;
     }
 
     /**
@@ -79,7 +103,16 @@ class TwoFAccountPolicy
      */
     public function updateEach(User $user, TwoFAccount $twofaccount, $twofaccounts)
     {
-        return $this->isOwnerOfEach($user, $twofaccounts);
+        $can = $this->isOwnerOfEach($user, $twofaccounts);
+
+        if (! $can) {
+            $ids = $twofaccounts->map(function ($twofaccount, $key) {
+                return $twofaccount->id;
+            });
+            Log::notice(sprintf('User ID #%s cannot update all twofaccounts in IDs #%s', $user->id, implode(',', $ids->toArray())));
+        }
+
+        return $can;
     }
 
     /**
@@ -91,7 +124,13 @@ class TwoFAccountPolicy
      */
     public function delete(User $user, TwoFAccount $twofaccount)
     {
-        return $this->isOwnerOf($user, $twofaccount);
+        $can = $this->isOwnerOf($user, $twofaccount);
+
+        if (! $can) {
+            Log::notice(sprintf('User ID #%s cannot delete twofaccount ID #%s', $user->id, $twofaccount->id));
+        }
+
+        return $can;
     }
 
     /**
@@ -104,7 +143,16 @@ class TwoFAccountPolicy
      */
     public function deleteEach(User $user, TwoFAccount $twofaccount, $twofaccounts)
     {
-        return $this->isOwnerOfEach($user, $twofaccounts);
+        $can = $this->isOwnerOfEach($user, $twofaccounts);
+
+        if (! $can) {
+            $ids = $twofaccounts->map(function ($twofaccount, $key) {
+                return $twofaccount->id;
+            });
+            Log::notice(sprintf('User ID #%s cannot delete all twofaccounts in IDs #%s', $user->id, implode(',', $ids->toArray())));
+        }
+
+        return $can;
     }
 
     /**

app/Services/TwoFAccountService.php

@@ -42,7 +42,7 @@ class TwoFAccountService
                         ['group_id' => null]
                     );
 
-        Log::info(sprintf('TwoFAccounts #%s withdrawn', implode(',#', $ids)));
+        Log::info(sprintf('TwoFAccounts IDs #%s withdrawn', implode(',', $ids)));
     }
 
     /**

database/factories/TwoFAccountFactory.php

@@ -18,7 +18,7 @@ class TwoFAccountFactory extends Factory
     public function definition()
     {
         $account = $this->faker->safeEmail();
-        $service = $this->faker->unique()->domainName();
+        $service = $this->faker->domainName();
         $secret = Base32::encodeUpper($this->faker->regexify('[A-Z0-9]{8}'));
     
         return [

tests/Api/v1/Controllers/TwoFAccountControllerTest.php

@@ -23,14 +23,19 @@ use Tests\FeatureTestCase;
 class TwoFAccountControllerTest extends FeatureTestCase
 {
     /**
-     * @var \App\Models\User
+     * @var \App\Models\User|\Illuminate\Contracts\Auth\Authenticatable
      */
-    protected $user;
+    protected $user, $anotherUser;
 
     /**
-     * @var \App\Models\Group
+     * @var App\Models\Group
      */
-    protected $group;
+    protected $userGroupA, $userGroupB, $anotherUserGroupA, $anotherUserGroupB;
+
+    /**
+     * @var App\Models\TwoFAccount
+     */
+    protected $twofaccountA, $twofaccountB, $twofaccountC, $twofaccountD;
 
     private const VALID_RESOURCE_STRUCTURE_WITHOUT_SECRET = [
         'id',
@@ -129,8 +134,27 @@ class TwoFAccountControllerTest extends FeatureTestCase
     {
         parent::setUp();
 
-        $this->user  = User::factory()->create();
+        $this->user = User::factory()->create();
-        $this->group = Group::factory()->create();
+        $this->userGroupA = Group::factory()->for($this->user)->create();
+        $this->userGroupB = Group::factory()->for($this->user)->create();
+
+        $this->twofaccountA = TwoFAccount::factory()->for($this->user)->create([
+            'group_id' => $this->userGroupA->id,
+        ]);
+        $this->twofaccountB = TwoFAccount::factory()->for($this->user)->create([
+            'group_id' => $this->userGroupA->id,
+        ]);
+
+        $this->anotherUser = User::factory()->create();
+        $this->anotherUserGroupA = Group::factory()->for($this->anotherUser)->create();
+        $this->anotherUserGroupB = Group::factory()->for($this->anotherUser)->create();
+
+        $this->twofaccountC = TwoFAccount::factory()->for($this->anotherUser)->create([
+            'group_id' => $this->anotherUserGroupA->id,
+        ]);
+        $this->twofaccountD = TwoFAccount::factory()->for($this->anotherUser)->create([
+            'group_id' => $this->anotherUserGroupB->id,
+        ]);
     }
 
     /**
@@ -138,16 +162,26 @@ class TwoFAccountControllerTest extends FeatureTestCase
      *
      * @dataProvider indexUrlParameterProvider
      */
-    public function test_index_returns_twofaccount_collection($urlParameter, $expected)
+    public function test_index_returns_user_twofaccounts_only($urlParameter, $expected)
     {
-        TwoFAccount::factory()->count(3)->create();
-
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('GET', '/api/v1/twofaccounts' . $urlParameter)
             ->assertOk()
-            ->assertJsonCount(3, $key = null)
+            ->assertJsonCount(2, $key = null)
             ->assertJsonStructure([
                 '*' => $expected,
+            ])
+            ->assertJsonFragment([
+                'id' => $this->twofaccountA->id,
+            ])
+            ->assertJsonFragment([
+                'id' => $this->twofaccountB->id,
+            ])
+            ->assertJsonMissing([
+                'id' => $this->twofaccountC->id,
+            ])
+            ->assertJsonMissing([
+                'id' => $this->twofaccountD->id,
             ]);
     }
 
@@ -171,12 +205,10 @@ class TwoFAccountControllerTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_show_twofaccount_returns_twofaccount_resource_with_secret()
+    public function test_show_returns_twofaccount_resource_with_secret()
     {
-        $twofaccount = TwoFAccount::factory()->create();
-
         $response = $this->actingAs($this->user, 'api-guard')
-            ->json('GET', '/api/v1/twofaccounts/' . $twofaccount->id)
+            ->json('GET', '/api/v1/twofaccounts/' . $this->twofaccountA->id)
             ->assertOk()
             ->assertJsonStructure(self::VALID_RESOURCE_STRUCTURE_WITH_SECRET);
     }
@@ -184,12 +216,10 @@ class TwoFAccountControllerTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_show_twofaccount_returns_twofaccount_resource_without_secret()
+    public function test_show_returns_twofaccount_resource_without_secret()
     {
-        $twofaccount = TwoFAccount::factory()->create();
-
         $response = $this->actingAs($this->user, 'api-guard')
-            ->json('GET', '/api/v1/twofaccounts/' . $twofaccount->id . '?withSecret=0')
+            ->json('GET', '/api/v1/twofaccounts/' . $this->twofaccountA->id . '?withSecret=0')
             ->assertOk()
             ->assertJsonStructure(self::VALID_RESOURCE_STRUCTURE_WITHOUT_SECRET);
     }
@@ -232,6 +262,19 @@ class TwoFAccountControllerTest extends FeatureTestCase
             ]);
     }
 
+    /**
+     * @test
+     */
+    public function test_show_twofaccount_of_another_user_is_forbidden()
+    {
+        $response = $this->actingAs($this->user, 'api-guard')
+            ->json('GET', '/api/v1/twofaccounts/' . $this->twofaccountC->id)
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
+            ]);
+    }
+
     /**
      * @dataProvider accountCreationProvider
      * @test
@@ -331,14 +374,15 @@ class TwoFAccountControllerTest extends FeatureTestCase
     public function test_store_assigns_created_account_when_default_group_is_a_specific_one()
     {
         // Set the default group to a specific one
-        Settings::set('defaultGroup', $this->group->id);
+        $this->user['preferences->defaultGroup'] = $this->userGroupA->id;
+        $this->user->save();
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/twofaccounts', [
                 'uri' => OtpTestData::TOTP_SHORT_URI,
             ])
             ->assertJsonFragment([
-                'group_id' => $this->group->id,
+                'group_id' => $this->userGroupA->id,
             ]);
     }
 
@@ -348,16 +392,17 @@ class TwoFAccountControllerTest extends FeatureTestCase
     public function test_store_assigns_created_account_when_default_group_is_the_active_one()
     {
         // Set the default group to be the active one
-        Settings::set('defaultGroup', -1);
+        $this->user['preferences->defaultGroup'] = -1;
         // Set the active group
-        Settings::set('activeGroup', $this->group->id);
+        $this->user['preferences->activeGroup'] = $this->userGroupA->id;
+        $this->user->save();
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/twofaccounts', [
                 'uri' => OtpTestData::TOTP_SHORT_URI,
             ])
             ->assertJsonFragment([
-                'group_id' => $this->group->id,
+                'group_id' => $this->userGroupA->id,
             ]);
     }
 
@@ -367,7 +412,8 @@ class TwoFAccountControllerTest extends FeatureTestCase
     public function test_store_assigns_created_account_when_default_group_is_no_group()
     {
         // Set the default group to No group
-        Settings::set('defaultGroup', 0);
+        $this->user['preferences->defaultGroup'] = 0;
+        $this->user->save();
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/twofaccounts', [
@@ -384,7 +430,8 @@ class TwoFAccountControllerTest extends FeatureTestCase
     public function test_store_assigns_created_account_when_default_group_does_not_exist()
     {
         // Set the default group to a non-existing one
-        Settings::set('defaultGroup', 1000);
+        $this->user['preferences->defaultGroup'] = 1000;
+        $this->user->save();
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/twofaccounts', [
@@ -400,10 +447,8 @@ class TwoFAccountControllerTest extends FeatureTestCase
      */
     public function test_update_totp_returns_success_with_updated_resource()
     {
-        $twofaccount = TwoFAccount::factory()->create();
-
         $response = $this->actingAs($this->user, 'api-guard')
-            ->json('PUT', '/api/v1/twofaccounts/' . $twofaccount->id, OtpTestData::ARRAY_OF_FULL_VALID_PARAMETERS_FOR_CUSTOM_TOTP)
+            ->json('PUT', '/api/v1/twofaccounts/' . $this->twofaccountA->id, OtpTestData::ARRAY_OF_FULL_VALID_PARAMETERS_FOR_CUSTOM_TOTP)
             ->assertOk()
             ->assertJsonFragment(self::JSON_FRAGMENTS_FOR_CUSTOM_TOTP);
     }
@@ -413,10 +458,8 @@ class TwoFAccountControllerTest extends FeatureTestCase
      */
     public function test_update_hotp_returns_success_with_updated_resource()
     {
-        $twofaccount = TwoFAccount::factory()->create();
-
         $response = $this->actingAs($this->user, 'api-guard')
-            ->json('PUT', '/api/v1/twofaccounts/' . $twofaccount->id, OtpTestData::ARRAY_OF_FULL_VALID_PARAMETERS_FOR_CUSTOM_HOTP)
+            ->json('PUT', '/api/v1/twofaccounts/' . $this->twofaccountA->id, OtpTestData::ARRAY_OF_FULL_VALID_PARAMETERS_FOR_CUSTOM_HOTP)
             ->assertOk()
             ->assertJsonFragment(self::JSON_FRAGMENTS_FOR_CUSTOM_HOTP);
     }
@@ -439,10 +482,23 @@ class TwoFAccountControllerTest extends FeatureTestCase
         $twofaccount = TwoFAccount::factory()->create();
 
         $response = $this->actingAs($this->user, 'api-guard')
-            ->json('PUT', '/api/v1/twofaccounts/' . $twofaccount->id, self::ARRAY_OF_INVALID_PARAMETERS)
+            ->json('PUT', '/api/v1/twofaccounts/' . $this->twofaccountA->id, self::ARRAY_OF_INVALID_PARAMETERS)
             ->assertStatus(422);
     }
 
+    /**
+     * @test
+     */
+    public function test_update_twofaccount_of_another_user_is_forbidden()
+    {
+        $response = $this->actingAs($this->user, 'api-guard')
+            ->json('PUT', '/api/v1/twofaccounts/' . $this->twofaccountC->id, OtpTestData::ARRAY_OF_FULL_VALID_PARAMETERS_FOR_CUSTOM_HOTP)
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
+            ]);
+    }
+
     /**
      * @test
      */
@@ -721,11 +777,9 @@ class TwoFAccountControllerTest extends FeatureTestCase
      */
     public function test_reorder_returns_success()
     {
-        TwoFAccount::factory()->count(3)->create();
-
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/twofaccounts/reorder', [
-                'orderedIds' => [3, 2, 1],
+                'orderedIds' => [$this->twofaccountB->id, $this->twofaccountA->id],
             ])
             ->assertStatus(200)
             ->assertJsonStructure([
@@ -738,8 +792,6 @@ class TwoFAccountControllerTest extends FeatureTestCase
      */
     public function test_reorder_with_invalid_data_returns_validation_error()
     {
-        TwoFAccount::factory()->count(3)->create();
-
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/twofaccounts/reorder', [
                 'orderedIds' => '3,2,1',
@@ -747,6 +799,21 @@ class TwoFAccountControllerTest extends FeatureTestCase
             ->assertStatus(422);
     }
 
+    /**
+     * @test
+     */
+    public function test_reorder_twofaccounts_of_another_user_is_forbidden()
+    {
+        $response = $this->actingAs($this->user, 'api-guard')
+            ->json('POST', '/api/v1/twofaccounts/reorder', [
+                'orderedIds' => [$this->twofaccountB->id, $this->twofaccountD->id],
+            ])
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
+            ]);
+    }
+
     /**
      * @test
      */
@@ -792,7 +859,7 @@ class TwoFAccountControllerTest extends FeatureTestCase
      */
     public function test_get_otp_using_totp_twofaccount_id_returns_consistent_resource()
     {
-        $twofaccount = TwoFAccount::factory()->create([
+        $twofaccount = TwoFAccount::factory()->for($this->user)->create([
             'otp_type'   => 'totp',
             'account'    => OtpTestData::ACCOUNT,
             'service'    => OtpTestData::SERVICE,
@@ -851,7 +918,7 @@ class TwoFAccountControllerTest extends FeatureTestCase
      */
     public function test_get_otp_using_hotp_twofaccount_id_returns_consistent_resource()
     {
-        $twofaccount = TwoFAccount::factory()->create([
+        $twofaccount = TwoFAccount::factory()->for($this->user)->create([
             'otp_type'   => 'hotp',
             'account'    => OtpTestData::ACCOUNT,
             'service'    => OtpTestData::SERVICE,
@@ -929,7 +996,7 @@ class TwoFAccountControllerTest extends FeatureTestCase
     {
         Settings::set('useEncryption', true);
 
-        $twofaccount = TwoFAccount::factory()->create();
+        $twofaccount = TwoFAccount::factory()->for($this->user)->create();
 
         DB::table('twofaccounts')
             ->where('id', $twofaccount->id)
@@ -980,15 +1047,26 @@ class TwoFAccountControllerTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_count_returns_right_number_of_twofaccount()
+    public function test_get_otp_of_another_user_twofaccount_is_forbidden()
     {
-        TwoFAccount::factory()->count(3)->create();
+        $response = $this->actingAs($this->user, 'api-guard')
+            ->json('GET', '/api/v1/twofaccounts/'.$this->twofaccountC->id.'/otp')
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
+            ]);
+    }
 
+    /**
+     * @test
+     */
+    public function test_count_returns_right_number_of_twofaccounts()
+    {
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('GET', '/api/v1/twofaccounts/count')
             ->assertStatus(200)
             ->assertExactJson([
-                'count' => 3,
+                'count' => 2,
             ]);
     }
 
@@ -997,11 +1075,8 @@ class TwoFAccountControllerTest extends FeatureTestCase
      */
     public function test_withdraw_returns_success()
     {
-        TwoFAccount::factory()->count(3)->create();
-        $ids = DB::table('twofaccounts')->pluck('id')->implode(',');
-
         $response = $this->actingAs($this->user, 'api-guard')
-            ->json('PATCH', '/api/v1/twofaccounts/withdraw?ids=1,2,3' . $ids)
+            ->json('PATCH', '/api/v1/twofaccounts/withdraw?ids=1,2')
             ->assertOk()
             ->assertJsonStructure([
                 'message',
@@ -1013,8 +1088,9 @@ class TwoFAccountControllerTest extends FeatureTestCase
      */
     public function test_withdraw_too_many_ids_returns_bad_request()
     {
-        TwoFAccount::factory()->count(102)->create();
+        TwoFAccount::factory()->count(102)->for($this->user)->create();
-        $ids = DB::table('twofaccounts')->pluck('id')->implode(',');
+
+        $ids = DB::table('twofaccounts')->where('user_id', $this->user->id)->pluck('id')->implode(',');
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('PATCH', '/api/v1/twofaccounts/withdraw?ids=' . $ids)
@@ -1030,10 +1106,8 @@ class TwoFAccountControllerTest extends FeatureTestCase
      */
     public function test_destroy_twofaccount_returns_success()
     {
-        $twofaccount = TwoFAccount::factory()->create();
-
         $response = $this->actingAs($this->user, 'api-guard')
-            ->json('DELETE', '/api/v1/twofaccounts/' . $twofaccount->id)
+            ->json('DELETE', '/api/v1/twofaccounts/' . $this->twofaccountA->id)
             ->assertNoContent();
     }
 
@@ -1042,23 +1116,35 @@ class TwoFAccountControllerTest extends FeatureTestCase
      */
     public function test_destroy_missing_twofaccount_returns_not_found()
     {
-        $twofaccount = TwoFAccount::factory()->create();
-
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('DELETE', '/api/v1/twofaccounts/1000')
             ->assertNotFound();
     }
 
+    /**
+     * @test
+     */
+    public function test_destroy_twofaccount_of_another_user_is_forbidden()
+    {
+        $response = $this->actingAs($this->user, 'api-guard')
+        ->json('DELETE', '/api/v1/twofaccounts/' . $this->twofaccountC->id)
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
+            ]);
+    }
+
     /**
      * @test
      */
     public function test_batch_destroy_twofaccount_returns_success()
     {
-        TwoFAccount::factory()->count(3)->create();
+        TwoFAccount::factory()->count(3)->for($this->user)->create();
-        $ids = DB::table('twofaccounts')->pluck('id')->implode(',');
+
+        $ids = DB::table('twofaccounts')->where('user_id', $this->user->id)->pluck('id')->implode(',');
 
         $response = $this->actingAs($this->user, 'api-guard')
-            ->json('DELETE', '/api/v1/twofaccounts?ids=' . $ids)
+            ->json('DELETE', '/api/v1/twofaccounts?ids=' . $this->twofaccountA->id . ',' . $this->twofaccountB->id)
             ->assertNoContent();
     }
 
@@ -1067,8 +1153,9 @@ class TwoFAccountControllerTest extends FeatureTestCase
      */
     public function test_batch_destroy_too_many_twofaccounts_returns_bad_request()
     {
-        TwoFAccount::factory()->count(102)->create();
+        TwoFAccount::factory()->count(102)->for($this->user)->create();
-        $ids = DB::table('twofaccounts')->pluck('id')->implode(',');
+
+        $ids = DB::table('twofaccounts')->where('user_id', $this->user->id)->pluck('id')->implode(',');
 
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('DELETE', '/api/v1/twofaccounts?ids=' . $ids)
@@ -1078,4 +1165,24 @@ class TwoFAccountControllerTest extends FeatureTestCase
                 'reason',
             ]);
     }
+
+    /**
+     * @test
+     */
+    public function test_batch_destroy_twofaccount_of_another_user_is_forbidden()
+    {
+        TwoFAccount::factory()->count(2)->for($this->anotherUser)->create();
+
+        $ids = DB::table('twofaccounts')
+            ->where('user_id', $this->anotherUser->id)
+            ->pluck('id')
+            ->implode(',');
+
+        $response = $this->actingAs($this->user, 'api-guard')
+        ->json('DELETE', '/api/v1/twofaccounts?ids=' . $ids)
+            ->assertForbidden()
+            ->assertJsonStructure([
+                'message',
+            ]);
+    }
 }

tests/Feature/Services/TwoFAccountServiceTest.php

@@ -5,6 +5,7 @@ namespace Tests\Feature\Services;
 use App\Facades\TwoFAccounts;
 use App\Models\Group;
 use App\Models\TwoFAccount;
+use App\Models\User;
 use Tests\Data\MigrationTestData;
 use Tests\Data\OtpTestData;
 use Tests\FeatureTestCase;
@@ -16,19 +17,19 @@ use Tests\FeatureTestCase;
 class TwoFAccountServiceTest extends FeatureTestCase
 {
     /**
-     * App\Models\TwoFAccount $customTotpTwofaccount
+     * @var \App\Models\User|\Illuminate\Contracts\Auth\Authenticatable
      */
-    protected $customTotpTwofaccount;
+    protected $user;
 
     /**
-     * App\Models\Group $group
+     * @var \App\Models\TwoFAccount
      */
-    protected $group;
+    protected $customTotpTwofaccount, $customHotpTwofaccount;
 
     /**
-     * App\Models\TwoFAccount $customTotpTwofaccount
+     * @var \App\Models\Group
      */
-    protected $customHotpTwofaccount;
+    protected $userGroupA, $userGroupB;
 
     /**
      * @test
@@ -36,36 +37,36 @@ class TwoFAccountServiceTest extends FeatureTestCase
     public function setUp() : void
     {
         parent::setUp();
+        
+        $this->user = User::factory()->create();
+        $this->userGroupA = Group::factory()->for($this->user)->create();
+        $this->userGroupB = Group::factory()->for($this->user)->create();
 
-        $this->customTotpTwofaccount             = new TwoFAccount;
+        $this->customTotpTwofaccount = TwoFAccount::factory()->for($this->user)->create([
-        $this->customTotpTwofaccount->legacy_uri = OtpTestData::TOTP_FULL_CUSTOM_URI;
+            'legacy_uri' => OtpTestData::TOTP_FULL_CUSTOM_URI,
-        $this->customTotpTwofaccount->service    = OtpTestData::SERVICE;
+            'service'    => OtpTestData::SERVICE,
-        $this->customTotpTwofaccount->account    = OtpTestData::ACCOUNT;
+            'account'    => OtpTestData::ACCOUNT,
-        $this->customTotpTwofaccount->icon       = OtpTestData::ICON_PNG;
+            'icon'       => OtpTestData::ICON_PNG,
-        $this->customTotpTwofaccount->otp_type   = 'totp';
+            'otp_type'   => 'totp',
-        $this->customTotpTwofaccount->secret     = OtpTestData::SECRET;
+            'secret'     => OtpTestData::SECRET,
-        $this->customTotpTwofaccount->digits     = OtpTestData::DIGITS_CUSTOM;
+            'digits'     => OtpTestData::DIGITS_CUSTOM,
-        $this->customTotpTwofaccount->algorithm  = OtpTestData::ALGORITHM_CUSTOM;
+            'algorithm'  => OtpTestData::ALGORITHM_CUSTOM,
-        $this->customTotpTwofaccount->period     = OtpTestData::PERIOD_CUSTOM;
+            'period'     => OtpTestData::PERIOD_CUSTOM,
-        $this->customTotpTwofaccount->counter    = null;
+            'counter'    => null,
-        $this->customTotpTwofaccount->save();
+        ]);
 
-        $this->customHotpTwofaccount             = new TwoFAccount;
+        $this->customHotpTwofaccount = TwoFAccount::factory()->for($this->user)->create([
-        $this->customHotpTwofaccount->legacy_uri = OtpTestData::HOTP_FULL_CUSTOM_URI;
+            'legacy_uri' => OtpTestData::HOTP_FULL_CUSTOM_URI,
-        $this->customHotpTwofaccount->service    = OtpTestData::SERVICE;
+            'service'    => OtpTestData::SERVICE,
-        $this->customHotpTwofaccount->account    = OtpTestData::ACCOUNT;
+            'account'    => OtpTestData::ACCOUNT,
-        $this->customHotpTwofaccount->icon       = OtpTestData::ICON_PNG;
+            'icon'       => OtpTestData::ICON_PNG,
-        $this->customHotpTwofaccount->otp_type   = 'hotp';
+            'otp_type'   => 'hotp',
-        $this->customHotpTwofaccount->secret     = OtpTestData::SECRET;
+            'secret'     => OtpTestData::SECRET,
-        $this->customHotpTwofaccount->digits     = OtpTestData::DIGITS_CUSTOM;
+            'digits'     => OtpTestData::DIGITS_CUSTOM,
-        $this->customHotpTwofaccount->algorithm  = OtpTestData::ALGORITHM_CUSTOM;
+            'algorithm'  => OtpTestData::ALGORITHM_CUSTOM,
-        $this->customHotpTwofaccount->period     = null;
+            'period'     => null,
-        $this->customHotpTwofaccount->counter    = OtpTestData::COUNTER_CUSTOM;
+            'counter'    => OtpTestData::COUNTER_CUSTOM,
-        $this->customHotpTwofaccount->save();
+        ]);
-
-        $this->group       = new Group;
-        $this->group->name = 'MyGroup';
-        $this->group->save();
     }
 
     /**
@@ -74,7 +75,17 @@ class TwoFAccountServiceTest extends FeatureTestCase
     public function test_withdraw_comma_separated_ids_deletes_relation()
     {
         $twofaccounts = collect([$this->customHotpTwofaccount, $this->customTotpTwofaccount]);
-        $this->group->twofaccounts()->saveMany($twofaccounts);
+        $this->userGroupA->twofaccounts()->saveMany($twofaccounts);
+
+        $this->assertDatabaseHas('twofaccounts', [
+            'id'       => $this->customHotpTwofaccount->id,
+            'group_id' => $this->userGroupA->id,
+        ]);
+
+        $this->assertDatabaseHas('twofaccounts', [
+            'id'       => $this->customTotpTwofaccount->id,
+            'group_id' => $this->userGroupA->id,
+        ]);
 
         TwoFAccounts::withdraw($this->customHotpTwofaccount->id . ',' . $this->customTotpTwofaccount->id);
 
@@ -92,11 +103,20 @@ class TwoFAccountServiceTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_withdraw_array_of_model_ids_deletes_relation()
+    public function test_withdraw_array_of_ids_deletes_relation()
     {
         $twofaccounts = collect([$this->customHotpTwofaccount, $this->customTotpTwofaccount]);
-        $this->group->twofaccounts()->saveMany($twofaccounts);
+        $this->userGroupA->twofaccounts()->saveMany($twofaccounts);
 
+        $this->assertDatabaseHas('twofaccounts', [
+            'id'       => $this->customHotpTwofaccount->id,
+            'group_id' => $this->userGroupA->id,
+        ]);
+
+        $this->assertDatabaseHas('twofaccounts', [
+            'id'       => $this->customTotpTwofaccount->id,
+            'group_id' => $this->userGroupA->id,
+        ]);
         TwoFAccounts::withdraw([$this->customHotpTwofaccount->id, $this->customTotpTwofaccount->id]);
 
         $this->assertDatabaseHas('twofaccounts', [
@@ -116,7 +136,12 @@ class TwoFAccountServiceTest extends FeatureTestCase
     public function test_withdraw_single_id_deletes_relation()
     {
         $twofaccounts = collect([$this->customHotpTwofaccount, $this->customTotpTwofaccount]);
-        $this->group->twofaccounts()->saveMany($twofaccounts);
+        $this->userGroupA->twofaccounts()->saveMany($twofaccounts);
+
+        $this->assertDatabaseHas('twofaccounts', [
+            'id'       => $this->customTotpTwofaccount->id,
+            'group_id' => $this->userGroupA->id,
+        ]);
 
         TwoFAccounts::withdraw($this->customTotpTwofaccount->id);
 
@@ -137,50 +162,10 @@ class TwoFAccountServiceTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_delete_comma_separated_ids()
+    public function test_migrate_from_gauth_returns_correct_accounts()
     {
-        TwoFAccounts::delete($this->customHotpTwofaccount->id . ',' . $this->customTotpTwofaccount->id);
+        $this->actingAs($this->user);
 
-        $this->assertDatabaseMissing('twofaccounts', [
-            'id' => $this->customTotpTwofaccount->id,
-        ]);
-        $this->assertDatabaseMissing('twofaccounts', [
-            'id' => $this->customHotpTwofaccount->id,
-        ]);
-    }
-
-    /**
-     * @test
-     */
-    public function test_delete_array_of_ids()
-    {
-        TwoFAccounts::delete([$this->customTotpTwofaccount->id, $this->customHotpTwofaccount->id]);
-
-        $this->assertDatabaseMissing('twofaccounts', [
-            'id' => $this->customTotpTwofaccount->id,
-        ]);
-        $this->assertDatabaseMissing('twofaccounts', [
-            'id' => $this->customHotpTwofaccount->id,
-        ]);
-    }
-
-    /**
-     * @test
-     */
-    public function test_delete_single_id()
-    {
-        TwoFAccounts::delete($this->customTotpTwofaccount->id);
-
-        $this->assertDatabaseMissing('twofaccounts', [
-            'id' => $this->customTotpTwofaccount->id,
-        ]);
-    }
-
-    /**
-     * @test
-     */
-    public function test_convert_migration_from_gauth_returns_correct_accounts()
-    {
         $twofaccounts = TwoFAccounts::migrate(MigrationTestData::GOOGLE_AUTH_MIGRATION_URI);
 
         $this->assertCount(2, $twofaccounts);
@@ -207,8 +192,10 @@ class TwoFAccountServiceTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_convert_migration_from_gauth_returns_flagged_duplicates()
+    public function test_migrate_from_gauth_returns_flagged_duplicates()
     {
+        $this->actingAs($this->user);
+        
         $parameters = [
             'service'   => OtpTestData::SERVICE,
             'account'   => OtpTestData::ACCOUNT,
@@ -237,9 +224,75 @@ class TwoFAccountServiceTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_convert_invalid_migration_from_gauth_returns_InvalidMigrationData_exception()
+    public function test_migrate_invalid_migration_from_gauth_returns_InvalidMigrationData_exception()
     {
         $this->expectException(\App\Exceptions\InvalidMigrationDataException::class);
         $twofaccounts = TwoFAccounts::migrate(MigrationTestData::GOOGLE_AUTH_MIGRATION_URI_WITH_INVALID_DATA);
     }
+
+    /**
+     * @test
+     */
+    public function test_delete_comma_separated_ids()
+    {
+        $twofaccounts = TwoFAccount::factory()->count(2)->for($this->user)->create();
+
+        $this->assertDatabaseHas('twofaccounts', [
+            'id' => $twofaccounts[0]->id,
+        ]);
+        $this->assertDatabaseHas('twofaccounts', [
+            'id' => $twofaccounts[1]->id,
+        ]);
+
+        TwoFAccounts::delete($twofaccounts[0]->id . ',' . $twofaccounts[1]->id);
+
+        $this->assertDatabaseMissing('twofaccounts', [
+            'id' => $twofaccounts[0]->id,
+        ]);
+        $this->assertDatabaseMissing('twofaccounts', [
+            'id' => $twofaccounts[1]->id,
+        ]);
+    }
+
+    /**
+     * @test
+     */
+    public function test_delete_array_of_ids()
+    {
+        $twofaccounts = TwoFAccount::factory()->count(2)->for($this->user)->create();
+
+        $this->assertDatabaseHas('twofaccounts', [
+            'id' => $twofaccounts[0]->id,
+        ]);
+        $this->assertDatabaseHas('twofaccounts', [
+            'id' => $twofaccounts[1]->id,
+        ]);
+
+        TwoFAccounts::delete([$twofaccounts[0]->id, $twofaccounts[1]->id]);
+
+        $this->assertDatabaseMissing('twofaccounts', [
+            'id' => $twofaccounts[0]->id,
+        ]);
+        $this->assertDatabaseMissing('twofaccounts', [
+            'id' => $twofaccounts[1]->id,
+        ]);
+    }
+
+    /**
+     * @test
+     */
+    public function test_delete_single_id()
+    {
+        $twofaccount = TwoFAccount::factory()->for($this->user)->create();
+
+        $this->assertDatabaseHas('twofaccounts', [
+            'id' => $twofaccount->id,
+        ]);
+        
+        TwoFAccounts::delete($twofaccount->id);
+
+        $this->assertDatabaseMissing('twofaccounts', [
+            'id' => $twofaccount->id,
+        ]);
+    }
 }