Skip to content

Commit 3cf4f0b

Browse files
kenjiskenjis
authored
Merge pull request #452 from kenjis/fix-User-identites
fix: bug that when UserIdentity is changed, the correct user identities are not returned afterwards
2 parents 9b889b3 + 691a59f commit 3cf4f0b

File tree

6 files changed

+92
-6
lines changed

6 files changed

+92
-6
lines changed

‎src/Entities/User.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ public function createEmailIdentity(array $credentials): void
121121
$identityModel = model(UserIdentityModel::class);
122122

123123
$identityModel->createEmailIdentity($this, $credentials);
124+
125+
// Ensure we will reload all identities
126+
$this->identities = null;
124127
}
125128

126129
/**
@@ -151,6 +154,7 @@ public function saveEmailIdentity(): bool
151154
'email' => $this->email,
152155
'password' => '',
153156
]);
157+
154158
$identity = $this->getEmailIdentity();
155159
}
156160

‎src/Entities/UserIdentity.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121
* though a Authenticator may want to enforce only one exists for that
2222
* user, like a password.
2323
*
24-
* @property Time|null $last_used_at
24+
* @property Time|null $last_used_at
25+
* @property string|null $secret
26+
* @property string|null $secret2
2527
*/
2628
class UserIdentity extends Entity
2729
{

‎src/Models/UserIdentityModel.php‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use CodeIgniter\Shield\Entities\AccessToken;
1313
use CodeIgniter\Shield\Entities\User;
1414
use CodeIgniter\Shield\Entities\UserIdentity;
15+
use CodeIgniter\Shield\Exceptions\LogicException;
1516
use Faker\Generator;
1617

1718
class UserIdentityModel extends Model
@@ -59,6 +60,8 @@ public function create($data): void
5960
*/
6061
public function createEmailIdentity(User $user, array $credentials): void
6162
{
63+
$this->checkUserId($user);
64+
6265
/** @var Passwords $passwords */
6366
$passwords = service('passwords');
6467

@@ -72,6 +75,15 @@ public function createEmailIdentity(User $user, array $credentials): void
7275
$this->checkQueryReturn($return);
7376
}
7477

78+
private function checkUserId(User $user): void
79+
{
80+
if ($user->id === null) {
81+
throw new LogicException(
82+
'"$user->id" is null. You should not use the incomplete User object.'
83+
);
84+
}
85+
}
86+
7587
/**
7688
* Create an identity with 6 digits code for auth action
7789
*
@@ -85,7 +97,7 @@ public function createCodeIdentity(
8597
array $data,
8698
callable $codeGenerator
8799
): string {
88-
assert($user->id !== null);
100+
$this->checkUserId($user);
89101

90102
helper('text');
91103

@@ -120,6 +132,8 @@ public function createCodeIdentity(
120132
*/
121133
public function generateAccessToken(User $user, string $name, array $scopes = ['*']): AccessToken
122134
{
135+
$this->checkUserId($user);
136+
123137
helper('text');
124138

125139
$return = $this->insert([
@@ -153,6 +167,8 @@ public function getAccessTokenByRawToken(string $rawToken): ?AccessToken
153167

154168
public function getAccessToken(User $user, string $rawToken): ?AccessToken
155169
{
170+
$this->checkUserId($user);
171+
156172
return $this->where('user_id', $user->id)
157173
->where('type', AccessTokens::ID_TYPE_ACCESS_TOKEN)
158174
->where('secret', hash('sha256', $rawToken))
@@ -167,6 +183,8 @@ public function getAccessToken(User $user, string $rawToken): ?AccessToken
167183
*/
168184
public function getAccessTokenById($id, User $user): ?AccessToken
169185
{
186+
$this->checkUserId($user);
187+
170188
return $this->where('user_id', $user->id)
171189
->where('type', AccessTokens::ID_TYPE_ACCESS_TOKEN)
172190
->where('id', $id)
@@ -179,6 +197,8 @@ public function getAccessTokenById($id, User $user): ?AccessToken
179197
*/
180198
public function getAllAccessTokens(User $user): array
181199
{
200+
$this->checkUserId($user);
201+
182202
return $this
183203
->where('user_id', $user->id)
184204
->where('type', AccessTokens::ID_TYPE_ACCESS_TOKEN)
@@ -208,6 +228,8 @@ public function getIdentityBySecret(string $type, ?string $secret): ?UserIdentit
208228
*/
209229
public function getIdentities(User $user): array
210230
{
231+
$this->checkUserId($user);
232+
211233
return $this->where('user_id', $user->id)->orderBy($this->primaryKey)->findAll();
212234
}
213235

@@ -226,6 +248,8 @@ public function getIdentitiesByUserIds(array $userIds): array
226248
*/
227249
public function getIdentityByType(User $user, string $type): ?UserIdentity
228250
{
251+
$this->checkUserId($user);
252+
229253
return $this->where('user_id', $user->id)
230254
->where('type', $type)
231255
->orderBy($this->primaryKey)
@@ -241,6 +265,8 @@ public function getIdentityByType(User $user, string $type): ?UserIdentity
241265
*/
242266
public function getIdentitiesByTypes(User $user, array $types): array
243267
{
268+
$this->checkUserId($user);
269+
244270
if ($types === []) {
245271
return [];
246272
}
@@ -265,6 +291,8 @@ public function touchIdentity(UserIdentity $identity): void
265291

266292
public function deleteIdentitiesByType(User $user, string $type): void
267293
{
294+
$this->checkUserId($user);
295+
268296
$return = $this->where('user_id', $user->id)
269297
->where('type', $type)
270298
->delete();
@@ -277,6 +305,8 @@ public function deleteIdentitiesByType(User $user, string $type): void
277305
*/
278306
public function revokeAccessToken(User $user, string $rawToken): void
279307
{
308+
$this->checkUserId($user);
309+
280310
$return = $this->where('user_id', $user->id)
281311
->where('type', AccessTokens::ID_TYPE_ACCESS_TOKEN)
282312
->where('secret', hash('sha256', $rawToken))
@@ -290,6 +320,8 @@ public function revokeAccessToken(User $user, string $rawToken): void
290320
*/
291321
public function revokeAllAccessTokens(User $user): void
292322
{
323+
$this->checkUserId($user);
324+
293325
$return = $this->where('user_id', $user->id)
294326
->where('type', AccessTokens::ID_TYPE_ACCESS_TOKEN)
295327
->delete();

‎src/Models/UserModel.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,8 @@ public function activate(User $user): void
225225
*/
226226
public function insert($data = null, bool $returnID = true)
227227
{
228-
$this->tempUser = $data instanceof User ? $data : null;
228+
// Clone User object for not changing the passed object.
229+
$this->tempUser = $data instanceof User ? clone $data : null;
229230

230231
$result = parent::insert($data, $returnID);
231232

@@ -245,7 +246,8 @@ public function insert($data = null, bool $returnID = true)
245246
*/
246247
public function update($id = null, $data = null): bool
247248
{
248-
$this->tempUser = $data instanceof User ? $data : null;
249+
// Clone User object for not changing the passed object.
250+
$this->tempUser = $data instanceof User ? clone $data : null;
249251

250252
try {
251253
/** @throws DataException */
@@ -303,6 +305,9 @@ protected function saveEmailIdentity(array $data): array
303305
/** @var User $user */
304306
$user = $this->find($this->db->insertID());
305307

308+
// If you get identity (email/password), the User object must have the id.
309+
$this->tempUser->id = $user->id;
310+
306311
$user->email = $this->tempUser->email ?? '';
307312
$user->password = $this->tempUser->password ?? '';
308313
$user->password_hash = $this->tempUser->password_hash ?? '';

‎tests/Unit/UserModelTest.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace Tests\Unit;
66

77
use CodeIgniter\Shield\Entities\User;
8+
use CodeIgniter\Shield\Exceptions\LogicException;
89
use CodeIgniter\Shield\Models\UserModel;
910
use CodeIgniter\Test\DatabaseTestTrait;
1011
use Tests\Support\TestCase;
@@ -62,6 +63,22 @@ public function testInsertUserObject(): void
6263
]);
6364
}
6465

66+
/**
67+
* @see https://github.com/codeigniter4/shield/issues/450
68+
*/
69+
public function testSaveNewUserAndGetEmailIdentity(): void
70+
{
71+
$this->expectException(LogicException::class);
72+
$this->expectExceptionMessage('"$user->id" is null. You should not use the incomplete User object.');
73+
74+
$users = $this->createUserModel();
75+
$user = $this->createNewUser();
76+
77+
$users->save($user);
78+
79+
$user->getEmailIdentity();
80+
}
81+
6582
/**
6683
* This test is not correct.
6784
*

‎tests/Unit/UserTest.php‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public function testGetIdentitiesByType(): void
5555
$this->assertEmpty($this->user->getIdentities('foo'));
5656
}
5757

58-
public function testModelfindAllWithIdentities(): void
58+
public function testModelFindAllWithIdentities(): void
5959
{
6060
fake(UserModel::class);
6161
fake(UserIdentityModel::class, ['user_id' => $this->user->id, 'type' => 'password']);
@@ -69,7 +69,7 @@ public function testModelfindAllWithIdentities(): void
6969
$this->assertCount(2, $identities);
7070
}
7171

72-
public function testModelfindByIdWithIdentities(): void
72+
public function testModelFindByIdWithIdentities(): void
7373
{
7474
fake(UserModel::class);
7575
fake(UserIdentityModel::class, ['user_id' => $this->user->id, 'type' => 'password']);
@@ -216,4 +216,30 @@ public function testUpdatePasswordHash(): void
216216
'secret2' => $hash,
217217
]);
218218
}
219+
220+
public function testCreateEmailIdentity(): void
221+
{
222+
$identity = $this->user->getEmailIdentity();
223+
$this->assertNull($identity);
224+
225+
$this->user->createEmailIdentity([
226+
'email' => 'foo@example.com',
227+
'password' => 'passbar',
228+
]);
229+
230+
$identity = $this->user->getEmailIdentity();
231+
$this->assertSame('foo@example.com', $identity->secret);
232+
}
233+
234+
public function testSaveEmailIdentity(): void
235+
{
236+
$hash = service('passwords')->hash('passbar');
237+
$this->user->email = 'foo@example.com';
238+
$this->user->password_hash = $hash;
239+
240+
$this->user->saveEmailIdentity();
241+
242+
$identity = $this->user->getEmailIdentity();
243+
$this->assertSame('foo@example.com', $identity->secret);
244+
}
219245
}

0 commit comments

Comments
 (0)