Skip to content

Commit 9a12edc

Browse files
authored
Merge pull request #763 from cakephp/fix/deprecate-session-identify
Deprecate SessionAuthenticator identify option
2 parents d5ac2be + 373bd7a commit 9a12edc

File tree

5 files changed

+73
-48
lines changed

5 files changed

+73
-48
lines changed

docs/en/authenticators.rst

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,10 @@ Configuration options:
1818

1919
- **sessionKey**: The session key for the user data, default is
2020
``Auth``
21-
- **identify**: Set this key with a value of bool ``true`` to enable checking
22-
the session credentials against the identifiers. When ``true``, the configured
23-
:doc:`/identifiers` are used to identify the user using data
24-
stored in the session on each request. Default value is ``false``.
21+
- **identify**: Deprecated in 3.4.0. Use ``PrimaryKeySessionAuthenticator``
22+
instead if you need to fetch fresh user data from the database on each request.
2523
- **fields**: Allows you to map the ``username`` field to the unique
26-
identifier in your user storage. Defaults to ``username``. This option is
27-
used when the ``identify`` option is set to true.
24+
identifier in your user storage. Defaults to ``username``.
2825

2926
PrimaryKeySession
3027
=================

docs/en/index.rst

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ imports::
3232
use Authentication\AuthenticationServiceInterface;
3333
use Authentication\AuthenticationServiceProviderInterface;
3434
use Authentication\Identifier\AbstractIdentifier;
35-
use Authentication\Identifier\IdentifierInterface;
3635
use Authentication\Middleware\AuthenticationMiddleware;
3736
use Cake\Http\MiddlewareQueue;
3837
use Cake\Routing\Router;
@@ -94,23 +93,20 @@ define the ``AuthenticationService`` it wants to use. Add the following method t
9493
'queryParam' => 'redirect',
9594
]);
9695

97-
// Define identifiers
9896
$fields = [
9997
AbstractIdentifier::CREDENTIAL_USERNAME => 'email',
100-
AbstractIdentifier::CREDENTIAL_PASSWORD => 'password'
101-
];
102-
$passwordIdentifier = [
103-
'Authentication.Password' => [
104-
'fields' => $fields,
105-
],
98+
AbstractIdentifier::CREDENTIAL_PASSWORD => 'password',
10699
];
107100

108101
// Load the authenticators. Session should be first.
109-
$service->loadAuthenticator('Authentication.Session', [
110-
'identifier' => $passwordIdentifier,
111-
]);
102+
// Session just uses session data directly as identity, no identifier needed.
103+
$service->loadAuthenticator('Authentication.Session');
112104
$service->loadAuthenticator('Authentication.Form', [
113-
'identifier' => $passwordIdentifier,
105+
'identifier' => [
106+
'Authentication.Password' => [
107+
'fields' => $fields,
108+
],
109+
],
114110
'fields' => $fields,
115111
'loginUrl' => Router::url([
116112
'prefix' => false,
@@ -126,9 +122,9 @@ define the ``AuthenticationService`` it wants to use. Add the following method t
126122
First, we configure what to do with users when they are not authenticated.
127123
Next, we attach the ``Session`` and ``Form`` :doc:`/authenticators` which define the
128124
mechanisms that our application will use to authenticate users. ``Session`` enables us to identify
129-
users based on data in the session while ``Form`` enables us
130-
to handle a login form at the ``loginUrl``. Finally we attach an :doc:`identifier
131-
</identifiers>` to convert the credentials users will give us into an
125+
users based on data in the session - it uses the session data directly as identity without any
126+
database lookup. ``Form`` enables us to handle a login form at the ``loginUrl`` and uses an
127+
:doc:`identifier </identifiers>` to convert the credentials users will give us into an
132128
:doc:`identity </identity-object>` which represents our logged in user.
133129

134130
If one of the configured authenticators was able to validate the credentials,

src/Authenticator/SessionAuthenticator.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Cake\Http\Exception\UnauthorizedException;
2222
use Psr\Http\Message\ResponseInterface;
2323
use Psr\Http\Message\ServerRequestInterface;
24+
use function Cake\Core\deprecationWarning;
2425

2526
/**
2627
* Session Authenticator
@@ -31,9 +32,9 @@ class SessionAuthenticator extends AbstractAuthenticator implements PersistenceI
3132
* Default config for this object.
3233
* - `fields` The fields to use to verify a user by.
3334
* - `sessionKey` Session key.
34-
* - `identify` Whether to identify user data stored in a session. This is
35-
* useful if you want to remotely end sessions that have a different password stored,
36-
* or if your identification logic needs additional conditions before a user can login.
35+
* - `identify` Whether to identify user data stored in a session.
36+
* Deprecated: Use `PrimaryKeySessionAuthenticator` instead if you
37+
* need to fetch fresh user data from the database on each request.
3738
*
3839
* @var array
3940
*/
@@ -65,6 +66,11 @@ public function authenticate(ServerRequestInterface $request): ResultInterface
6566
}
6667

6768
if ($this->getConfig('identify') === true) {
69+
deprecationWarning(
70+
'3.4.0',
71+
'The `identify` option is deprecated. ' .
72+
'Use `PrimaryKeySessionAuthenticator` instead to fetch fresh user data on each request.',
73+
);
6874
$credentials = [];
6975
foreach ($this->getConfig('fields') as $key => $field) {
7076
$credentials[$key] = $user[$field];

tests/TestCase/AuthenticationServiceTest.php

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,15 @@ public function testAuthenticateWithChallengeDisabled()
172172
* Integration test for session auth + identify always getting a fresh user record.
173173
*
174174
* @return void
175+
* @deprecated The `identify` option is deprecated.
175176
*/
176177
public function testAuthenticationWithSessionIdentify()
177178
{
179+
$this->skipIf(
180+
version_compare(Version::id(), '11.0', '<'),
181+
'For some reason PHPUnit doesn\'t pick up the deprecation on v10',
182+
);
183+
178184
$users = $this->fetchTable('Users');
179185
$user = $users->get(1);
180186

@@ -197,25 +203,28 @@ public function testAuthenticationWithSessionIdentify()
197203
],
198204
]);
199205
};
200-
$service = $factory();
201-
$result = $service->authenticate($request);
202-
$this->assertTrue($result->isValid());
203-
204-
$dateValue = new DateTime('2022-01-01 10:11:12');
205-
$identity = $result->getData();
206-
$this->assertEquals($identity->username, $user->username);
207-
$this->assertNotEquals($identity->created, $dateValue);
208-
209-
// Update the user so that we can ensure session is reading from the db.
210-
$user->created = $dateValue;
211-
$users->saveOrFail($user);
212206

213-
$service = $factory();
214-
$result = $service->authenticate($request);
215-
$this->assertTrue($result->isValid());
216-
$identity = $result->getData();
217-
$this->assertEquals($identity->username, $user->username);
218-
$this->assertEquals($identity->created, $dateValue);
207+
$this->deprecated(function () use ($factory, $request, $users, $user) {
208+
$service = $factory();
209+
$result = $service->authenticate($request);
210+
$this->assertTrue($result->isValid());
211+
212+
$dateValue = new DateTime('2022-01-01 10:11:12');
213+
$identity = $result->getData();
214+
$this->assertEquals($identity->username, $user->username);
215+
$this->assertNotEquals($identity->created, $dateValue);
216+
217+
// Update the user so that we can ensure session is reading from the db.
218+
$user->created = $dateValue;
219+
$users->saveOrFail($user);
220+
221+
$service = $factory();
222+
$result = $service->authenticate($request);
223+
$this->assertTrue($result->isValid());
224+
$identity = $result->getData();
225+
$this->assertEquals($identity->username, $user->username);
226+
$this->assertEquals($identity->created, $dateValue);
227+
});
219228
}
220229

221230
/**

tests/TestCase/Authenticator/SessionAuthenticatorTest.php

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use Cake\Http\ServerRequestFactory;
2828
use Cake\Http\Session;
2929
use Cake\ORM\TableRegistry;
30+
use PHPUnit\Runner\Version;
3031
use Psr\Http\Message\RequestInterface;
3132
use Psr\Http\Message\ResponseInterface;
3233

@@ -203,9 +204,15 @@ public function testAuthenticateFailure()
203204
* Test successful session data verification by database lookup
204205
*
205206
* @return void
207+
* @deprecated The `identify` option is deprecated.
206208
*/
207209
public function testVerifyByDatabaseSuccess()
208210
{
211+
$this->skipIf(
212+
version_compare(Version::id(), '11.0', '<'),
213+
'For some reason PHPUnit doesn\'t pick up the deprecation on v10',
214+
);
215+
209216
$request = ServerRequestFactory::fromGlobals(['REQUEST_URI' => '/']);
210217

211218
$this->sessionMock->expects($this->once())
@@ -221,19 +228,27 @@ public function testVerifyByDatabaseSuccess()
221228
$authenticator = new SessionAuthenticator($this->identifiers, [
222229
'identify' => true,
223230
]);
224-
$result = $authenticator->authenticate($request);
231+
$this->deprecated(function () use ($authenticator, $request) {
232+
$result = $authenticator->authenticate($request);
225233

226-
$this->assertInstanceOf(Result::class, $result);
227-
$this->assertSame(Result::SUCCESS, $result->getStatus());
234+
$this->assertInstanceOf(Result::class, $result);
235+
$this->assertSame(Result::SUCCESS, $result->getStatus());
236+
});
228237
}
229238

230239
/**
231240
* Test session data verification by database lookup failure
232241
*
233242
* @return void
243+
* @deprecated The `identify` option is deprecated.
234244
*/
235245
public function testVerifyByDatabaseFailure()
236246
{
247+
$this->skipIf(
248+
version_compare(Version::id(), '11.0', '<'),
249+
'For some reason PHPUnit doesn\'t pick up the deprecation on v10',
250+
);
251+
237252
$request = ServerRequestFactory::fromGlobals(['REQUEST_URI' => '/']);
238253

239254
$this->sessionMock->expects($this->once())
@@ -249,10 +264,12 @@ public function testVerifyByDatabaseFailure()
249264
$authenticator = new SessionAuthenticator($this->identifiers, [
250265
'identify' => true,
251266
]);
252-
$result = $authenticator->authenticate($request);
267+
$this->deprecated(function () use ($authenticator, $request) {
268+
$result = $authenticator->authenticate($request);
253269

254-
$this->assertInstanceOf(Result::class, $result);
255-
$this->assertSame(Result::FAILURE_CREDENTIALS_INVALID, $result->getStatus());
270+
$this->assertInstanceOf(Result::class, $result);
271+
$this->assertSame(Result::FAILURE_CREDENTIALS_INVALID, $result->getStatus());
272+
});
256273
}
257274

258275
/**

0 commit comments

Comments
 (0)