Bug: Assigning null to ?array $identities property without proper handling

[WCrash7]

PHP Version

8.2.7

CodeIgniter4 Version

4.6.3

Shield Version

1.2.0

Which operating systems have you tested for this bug?

Windows

Which server did you use?

cli-server (PHP built-in webserver)

Database

mysqlite(memory unittest)

Did you customize Shield?

yes:

class User extends \CodeIgniter\Shield\Entities\User
.
.
.
public function saveUsernameIdentity(): bool
	{
		if (empty($this->username) && empty($this->password) && empty($this->password_hash)) {
			return true;
		}

		/** @var UserIdentityModel $identityModel */
		$identityModel = model(UserIdentityModel::class);

		$identity = $this->getIdentity('username_password');
		if ($identity === null && !empty($this->username)) {
			$this->identities = null;

			$this->createUsernameIdentity( [
				'username' => $this->username,
				'password' => '',
			]);


			$identity = $this->getUsernameIdentity();
		}

		if ($identity !== null) {
			if (!empty($this->username)) {
				$identity->secret = $this->username;
			}

			if (!empty($this->password)) {
				$identity->secret2 = service('passwords')->hash($this->password);
			}

			if (!empty($this->password_hash) && empty($this->password)) {
				$identity->secret2 = $this->password_hash;
			}

			try {
				$identityModel->save($identity);
			} catch (\CodeIgniter\Database\Exceptions\DataException $e) {
				$messages = [
					lang('Database.emptyDataset', ['insert']),
					lang('Database.emptyDataset', ['update']),
				];
				if (in_array($e->getMessage(), $messages, true)) {
					return true;
				}

				throw $e;
			}
		}

		return true;
	}

What happened?

the $identities property is correctly declared as nullable array:

shield/src/Entities/User.php

Line 50 in d07c0f9

private ?array $identities = null;

However, in several places, we directly assign null like these:

shield/src/Entities/User.php

Line 130 in d07c0f9

public function setIdentities(array $identities): void

shield/src/Entities/User.php

Line 149 in d07c0f9

$this->identities = null;

TypeError: CodeIgniter\Shield\Entities\User::setIdentities(): Argument 1 ($identities) must be of type array, null given,

Steps to Reproduce

try to reload all identities

Expected Output

reload identities without error

Anything else?

No response

[datamweb]

I’m not able to understand the issue you’re describing. While your point about method setIdentities might be valid in theory, we actually don’t use that method anywhere in our codebase.

You should try to provide at least the minimal required code along with clear steps to reproduce the issue.

As for loading, I have no problem doing it in the following way.

try to reload all identities

dd( auth()->user()->getIdentities());

[WCrash7]

in my code:
. . . $identity = $this->getIdentity('username_password'); if ($identity === null && !empty($this->username)) { $this->identities = null; . . .
This line is used to set identities to null in order to force reloading of identities.
$this->identities = null;
The statement calls this function:

shield/src/Entities/User.php

Line 130 in d07c0f9

public function setIdentities(array $identities): void

We set identities to null because the getIdentities() method:

shield/src/Entities/User.php

Line 111 in d07c0f9

public function getIdentities(string $type = 'all'): array

calls $this->populateIdentities()

shield/src/Entities/User.php

Line 113 in d07c0f9

$this->populateIdentities();

For reloading identities after changes, $this->identities must be null:

https://github.com/codeigniter4/shield/blob/d07c0f9442dd0712b3fa39bfba677838ff1b8e26/src/Entities/User.php#L93C2-L101C6

This behavior is necessary when you update a user's identities (e.g. username, email, or password) and need the updated identities to be reloaded from the database on the next access.

[datamweb]

So, based on this, you are suggesting that the method signature should be changed to:

public function setIdentities(?array $identities): void

Is that correct?

[WCrash7]

Yes