Fix loadIdentifier called after loadAuthenticator losing resolver config

dereuromark · claude · dereuromark · commit acfdcf80a1ea · 2025-11-25T12:35:16.000+01:00
When loadAuthenticator() was called before loadIdentifier(), the authenticator would receive an empty IdentifierCollection and create its own default Password identifier immediately in the constructor. Later calls to loadIdentifier() would add to the service's identifier collection, but the authenticator already had its own separate collection with the default identifier. This fix changes the default identifier loading from eager (in constructor) to lazy (in authenticate()). This ensures that if loadIdentifier() is called after loadAuthenticator(), the identifier will be loaded into the shared collection before the authenticator tries to use it. Fixes #754 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
diff --git a/src/Authenticator/CookieAuthenticator.php b/src/Authenticator/CookieAuthenticator.php
@@ -58,26 +58,23 @@ class CookieAuthenticator extends AbstractAuthenticator implements PersistenceIn
     ];
 
     /**
-     * Constructor
+     * Gets the identifier, loading a default Password identifier if none configured.
      *
-     * @param \Authentication\Identifier\IdentifierInterface $identifier Identifier or identifiers collection.
-     * @param array<string, mixed> $config Configuration settings.
+     * This is done lazily to allow loadIdentifier() to be called after loadAuthenticator().
+     *
+     * @return \Authentication\Identifier\IdentifierInterface
      */
-    public function __construct(IdentifierInterface $identifier, array $config = [])
+    public function getIdentifier(): IdentifierInterface
     {
-        // If no identifier is configured, set up a default Password identifier
-        if ($identifier instanceof IdentifierCollection && $identifier->isEmpty()) {
-            // Pass the authenticator's fields configuration to the identifier
+        if ($this->_identifier instanceof IdentifierCollection && $this->_identifier->isEmpty()) {
             $identifierConfig = [];
-            if (isset($config['fields'])) {
-                $identifierConfig['fields'] = $config['fields'];
+            if ($this->getConfig('fields')) {
+                $identifierConfig['fields'] = $this->getConfig('fields');
             }
-            $identifier = new IdentifierCollection([
-                'Authentication.Password' => $identifierConfig,
-            ]);
+            $this->_identifier->load('Authentication.Password', $identifierConfig);
         }
 
-        parent::__construct($identifier, $config);
+        return $this->_identifier;
     }
 
     /**
@@ -107,10 +104,11 @@ public function authenticate(ServerRequestInterface $request): ResultInterface
 
         [$username, $tokenHash] = $token;
 
-        $identity = $this->_identifier->identify(compact('username'));
+        $identifier = $this->getIdentifier();
+        $identity = $identifier->identify(compact('username'));
 
         if (!$identity) {
-            return new Result(null, Result::FAILURE_IDENTITY_NOT_FOUND, $this->_identifier->getErrors());
+            return new Result(null, Result::FAILURE_IDENTITY_NOT_FOUND, $identifier->getErrors());
         }
 
         if (!$this->_checkToken($identity, $tokenHash)) {
diff --git a/src/Authenticator/FormAuthenticator.php b/src/Authenticator/FormAuthenticator.php
@@ -50,26 +50,23 @@ class FormAuthenticator extends AbstractAuthenticator
     ];
 
     /**
-     * Constructor
+     * Gets the identifier, loading a default Password identifier if none configured.
      *
-     * @param \Authentication\Identifier\IdentifierInterface $identifier Identifier or identifiers collection.
-     * @param array<string, mixed> $config Configuration settings.
+     * This is done lazily to allow loadIdentifier() to be called after loadAuthenticator().
+     *
+     * @return \Authentication\Identifier\IdentifierInterface
      */
-    public function __construct(IdentifierInterface $identifier, array $config = [])
+    public function getIdentifier(): IdentifierInterface
     {
-        // If no identifier is configured, set up a default Password identifier
-        if ($identifier instanceof IdentifierCollection && $identifier->isEmpty()) {
-            // Pass the authenticator's fields configuration to the identifier
+        if ($this->_identifier instanceof IdentifierCollection && $this->_identifier->isEmpty()) {
             $identifierConfig = [];
-            if (isset($config['fields'])) {
-                $identifierConfig['fields'] = $config['fields'];
+            if ($this->getConfig('fields')) {
+                $identifierConfig['fields'] = $this->getConfig('fields');
             }
-            $identifier = new IdentifierCollection([
-                'Authentication.Password' => $identifierConfig,
-            ]);
+            $this->_identifier->load('Authentication.Password', $identifierConfig);
         }
 
-        parent::__construct($identifier, $config);
+        return $this->_identifier;
     }
 
     /**
@@ -141,9 +138,12 @@ protected function _buildLoginUrlErrorResult(ServerRequestInterface $request): R
     }
 
     /**
-     * Authenticates the identity contained in a request. Will use the `config.userModel`, and `config.fields`
-     * to find POST data that is used to find a matching record in the `config.userModel`. Will return false if
-     * there is no post data, either username or password is missing, or if the scope conditions have not been met.
+     * Authenticates the identity contained in a request.
+     *
+     * Will use the `config.userModel`, and `config.fields` to find POST data
+     * that is used to find a matching record in the `config.userModel`.
+     * Will return false if there is no post data, either username or password is missing,
+     * or if the scope conditions have not been met.
      *
      * @param \Psr\Http\Message\ServerRequestInterface $request The request that contains login information.
      * @return \Authentication\Authenticator\ResultInterface
@@ -161,10 +161,11 @@ public function authenticate(ServerRequestInterface $request): ResultInterface
             ]);
         }
 
-        $user = $this->_identifier->identify($data);
+        $identifier = $this->getIdentifier();
+        $user = $identifier->identify($data);
 
         if (!$user) {
-            return new Result(null, Result::FAILURE_IDENTITY_NOT_FOUND, $this->_identifier->getErrors());
+            return new Result(null, Result::FAILURE_IDENTITY_NOT_FOUND, $identifier->getErrors());
         }
 
         return new Result($user, Result::SUCCESS);
diff --git a/src/Authenticator/HttpBasicAuthenticator.php b/src/Authenticator/HttpBasicAuthenticator.php
@@ -44,26 +44,23 @@ class HttpBasicAuthenticator extends AbstractAuthenticator implements StatelessI
     ];
 
     /**
-     * Constructor
+     * Gets the identifier, loading a default Password identifier if none configured.
      *
-     * @param \Authentication\Identifier\IdentifierInterface $identifier Identifier or identifiers collection.
-     * @param array<string, mixed> $config Configuration settings.
+     * This is done lazily to allow loadIdentifier() to be called after loadAuthenticator().
+     *
+     * @return \Authentication\Identifier\IdentifierInterface
      */
-    public function __construct(IdentifierInterface $identifier, array $config = [])
+    public function getIdentifier(): IdentifierInterface
     {
-        // If no identifier is configured, set up a default Password identifier
-        if ($identifier instanceof IdentifierCollection && $identifier->isEmpty()) {
-            // Pass the authenticator's fields configuration to the identifier
+        if ($this->_identifier instanceof IdentifierCollection && $this->_identifier->isEmpty()) {
             $identifierConfig = [];
-            if (isset($config['fields'])) {
-                $identifierConfig['fields'] = $config['fields'];
+            if ($this->getConfig('fields')) {
+                $identifierConfig['fields'] = $this->getConfig('fields');
             }
-            $identifier = new IdentifierCollection([
-                'Authentication.Password' => $identifierConfig,
-            ]);
+            $this->_identifier->load('Authentication.Password', $identifierConfig);
         }
 
-        parent::__construct($identifier, $config);
+        return $this->_identifier;
     }
 
     /**
@@ -83,7 +80,7 @@ public function authenticate(ServerRequestInterface $request): ResultInterface
             return new Result(null, Result::FAILURE_CREDENTIALS_MISSING);
         }
 
-        $user = $this->_identifier->identify([
+        $user = $this->getIdentifier()->identify([
             AbstractIdentifier::CREDENTIAL_USERNAME => $username,
             AbstractIdentifier::CREDENTIAL_PASSWORD => $password,
         ]);
diff --git a/src/Authenticator/JwtAuthenticator.php b/src/Authenticator/JwtAuthenticator.php
@@ -57,13 +57,7 @@ class JwtAuthenticator extends TokenAuthenticator
      */
     public function __construct(IdentifierInterface $identifier, array $config = [])
     {
-        // Override parent's default - JWT should use JwtSubject identifier
-        if ($identifier instanceof IdentifierCollection && $identifier->isEmpty()) {
-            $identifier = new IdentifierCollection(['Authentication.JwtSubject']);
-        }
-
-        // Call TokenAuthenticator's constructor but skip its default
-        AbstractAuthenticator::__construct($identifier, $config);
+        parent::__construct($identifier, $config);
 
         if (empty($this->_config['secretKey'])) {
             if (!class_exists(Security::class)) {
@@ -73,6 +67,22 @@ public function __construct(IdentifierInterface $identifier, array $config = [])
         }
     }
 
+    /**
+     * Gets the identifier, loading a default JwtSubject identifier if none configured.
+     *
+     * This is done lazily to allow loadIdentifier() to be called after loadAuthenticator().
+     *
+     * @return \Authentication\Identifier\IdentifierInterface
+     */
+    public function getIdentifier(): IdentifierInterface
+    {
+        if ($this->_identifier instanceof IdentifierCollection && $this->_identifier->isEmpty()) {
+            $this->_identifier->load('Authentication.JwtSubject');
+        }
+
+        return $this->_identifier;
+    }
+
     /**
      * Authenticates the identity based on a JWT token contained in a request.
      *
@@ -113,12 +123,13 @@ public function authenticate(ServerRequestInterface $request): ResultInterface
             return new Result($user, Result::SUCCESS);
         }
 
-        $user = $this->_identifier->identify([
+        $identifier = $this->getIdentifier();
+        $user = $identifier->identify([
             $subjectKey => $result[$subjectKey],
         ]);
 
         if (!$user) {
-            return new Result(null, Result::FAILURE_IDENTITY_NOT_FOUND, $this->_identifier->getErrors());
+            return new Result(null, Result::FAILURE_IDENTITY_NOT_FOUND, $identifier->getErrors());
         }
 
         return new Result($user, Result::SUCCESS);
diff --git a/src/Authenticator/TokenAuthenticator.php b/src/Authenticator/TokenAuthenticator.php
@@ -38,19 +38,19 @@ class TokenAuthenticator extends AbstractAuthenticator implements StatelessInter
     ];
 
     /**
-     * Constructor
+     * Gets the identifier, loading a default Token identifier if none configured.
      *
-     * @param \Authentication\Identifier\IdentifierInterface $identifier Identifier or identifiers collection.
-     * @param array<string, mixed> $config Configuration settings.
+     * This is done lazily to allow loadIdentifier() to be called after loadAuthenticator().
+     *
+     * @return \Authentication\Identifier\IdentifierInterface
      */
-    public function __construct(IdentifierInterface $identifier, array $config = [])
+    public function getIdentifier(): IdentifierInterface
     {
-        // If no identifier is configured, set up a default Token identifier
-        if ($identifier instanceof IdentifierCollection && $identifier->isEmpty()) {
-            $identifier = new IdentifierCollection(['Authentication.Token']);
+        if ($this->_identifier instanceof IdentifierCollection && $this->_identifier->isEmpty()) {
+            $this->_identifier->load('Authentication.Token');
         }
 
-        parent::__construct($identifier, $config);
+        return $this->_identifier;
     }
 
     /**
@@ -142,12 +142,13 @@ public function authenticate(ServerRequestInterface $request): ResultInterface
             return new Result(null, Result::FAILURE_CREDENTIALS_MISSING);
         }
 
-        $user = $this->_identifier->identify([
+        $identifier = $this->getIdentifier();
+        $user = $identifier->identify([
             TokenIdentifier::CREDENTIAL_TOKEN => $token,
         ]);
 
         if (!$user) {
-            return new Result(null, Result::FAILURE_IDENTITY_NOT_FOUND, $this->_identifier->getErrors());
+            return new Result(null, Result::FAILURE_IDENTITY_NOT_FOUND, $identifier->getErrors());
         }
 
         return new Result($user, Result::SUCCESS);
diff --git a/tests/TestCase/AuthenticationServiceTest.php b/tests/TestCase/AuthenticationServiceTest.php
@@ -1373,4 +1373,63 @@ public function testFormAuthenticatorDefaultIdentifier()
         $authenticator = $service->getAuthenticationProvider();
         $this->assertInstanceOf(FormAuthenticator::class, $authenticator);
     }
+
+    /**
+     * Test that loadIdentifier called after loadAuthenticator still works.
+     *
+     * This is a regression test for https://github.com/cakephp/authentication/issues/754
+     * When loadAuthenticator was called before loadIdentifier, the authenticator would
+     * create its own default identifier collection and ignore the later loadIdentifier call.
+     *
+     * @deprecated Note that this test will be removed in 4.x as this is only to keep BC in 3.x.
+     *
+     * @return void
+     */
+    public function testLoadIdentifierAfterLoadAuthenticator()
+    {
+        $this->skipIf(
+            version_compare(Version::id(), '11.0', '<'),
+            'For some reason PHPUnit doesn\'t pick up the deprecation on v10',
+        );
+
+        $request = ServerRequestFactory::fromGlobals(
+            ['REQUEST_URI' => '/testpath'],
+            [],
+            ['username' => 'mariano', 'password' => 'password'],
+        );
+
+        $service = new AuthenticationService();
+
+        // Load authenticator FIRST
+        $service->loadAuthenticator('Authentication.Form');
+
+        // Then load identifier with custom resolver config
+        $this->deprecated(function () use ($service) {
+            $service->loadIdentifier('Authentication.Password', [
+                'resolver' => [
+                    'className' => 'Authentication.Orm',
+                    'userModel' => 'AuthUsers',
+                    'finder' => 'auth',
+                ],
+            ]);
+        });
+
+        // The authenticator should use the identifier we loaded, not its default
+        $formAuth = $service->authenticators()->get('Form');
+        $identifierCollection = $formAuth->getIdentifier();
+        $this->assertInstanceOf(IdentifierCollection::class, $identifierCollection);
+
+        $passwordIdentifier = $identifierCollection->get('Password');
+        $this->assertInstanceOf(PasswordIdentifier::class, $passwordIdentifier);
+
+        // Verify the resolver config was applied
+        $resolverConfig = $passwordIdentifier->getConfig('resolver');
+        $this->assertIsArray($resolverConfig);
+        $this->assertSame('AuthUsers', $resolverConfig['userModel']);
+        $this->assertSame('auth', $resolverConfig['finder']);
+
+        // Verify authentication actually works with the custom config
+        $result = $service->authenticate($request);
+        $this->assertTrue($result->isValid());
+    }
 }
diff --git a/tests/TestCase/Authenticator/FormAuthenticatorTest.php b/tests/TestCase/Authenticator/FormAuthenticatorTest.php
@@ -641,25 +641,36 @@ public function testDefaultIdentifierInheritsFieldsConfig()
         $identifiers = new IdentifierCollection();
 
         // Configure authenticator with custom fields mapping
+        // Also set a loginUrl that won't match, so authenticate() returns early
+        // without actually trying to identify (which would require database access)
         $config = [
             'fields' => [
                 'username' => 'user_name',
                 'password' => 'pass_word',
             ],
+            'loginUrl' => '/login',
         ];
 
         // FormAuthenticator should create default identifier with inherited fields
+        // The default identifier is loaded lazily when authenticate() is called
         $form = new FormAuthenticator($identifiers, $config);
 
+        // Trigger the lazy loading by calling authenticate on a non-matching URL
+        $request = ServerRequestFactory::fromGlobals(
+            ['REQUEST_URI' => '/testpath'],
+            [],
+            ['user_name' => 'mariano', 'pass_word' => 'password'],
+        );
+        $form->authenticate($request);
+
         // Verify the identifier was created with the correct configuration
         $identifier = $form->getIdentifier();
         $this->assertInstanceOf(IdentifierCollection::class, $identifier);
         $this->assertFalse($identifier->isEmpty());
 
-        // Verify the fields are properly configured
-        // We can't directly access the internal configuration, but we can verify
-        // the FormAuthenticator has the expected configuration
-        $this->assertEquals('user_name', $form->getConfig('fields.username'));
-        $this->assertEquals('pass_word', $form->getConfig('fields.password'));
+        // Verify the fields are properly configured on the identifier
+        $passwordIdentifier = $identifier->get('Password');
+        $this->assertEquals('user_name', $passwordIdentifier->getConfig('fields.username'));
+        $this->assertEquals('pass_word', $passwordIdentifier->getConfig('fields.password'));
     }
 }
