Fix duplicate detection being made on all twofaccounts in db

app/Services/TwoFAccountService.php

@@ -6,6 +6,7 @@
 use App\Helpers\Helpers;
 use App\Models\TwoFAccount;
 use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Log;
 
 class TwoFAccountService
@@ -93,17 +94,18 @@ public static function delete($ids) : int
     }
 
     /**
-     * Return the given collection with items marked as Duplicates (using id=-1) if a similar record exists in database
+     * Return the given collection with items marked as Duplicates (using id=-1) if similar records exist
+     * in the authenticated user accounts
      *
      * @param  \Illuminate\Support\Collection<int|string, TwoFAccount>  $twofaccounts
      * @return \Illuminate\Support\Collection<int|string, TwoFAccount>
      */
     private static function markAsDuplicate(Collection $twofaccounts) : Collection
     {
-        $storage = TwoFAccount::all();
+        $userTwofaccounts = Auth::user()->twofaccounts;
 
-        $twofaccounts = $twofaccounts->map(function ($twofaccount, $key) use ($storage) {
+        $twofaccounts = $twofaccounts->map(function ($twofaccount, $key) use ($userTwofaccounts) {
-            if ($storage->contains(function ($value, $key) use ($twofaccount) {
+            if ($userTwofaccounts->contains(function ($value, $key) use ($twofaccount) {
                 return $value->secret == $twofaccount->secret
                     && $value->service == $twofaccount->service
                     && $value->account == $twofaccount->account

tests/Api/v1/Controllers/TwoFAccountControllerTest.php

@@ -541,7 +541,7 @@ public function test_update_twofaccount_of_another_user_is_forbidden()
     /**
      * @test
      */
-    public function test_import_valid_gauth_payload_returns_success_with_consistent_resources()
+    public function test_migrate_valid_gauth_payload_returns_success_with_consistent_resources()
     {
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/twofaccounts/migration', [
@@ -577,7 +577,7 @@ public function test_import_valid_gauth_payload_returns_success_with_consistent_
     /**
      * @test
      */
-    public function test_import_with_invalid_gauth_payload_returns_validation_error()
+    public function test_migrate_with_invalid_gauth_payload_returns_validation_error()
     {
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/twofaccounts/migration', [
@@ -589,9 +589,9 @@ public function test_import_with_invalid_gauth_payload_returns_validation_error(
     /**
      * @test
      */
-    public function test_import_gauth_payload_with_duplicates_returns_negative_ids()
+    public function test_migrate_payload_with_duplicates_returns_negative_ids()
     {
-        $twofaccount = TwoFAccount::factory()->create([
+        $twofaccount = TwoFAccount::factory()->for($this->user)->create([
             'otp_type'   => 'totp',
             'account'    => OtpTestData::ACCOUNT,
             'service'    => OtpTestData::SERVICE,
@@ -604,7 +604,7 @@ public function test_import_gauth_payload_with_duplicates_returns_negative_ids()
         ]);
 
         $response = $this->actingAs($this->user, 'api-guard')
-            ->json('POST', '/api/v1/twofaccounts/migration', [
+            ->json('POST', '/api/v1/twofaccounts/migration?withSecret=1', [
                 'payload' => MigrationTestData::GOOGLE_AUTH_MIGRATION_URI,
             ])
             ->assertOk()
@@ -612,13 +612,76 @@ public function test_import_gauth_payload_with_duplicates_returns_negative_ids()
                 'id'        => -1,
                 'service'   => OtpTestData::SERVICE,
                 'account'   => OtpTestData::ACCOUNT,
+                'otp_type'  => 'totp',
+                'secret'    => OtpTestData::SECRET,
+                'digits'    => OtpTestData::DIGITS_DEFAULT,
+                'algorithm' => OtpTestData::ALGORITHM_DEFAULT,
+                'period'    => OtpTestData::PERIOD_DEFAULT,
+                'counter'   => null,
+            ])
+            ->assertJsonFragment([
+                'id'        => 0,
+                'service'   => OtpTestData::SERVICE . '_bis',
+                'account'   => OtpTestData::ACCOUNT . '_bis',
+                'otp_type'  => 'totp',
+                'secret'    => OtpTestData::SECRET,
+                'digits'    => OtpTestData::DIGITS_DEFAULT,
+                'algorithm' => OtpTestData::ALGORITHM_DEFAULT,
+                'period'    => OtpTestData::PERIOD_DEFAULT,
+                'counter'   => null,
             ]);
     }
 
     /**
      * @test
      */
-    public function test_import_invalid_gauth_payload_returns_bad_request()
+    public function test_migrate_identify_duplicates_in_authenticated_user_twofaccounts_only()
+    {
+        $twofaccount = TwoFAccount::factory()->for($this->anotherUser)->create([
+            'otp_type'   => 'totp',
+            'account'    => OtpTestData::ACCOUNT,
+            'service'    => OtpTestData::SERVICE,
+            'secret'     => OtpTestData::SECRET,
+            'algorithm'  => OtpTestData::ALGORITHM_DEFAULT,
+            'digits'     => OtpTestData::DIGITS_DEFAULT,
+            'period'     => OtpTestData::PERIOD_DEFAULT,
+            'legacy_uri' => OtpTestData::TOTP_SHORT_URI,
+            'icon'       => '',
+        ]);
+
+        $response = $this->actingAs($this->user, 'api-guard')
+            ->json('POST', '/api/v1/twofaccounts/migration?withSecret=1', [
+                'payload' => MigrationTestData::GOOGLE_AUTH_MIGRATION_URI,
+            ])
+            ->assertOk()
+            ->assertJsonFragment([
+                'id'         => 0,
+                'account'    => OtpTestData::ACCOUNT,
+                'service'    => OtpTestData::SERVICE,
+                'otp_type'   => 'totp',
+                'secret'     => OtpTestData::SECRET,
+                'algorithm'  => OtpTestData::ALGORITHM_DEFAULT,
+                'digits'     => OtpTestData::DIGITS_DEFAULT,
+                'period'     => OtpTestData::PERIOD_DEFAULT,
+                'icon'       => null,
+            ])
+            ->assertJsonFragment([
+                'id'        => 0,
+                'service'   => OtpTestData::SERVICE . '_bis',
+                'account'   => OtpTestData::ACCOUNT . '_bis',
+                'otp_type'  => 'totp',
+                'secret'    => OtpTestData::SECRET,
+                'digits'    => OtpTestData::DIGITS_DEFAULT,
+                'algorithm' => OtpTestData::ALGORITHM_DEFAULT,
+                'period'    => OtpTestData::PERIOD_DEFAULT,
+                'counter'   => null,
+            ]);
+    }
+
+    /**
+     * @test
+     */
+    public function test_migrate_invalid_gauth_payload_returns_bad_request()
     {
         $response = $this->actingAs($this->user, 'api-guard')
             ->json('POST', '/api/v1/twofaccounts/migration', [
@@ -633,7 +696,7 @@ public function test_import_invalid_gauth_payload_returns_bad_request()
     /**
      * @test
      */
-    public function test_import_valid_aegis_json_file_returns_success()
+    public function test_migrate_valid_aegis_json_file_returns_success()
     {
         $file = LocalFile::fake()->validAegisJsonFile();
 
@@ -685,7 +748,7 @@ public function test_import_valid_aegis_json_file_returns_success()
      *
      * @dataProvider invalidAegisJsonFileProvider
      */
-    public function test_import_invalid_aegis_json_file_returns_bad_request($file)
+    public function test_migrate_invalid_aegis_json_file_returns_bad_request($file)
     {
         $response = $this->withHeaders(['Content-Type' => 'multipart/form-data'])
             ->actingAs($this->user, 'api-guard')
@@ -715,7 +778,7 @@ public function invalidAegisJsonFileProvider()
      *
      * @dataProvider validPlainTextFileProvider
      */
-    public function test_import_valid_plain_text_file_returns_success($file)
+    public function test_migrate_valid_plain_text_file_returns_success($file)
     {
         $response = $this->withHeaders(['Content-Type' => 'multipart/form-data'])
             ->actingAs($this->user, 'api-guard')
@@ -780,7 +843,7 @@ public function validPlainTextFileProvider()
      *
      * @dataProvider invalidPlainTextFileProvider
      */
-    public function test_import_invalid_plain_text_file_returns_bad_request($file)
+    public function test_migrate_invalid_plain_text_file_returns_bad_request($file)
     {
         $response = $this->withHeaders(['Content-Type' => 'multipart/form-data'])
             ->actingAs($this->user, 'api-guard')