Skip to content

Commit 4884c2b

Browse files
markstorymarkstory
authored
Merge pull request #286 from cakephp/read-session
Fix Security component & session authenticator
2 parents 746a234 + 4eaad2e commit 4884c2b

File tree

4 files changed

+127
-85
lines changed

4 files changed

+127
-85
lines changed

‎src/AuthenticationService.php‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,6 @@ public function authenticate(ServerRequestInterface $request, ResponseInterface
177177
foreach ($this->authenticators() as $authenticator) {
178178
$result = $authenticator->authenticate($request, $response);
179179
if ($result->isValid()) {
180-
if (!($authenticator instanceof StatelessInterface)) {
181-
$requestResponse = $this->persistIdentity($request, $response, $result->getData());
182-
$request = $requestResponse['request'];
183-
$response = $requestResponse['response'];
184-
}
185-
186180
$this->_successfulAuthenticator = $authenticator;
187181
$this->_result = $result;
188182

‎src/Middleware/AuthenticationMiddleware.php‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Authentication\AuthenticationService;
1818
use Authentication\AuthenticationServiceInterface;
1919
use Authentication\AuthenticationServiceProviderInterface;
20+
use Authentication\Authenticator\StatelessInterface;
2021
use Authentication\Authenticator\UnauthenticatedException;
2122
use Authentication\Authenticator\UnauthorizedException;
2223
use Cake\Core\InstanceConfigTrait;
@@ -119,7 +120,19 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res
119120
$response = $result['response'];
120121

121122
try {
122-
return $next($request, $response);
123+
$response = $next($request, $response);
124+
125+
$authenticator = $service->getAuthenticationProvider();
126+
if ($authenticator !== null && !($authenticator instanceof StatelessInterface)) {
127+
$requestResponse = $service->persistIdentity(
128+
$request,
129+
$response,
130+
$result['result']->getData()
131+
);
132+
$response = $requestResponse['response'];
133+
}
134+
135+
return $response;
123136
} catch (UnauthenticatedException $e) {
124137
$url = $service->getUnauthenticatedRedirectUrl($request);
125138
if ($url) {

‎tests/TestCase/AuthenticationServiceTest.php‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ public function testAuthenticate()
6666
],
6767
'authenticators' => [
6868
'Authentication.Form',
69-
'Authentication.Session',
7069
]
7170
]);
7271

@@ -79,9 +78,9 @@ public function testAuthenticate()
7978
$result = $service->getAuthenticationProvider();
8079
$this->assertInstanceOf(FormAuthenticator::class, $result);
8180

82-
$this->assertEquals(
83-
'mariano',
84-
$request->getAttribute('session')->read('Auth.username')
81+
$this->assertNull(
82+
$request->getAttribute('session')->read('Auth'),
83+
'Session is populated after authenticate by middleware'
8584
);
8685
}
8786

‎tests/TestCase/Middleware/AuthenticationMiddlewareTest.php‎

Lines changed: 110 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,19 @@ public function testApplicationAuthentication()
6666
);
6767
$response = new Response();
6868
$next = function ($request, $response) {
69-
return $request;
70-
};
71-
72-
$middleware = new AuthenticationMiddleware($this->application);
69+
/* @var $service AuthenticationService */
70+
$service = $request->getAttribute('authentication');
71+
$this->assertInstanceOf(AuthenticationService::class, $service);
7372

74-
$request = $middleware($request, $response, $next);
73+
$this->assertTrue($service->identifiers()->has('Password'));
74+
$this->assertTrue($service->authenticators()->has('Form'));
75+
$this->assertEquals('identity', $service->getConfig("identityAttribute"));
7576

76-
/* @var $service AuthenticationService */
77-
$service = $request->getAttribute('authentication');
78-
$this->assertInstanceOf(AuthenticationService::class, $service);
77+
return $response;
78+
};
7979

80-
$this->assertTrue($service->identifiers()->has('Password'));
81-
$this->assertTrue($service->authenticators()->has('Form'));
82-
$this->assertEquals('identity', $service->getConfig("identityAttribute"));
80+
$middleware = new AuthenticationMiddleware($this->application);
81+
$middleware($request, $response, $next);
8382
}
8483

8584
public function testProviderAuthentication()
@@ -91,7 +90,16 @@ public function testProviderAuthentication()
9190
);
9291
$response = new Response();
9392
$next = function ($request, $response) {
94-
return $request;
93+
/* @var $service AuthenticationService */
94+
$service = $request->getAttribute('authentication');
95+
$this->assertInstanceOf(AuthenticationService::class, $service);
96+
$this->assertSame($this->service, $service);
97+
98+
$this->assertTrue($service->identifiers()->has('Password'));
99+
$this->assertTrue($service->authenticators()->has('Form'));
100+
$this->assertSame('identity', $service->getConfig("identityAttribute"));
101+
102+
return $response;
95103
};
96104

97105
$provider = $this->createMock(AuthenticationServiceProviderInterface::class);
@@ -100,16 +108,7 @@ public function testProviderAuthentication()
100108
->willReturn($this->service);
101109

102110
$middleware = new AuthenticationMiddleware($provider);
103-
$request = $middleware($request, $response, $next);
104-
105-
/* @var $service AuthenticationService */
106-
$service = $request->getAttribute('authentication');
107-
$this->assertInstanceOf(AuthenticationService::class, $service);
108-
$this->assertSame($this->service, $service);
109-
110-
$this->assertTrue($service->identifiers()->has('Password'));
111-
$this->assertTrue($service->authenticators()->has('Form'));
112-
$this->assertSame('identity', $service->getConfig("identityAttribute"));
111+
$middleware($request, $response, $next);
113112
}
114113

115114
public function testProviderInvalidService()
@@ -120,10 +119,6 @@ public function testProviderInvalidService()
120119
['username' => 'mariano', 'password' => 'password']
121120
);
122121
$response = new Response();
123-
$next = function ($request, $response) {
124-
return $request;
125-
};
126-
127122
$app = $this->createMock(BaseApplication::class);
128123
$provider = $this->createMock(AuthenticationServiceProviderInterface::class);
129124
$provider
@@ -133,6 +128,9 @@ public function testProviderInvalidService()
133128
$this->expectException(RuntimeException::class);
134129
$this->expectExceptionMessage('Service provided by a subject must be an instance of `Authentication\AuthenticationServiceInterface`, `Mock_BaseApplication_');
135130

131+
$next = function ($request, $response) {
132+
return $response;
133+
};
136134
$middleware = new AuthenticationMiddleware($provider);
137135
$middleware($request, $response, $next);
138136
}
@@ -207,17 +205,18 @@ public function testSuccessfulAuthentication()
207205
$response = new Response();
208206

209207
$next = function ($request, $response) {
210-
return $request;
208+
$identity = $request->getAttribute('identity');
209+
$service = $request->getAttribute('authentication');
210+
211+
$this->assertInstanceOf(IdentityInterface::class, $identity);
212+
$this->assertInstanceOf(AuthenticationService::class, $service);
213+
$this->assertTrue($service->getResult()->isValid());
214+
215+
return $response;
211216
};
212217

213218
$middleware = new AuthenticationMiddleware($this->service);
214-
$request = $middleware($request, $response, $next);
215-
$identity = $request->getAttribute('identity');
216-
$service = $request->getAttribute('authentication');
217-
218-
$this->assertInstanceOf(IdentityInterface::class, $identity);
219-
$this->assertInstanceOf(AuthenticationService::class, $service);
220-
$this->assertTrue($service->getResult()->isValid());
219+
$middleware($request, $response, $next);
221220
}
222221

223222
/**
@@ -233,24 +232,23 @@ public function testApplicationAuthenticationCustomIdentityAttribute()
233232
['username' => 'mariano', 'password' => 'password']
234233
);
235234
$response = new Response();
236-
$next = function ($request, $response) {
237-
return $request;
238-
};
239-
240-
// Using the middleware option requires this test to use deprecated()
241-
$middleware = new AuthenticationMiddleware($this->application, [
242-
'identityAttribute' => 'customIdentity'
243-
]);
244-
$this->deprecated(function () use ($middleware, $request, $response, $next) {
245-
$request = $middleware($request, $response, $next);
246235

236+
$next = function ($req, $resp) {
247237
/* @var $service AuthenticationService */
248-
$service = $request->getAttribute('authentication');
238+
$service = $req->getAttribute('authentication');
249239
$this->assertInstanceOf(AuthenticationService::class, $service);
250-
251240
$this->assertEquals('customIdentity', $service->getConfig("identityAttribute"));
252241
$this->assertTrue($service->identifiers()->has('Password'));
253242
$this->assertTrue($service->authenticators()->has('Form'));
243+
244+
return $resp;
245+
};
246+
$this->deprecated(function () use ($request, $response, $next) {
247+
// Using the middleware option requires this test to use deprecated()
248+
$middleware = new AuthenticationMiddleware($this->application, [
249+
'identityAttribute' => 'customIdentity'
250+
]);
251+
$middleware($request, $response, $next);
254252
});
255253
}
256254

@@ -272,16 +270,16 @@ public function testSuccessfulAuthenticationWithCustomIdentityAttribute()
272270
$middleware = new AuthenticationMiddleware($this->service);
273271

274272
$next = function ($request, $response) {
275-
return $request;
276-
};
273+
$identity = $request->getAttribute('customIdentity');
274+
$service = $request->getAttribute('authentication');
277275

278-
$request = $middleware($request, $response, $next);
279-
$identity = $request->getAttribute('customIdentity');
280-
$service = $request->getAttribute('authentication');
276+
$this->assertInstanceOf(IdentityInterface::class, $identity);
277+
$this->assertInstanceOf(AuthenticationService::class, $service);
278+
$this->assertTrue($service->getResult()->isValid());
281279

282-
$this->assertInstanceOf(IdentityInterface::class, $identity);
283-
$this->assertInstanceOf(AuthenticationService::class, $service);
284-
$this->assertTrue($service->getResult()->isValid());
280+
return $response;
281+
};
282+
$middleware($request, $response, $next);
285283
}
286284

287285
/**
@@ -301,16 +299,56 @@ public function testSuccessfulAuthenticationApplicationHook()
301299
$middleware = new AuthenticationMiddleware($this->application);
302300

303301
$next = function ($request, $response) {
304-
return $request;
302+
$identity = $request->getAttribute('identity');
303+
$service = $request->getAttribute('authentication');
304+
305+
$this->assertInstanceOf(IdentityInterface::class, $identity);
306+
$this->assertInstanceOf(AuthenticationService::class, $service);
307+
$this->assertTrue($service->getResult()->isValid());
308+
309+
return $response;
305310
};
311+
$middleware($request, $response, $next);
312+
}
306313

307-
$request = $middleware($request, $response, $next);
308-
$identity = $request->getAttribute('identity');
309-
$service = $request->getAttribute('authentication');
314+
/**
315+
* test success persist to session
316+
*
317+
* @return void
318+
*/
319+
public function testSuccessfulAuthenticationPersistIdentity()
320+
{
321+
$request = ServerRequestFactory::fromGlobals(
322+
['REQUEST_URI' => '/testpath'],
323+
[],
324+
['username' => 'mariano', 'password' => 'password']
325+
);
326+
$response = new Response();
310327

311-
$this->assertInstanceOf(IdentityInterface::class, $identity);
312-
$this->assertInstanceOf(AuthenticationService::class, $service);
313-
$this->assertTrue($service->getResult()->isValid());
328+
$this->service = new AuthenticationService([
329+
'identifiers' => [
330+
'Authentication.Password',
331+
],
332+
'authenticators' => [
333+
'Authentication.Form',
334+
'Authentication.Session',
335+
]
336+
]);
337+
$middleware = new AuthenticationMiddleware($this->service);
338+
339+
$next = function ($request, $response) {
340+
$service = $request->getAttribute('authentication');
341+
$this->assertInstanceOf(AuthenticationService::class, $service);
342+
343+
$this->assertTrue($service->getResult()->isValid());
344+
$this->assertNull($request->getAttribute('session')->read('Auth'));
345+
346+
return $response;
347+
};
348+
$middleware($request, $response, $next);
349+
350+
// After the middleware is done session should be populated
351+
$this->assertSame('mariano', $request->getAttribute('session')->read('Auth.username'));
314352
}
315353

316354
/**
@@ -710,23 +748,21 @@ public function testJwtTokenAuthorizationThroughTheMiddlewareStack()
710748
['REQUEST_URI' => '/'],
711749
['token' => $token]
712750
);
713-
714751
$response = new Response();
715752

716-
$middleware = new AuthenticationMiddleware($this->service);
717-
718-
$next = function ($request, $response) {
719-
return $request;
720-
};
753+
$next = function ($request, $response) use ($data) {
754+
$identity = $request->getAttribute('identity');
755+
$service = $request->getAttribute('authentication');
721756

722-
$request = $middleware($request, $response, $next);
723-
$identity = $request->getAttribute('identity');
724-
$service = $request->getAttribute('authentication');
757+
$this->assertInstanceOf(IdentityInterface::class, $identity);
758+
$this->assertInstanceOf(AuthenticationService::class, $service);
759+
$this->assertTrue($service->getResult()->isValid());
760+
$this->assertEquals($data, $identity->getOriginalData()->getArrayCopy());
725761

726-
$this->assertInstanceOf(IdentityInterface::class, $identity);
727-
$this->assertInstanceOf(AuthenticationService::class, $service);
728-
$this->assertTrue($service->getResult()->isValid());
729-
$this->assertEquals($data, $identity->getOriginalData()->getArrayCopy());
762+
return $response;
763+
};
764+
$middleware = new AuthenticationMiddleware($this->service);
765+
$middleware($request, $response, $next);
730766
}
731767

732768
/**

0 commit comments

Comments
 (0)