Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 13 additions & 15 deletions src/Authenticator/CookieAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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)) {
Expand Down
37 changes: 19 additions & 18 deletions src/Authenticator/FormAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down
25 changes: 11 additions & 14 deletions src/Authenticator/HttpBasicAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -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,
]);
Expand Down
29 changes: 20 additions & 9 deletions src/Authenticator/JwtAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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.
*
Expand Down Expand Up @@ -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);
Expand Down
21 changes: 11 additions & 10 deletions src/Authenticator/TokenAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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);
Expand Down
58 changes: 58 additions & 0 deletions tests/TestCase/AuthenticationServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1373,4 +1373,62 @@ 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());
}
}
21 changes: 16 additions & 5 deletions tests/TestCase/Authenticator/FormAuthenticatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
}