Merge pull request #308 from kenjis/fix-UserModel-findById

fix: UserModel::findById() does not work with withIdentities()

Author: kenjis

src/Models/UserModel.php

@@ -6,6 +6,7 @@
 use CodeIgniter\Model;
 use CodeIgniter\Shield\Authentication\Authenticators\Session;
 use CodeIgniter\Shield\Entities\User;
+use CodeIgniter\Shield\Entities\UserIdentity;
 use CodeIgniter\Shield\Exceptions\InvalidArgumentException;
 use CodeIgniter\Shield\Exceptions\ValidationException;
 use Faker\Generator;
@@ -82,27 +83,44 @@ protected function fetchIdentities(array $data): array
             return $data;
         }
 
-        // Map our users by ID to make assigning simpler
+        $mappedUsers = $this->assignIdentities($data, $identities);
+
+        $data['data'] = $data['singleton'] ? $mappedUsers[$data['id']] : $mappedUsers;
+
+        return $data;
+    }
+
+    /**
+     * Map our users by ID to make assigning simpler
+     *
+     * @param array          $data       Event $data
+     * @param UserIdentity[] $identities
+     *
+     * @return User[] UserId => User object
+     * @phpstan-return array<int|string, User> UserId => User object
+     */
+    private function assignIdentities(array $data, array $identities): array
+    {
         $mappedUsers = [];
-        $users       = $data['singleton']
-            ? $data
-            : $data['data'];
+
+        $users = $data['singleton'] ? [$data['data']] : $data['data'];
 
         foreach ($users as $user) {
             $mappedUsers[$user->id] = $user;
         }
         unset($users);
 
         // Now assign the identities to the user
-        foreach ($identities as $id) {
-            $array                                 = $mappedUsers[$id->user_id]->identities;
-            $array[]                               = $id;
-            $mappedUsers[$id->user_id]->identities = $array;
-        }
+        foreach ($identities as $identity) {
+            $userId = $identity->user_id;
 
-        $data['data'] = $mappedUsers;
+            $newIdentities   = $mappedUsers[$userId]->identities;
+            $newIdentities[] = $identity;
 
-        return $data;
+            $mappedUsers[$userId]->identities = $newIdentities;
+        }
+
+        return $mappedUsers;
     }
 
     /**

tests/Unit/UserTest.php

@@ -53,7 +53,7 @@ public function testGetIdentitiesByType(): void
         $this->assertEmpty($this->user->getIdentities('foo'));
     }
 
-    public function testModelWithIdentities(): void
+    public function testModelfindAllWithIdentities(): void
     {
         fake(UserModel::class);
         fake(UserIdentityModel::class, ['user_id' => $this->user->id, 'type' => 'password']);
@@ -67,6 +67,18 @@ public function testModelWithIdentities(): void
         $this->assertCount(2, $identities);
     }
 
+    public function testModelfindByIdWithIdentities(): void
+    {
+        fake(UserModel::class);
+        fake(UserIdentityModel::class, ['user_id' => $this->user->id, 'type' => 'password']);
+        fake(UserIdentityModel::class, ['user_id' => $this->user->id, 'type' => 'access_token']);
+
+        // Grab the user again, using the model's identity helper
+        $user = model(UserModel::class)->withIdentities()->findById(1);
+
+        $this->assertCount(2, $user->identities);
+    }
+
     public function testLastLogin(): void
     {
         fake(