From 4092e599c372bfd003bd483a53fff1e204bfc7f2 Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Wed, 1 Apr 2020 08:21:42 +0000 Subject: [PATCH 01/35] AutoLoginMiddleware #218 --- src/User/AutoLoginMiddleware.php | 67 +++++++++++++++++- tests/User/AutoLoginMiddlewareTest.php | 98 ++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 3 deletions(-) create mode 100644 tests/User/AutoLoginMiddlewareTest.php diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index e9f7f5847..c2bb813dd 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -2,15 +2,76 @@ namespace Yiisoft\Yii\Web\User; +use Psr\Http\Server\RequestHandlerInterface; +use Psr\Http\Message\ResponseFactoryInterface; +use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Server\MiddlewareInterface; +use Yiisoft\Auth\IdentityInterface; +use Yiisoft\Auth\IdentityRepositoryInterface; +use Yiisoft\Yii\Web\User\User; + /** * AutoLoginMiddleware automatically logs user in based on "remember me" cookie */ -class AutoLoginMiddleware +class AutoLoginMiddleware implements MiddlewareInterface { private User $user; + private IdentityRepositoryInterface $identityRepository; - public function __construct(User $user) - { + public function __construct( + User $user, + IdentityRepositoryInterface $identityRepository + ) { $this->user = $user; + $this->identityRepository = $identityRepository; + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + $data = $this->getIdentityAndDurationFromCookie($request); + if ($this->user->login($data['identity'], $data['duration'])) { + try { + $response = $handler->handle($request); + } catch (\Throwable $e) { + throw $e; + } + + return $response; + } else { + throw new \Exception("Error authentication"); + } + } + + /** + * Determines if an identity cookie has a valid format and contains a valid auth key. + * This method is used when [[enableAutoLogin]] is true. + * This method attempts to authenticate a user using the information in the identity cookie. + * @param ServerRequestInterface $request Request to handle + * @return array|null Returns an array of 'identity' and 'duration' if valid, otherwise null. + */ + protected function getIdentityAndDurationFromCookie(ServerRequestInterface $request) + { + $cookies = $request->getCookieParams(); + $value = $cookies['remember'] ?? null; + + if ($value === null) { + return null; + } + + $data = json_decode($value, true); + if (is_array($data) && count($data) == 3) { + list($id, $authKey, $duration) = $data; + $identity = $this->identityRepository->findIdentity($id); + if ($identity !== null) { + if (!$identity instanceof IdentityInterface) { + throw new \Exception("findIdentity() must return an object implementing IdentityInterface."); + } else { + return ['identity' => $identity, 'duration' => $duration]; + } + } + } + + return null; } } diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php new file mode 100644 index 000000000..301fca69c --- /dev/null +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -0,0 +1,98 @@ +requestHandlerMock = $this->createMock(RequestHandlerInterface::class); + $this->userMock = $this->createMock(User::class); + $this->identityInterfaceMock = $this->createMock(IdentityInterface::class); + $this->identityRepositoryInterfaceMock = $this->createMock(IdentityRepositoryInterface::class); + $this->autoLoginMiddlewareMock = new AutoLoginMiddleware($this->userMock, $this->identityRepositoryInterfaceMock); + $this->requestMock = $this->createMock(ServerRequestInterface::class); + + $this->requestMock + ->expects($this->any()) + ->method('getCookieParams') + ->willReturn([ + "remember" => json_encode([1, '123456', 60]) + ]); + + $this->identityRepositoryInterfaceMock + ->expects($this->any()) + ->method('findIdentity') + ->willReturn($this->identityInterfaceMock); + } + + public function testProcessOK(): void + { + $this->userMock + ->expects($this->once()) + ->method('login') + ->willReturn(true); + + $response = new Response(); + $this->requestHandlerMock + ->expects($this->once()) + ->method('handle') + ->willReturn($response); + + + $this->assertEquals($this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock), $response); + } + + public function testProcessErrorLogin(): void + { + $this->userMock + ->expects($this->once()) + ->method('login') + ->willReturn(false); + + $this->expectException(\Exception::class); + $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); + } +} From 08ba4fde9c96af9987083acdedca6ef089846962 Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Fri, 3 Apr 2020 19:41:26 +0000 Subject: [PATCH 02/35] Fix types #218 --- src/User/AutoLoginMiddleware.php | 36 +++++++++++++------------- tests/User/AutoLoginMiddlewareTest.php | 2 +- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index c2bb813dd..617be2c77 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -1,5 +1,7 @@ getIdentityAndDurationFromCookie($request); - if ($this->user->login($data['identity'], $data['duration'])) { - try { - $response = $handler->handle($request); - } catch (\Throwable $e) { - throw $e; - } + if (!is_null($data)) { + if ($this->user->login($data['identity'], $data['duration'])) { + try { + $response = $handler->handle($request); + } catch (\Throwable $e) { + throw $e; + } - return $response; - } else { - throw new \Exception("Error authentication"); + return $response; + } } + + throw new \Exception("Error authentication"); } /** @@ -50,7 +54,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface * @param ServerRequestInterface $request Request to handle * @return array|null Returns an array of 'identity' and 'duration' if valid, otherwise null. */ - protected function getIdentityAndDurationFromCookie(ServerRequestInterface $request) + private function getIdentityAndDurationFromCookie(ServerRequestInterface $request): ?array { $cookies = $request->getCookieParams(); $value = $cookies['remember'] ?? null; @@ -60,15 +64,11 @@ protected function getIdentityAndDurationFromCookie(ServerRequestInterface $requ } $data = json_decode($value, true); - if (is_array($data) && count($data) == 3) { - list($id, $authKey, $duration) = $data; + if (is_array($data) && count($data) === 3) { + [$id, $authKey, $duration] = $data; $identity = $this->identityRepository->findIdentity($id); if ($identity !== null) { - if (!$identity instanceof IdentityInterface) { - throw new \Exception("findIdentity() must return an object implementing IdentityInterface."); - } else { - return ['identity' => $identity, 'duration' => $duration]; - } + return ['identity' => $identity, 'duration' => $duration]; } } diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index 301fca69c..587b652f0 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -59,7 +59,7 @@ public function setUp(): void ->expects($this->any()) ->method('getCookieParams') ->willReturn([ - "remember" => json_encode([1, '123456', 60]) + "remember" => json_encode(['1', '123456', 60]) ]); $this->identityRepositoryInterfaceMock From 00c872ea50b2e7a16aa4e2cce0fa23efc40d8b8a Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Sat, 4 Apr 2020 14:21:47 +0000 Subject: [PATCH 03/35] More tests and general adjustments #218 --- src/User/AutoLoginMiddleware.php | 39 ++++++++----- tests/User/AutoLoginMiddlewareTest.php | 77 ++++++++++++++++++-------- 2 files changed, 79 insertions(+), 37 deletions(-) diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index 617be2c77..5387d9f02 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -5,11 +5,9 @@ namespace Yiisoft\Yii\Web\User; use Psr\Http\Server\RequestHandlerInterface; -use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; -use Yiisoft\Auth\IdentityInterface; use Yiisoft\Auth\IdentityRepositoryInterface; use Yiisoft\Yii\Web\User\User; @@ -31,20 +29,13 @@ public function __construct( public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - $data = $this->getIdentityAndDurationFromCookie($request); - if (!is_null($data)) { - if ($this->user->login($data['identity'], $data['duration'])) { - try { - $response = $handler->handle($request); - } catch (\Throwable $e) { - throw $e; - } - - return $response; - } + if (!$this->userIsAuth($request)) { + throw new \Exception('Error authentication'); } - throw new \Exception("Error authentication"); + $response = $handler->handle($request); + + return $response; } /** @@ -74,4 +65,24 @@ private function getIdentityAndDurationFromCookie(ServerRequestInterface $reques return null; } + + /** + * Check if the user can be authenticated + * @param ServerRequestInterface $request Request to handle + * @return bool + */ + private function userIsAuth(ServerRequestInterface $request): bool + { + $data = $this->getIdentityAndDurationFromCookie($request); + + if ($data === null) { + return false; + } + + if (!$this->user->login($data['identity'], $data['duration'])) { + return false; + } + + return true; + } } diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index 587b652f0..23a96cf93 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -45,31 +45,12 @@ class AutoLoginMiddlewareTest extends TestCase */ private $userMock; - public function setUp(): void - { - parent::setUp(); - $this->requestHandlerMock = $this->createMock(RequestHandlerInterface::class); - $this->userMock = $this->createMock(User::class); - $this->identityInterfaceMock = $this->createMock(IdentityInterface::class); - $this->identityRepositoryInterfaceMock = $this->createMock(IdentityRepositoryInterface::class); - $this->autoLoginMiddlewareMock = new AutoLoginMiddleware($this->userMock, $this->identityRepositoryInterfaceMock); - $this->requestMock = $this->createMock(ServerRequestInterface::class); - - $this->requestMock - ->expects($this->any()) - ->method('getCookieParams') - ->willReturn([ - "remember" => json_encode(['1', '123456', 60]) - ]); - - $this->identityRepositoryInterfaceMock - ->expects($this->any()) - ->method('findIdentity') - ->willReturn($this->identityInterfaceMock); - } - public function testProcessOK(): void { + $this->mockDataRequest(); + $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60])]); + $this->mockFindIdentity(); + $this->userMock ->expects($this->once()) ->method('login') @@ -87,6 +68,10 @@ public function testProcessOK(): void public function testProcessErrorLogin(): void { + $this->mockDataRequest(); + $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60])]); + $this->mockFindIdentity(); + $this->userMock ->expects($this->once()) ->method('login') @@ -95,4 +80,50 @@ public function testProcessErrorLogin(): void $this->expectException(\Exception::class); $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); } + + public function testProcessCookieEmpty(): void + { + $this->mockDataRequest(); + $this->mockDataCookie([]); + $this->mockFindIdentity(); + + $this->expectException(\Exception::class); + $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); + } + + public function testProcessCookieWithInvalidParams(): void + { + $this->mockDataRequest(); + $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60, "paramInvalid"])]); + $this->mockFindIdentity(); + + $this->expectException(\Exception::class); + $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); + } + + private function mockDataRequest(): void + { + $this->requestHandlerMock = $this->createMock(RequestHandlerInterface::class); + $this->userMock = $this->createMock(User::class); + $this->identityInterfaceMock = $this->createMock(IdentityInterface::class); + $this->identityRepositoryInterfaceMock = $this->createMock(IdentityRepositoryInterface::class); + $this->autoLoginMiddlewareMock = new AutoLoginMiddleware($this->userMock, $this->identityRepositoryInterfaceMock); + $this->requestMock = $this->createMock(ServerRequestInterface::class); + } + + private function mockDataCookie(array $cookie): void + { + $this->requestMock + ->expects($this->any()) + ->method('getCookieParams') + ->willReturn($cookie); + } + + private function mockFindIdentity(): void + { + $this->identityRepositoryInterfaceMock + ->expects($this->any()) + ->method('findIdentity') + ->willReturn($this->identityInterfaceMock); + } } From d5f52bff3a48feb89170a6831b1fb6f9a21d534f Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Sat, 4 Apr 2020 17:25:18 +0000 Subject: [PATCH 04/35] Adjustments in methods and tests #218 --- src/User/AutoLoginMiddleware.php | 47 +++++++++++--------------- tests/User/AutoLoginMiddlewareTest.php | 7 ++-- 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index 5387d9f02..052bac987 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -33,37 +33,34 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface throw new \Exception('Error authentication'); } - $response = $handler->handle($request); - - return $response; + return $handler->handle($request); } /** - * Determines if an identity cookie has a valid format and contains a valid auth key. - * This method is used when [[enableAutoLogin]] is true. - * This method attempts to authenticate a user using the information in the identity cookie. + * Parse and determines if an identity cookie has a valid format. * @param ServerRequestInterface $request Request to handle - * @return array|null Returns an array of 'identity' and 'duration' if valid, otherwise null. + * @return array Returns an array of 'identity' and 'duration' if valid, otherwise []. */ - private function getIdentityAndDurationFromCookie(ServerRequestInterface $request): ?array + private function parseCredentials(ServerRequestInterface $request): array { - $cookies = $request->getCookieParams(); - $value = $cookies['remember'] ?? null; + try { + $cookies = $request->getCookieParams(); + $data = json_decode($cookies['remember'], true, 512); + } catch (\Exception $e) { + return []; + } - if ($value === null) { - return null; + if (!is_array($data) || count($data) !== 3) { + return []; } - $data = json_decode($value, true); - if (is_array($data) && count($data) === 3) { - [$id, $authKey, $duration] = $data; - $identity = $this->identityRepository->findIdentity($id); - if ($identity !== null) { - return ['identity' => $identity, 'duration' => $duration]; - } + [$id, , $duration] = $data; + $identity = $this->identityRepository->findIdentity($id); + if ($identity === null) { + return []; } - return null; + return ['identity' => $identity, 'duration' => $duration]; } /** @@ -73,16 +70,12 @@ private function getIdentityAndDurationFromCookie(ServerRequestInterface $reques */ private function userIsAuth(ServerRequestInterface $request): bool { - $data = $this->getIdentityAndDurationFromCookie($request); - - if ($data === null) { - return false; - } + $data = $this->parseCredentials($request); - if (!$this->user->login($data['identity'], $data['duration'])) { + if ($data === []) { return false; } - return true; + return $this->user->login($data['identity'], $data['duration']); } } diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index 23a96cf93..26fc0ffd3 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -1,12 +1,9 @@ Date: Mon, 6 Apr 2020 18:14:24 +0000 Subject: [PATCH 05/35] Change name of method and fix description #218 --- src/User/AutoLoginMiddleware.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index 052bac987..20a8289d8 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -29,7 +29,7 @@ public function __construct( public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - if (!$this->userIsAuth($request)) { + if (!$this->authenticateUserFromRequest($request)) { throw new \Exception('Error authentication'); } @@ -45,7 +45,7 @@ private function parseCredentials(ServerRequestInterface $request): array { try { $cookies = $request->getCookieParams(); - $data = json_decode($cookies['remember'], true, 512); + $data = json_decode($cookies['remember'], true, 512, JSON_THROW_ON_ERROR); } catch (\Exception $e) { return []; } @@ -64,11 +64,11 @@ private function parseCredentials(ServerRequestInterface $request): array } /** - * Check if the user can be authenticated + * Check if the user can authenticate and if everything is ok, authenticate * @param ServerRequestInterface $request Request to handle * @return bool */ - private function userIsAuth(ServerRequestInterface $request): bool + private function authenticateUserFromRequest(ServerRequestInterface $request): bool { $data = $this->parseCredentials($request); From f1654e23cb3df0b95f2f66ec3b60453ba03b7cc1 Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Tue, 7 Apr 2020 20:05:35 +0000 Subject: [PATCH 06/35] Logger interface for warning in error authentication #218 --- src/User/AutoLoginMiddleware.php | 8 +++- tests/User/AutoLoginMiddlewareTest.php | 60 +++++++++++++++++++++++--- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index 20a8289d8..ce115fc96 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -8,6 +8,7 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; +use Psr\Log\LoggerInterface; use Yiisoft\Auth\IdentityRepositoryInterface; use Yiisoft\Yii\Web\User\User; @@ -18,19 +19,22 @@ final class AutoLoginMiddleware implements MiddlewareInterface { private User $user; private IdentityRepositoryInterface $identityRepository; + private LoggerInterface $logger; public function __construct( User $user, - IdentityRepositoryInterface $identityRepository + IdentityRepositoryInterface $identityRepository, + LoggerInterface $logger ) { $this->user = $user; $this->identityRepository = $identityRepository; + $this->logger = $logger; } public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { if (!$this->authenticateUserFromRequest($request)) { - throw new \Exception('Error authentication'); + $this->logger->warning('Unable to authenticate used by cookie.'); } return $handler->handle($request); diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index 26fc0ffd3..6d5bd5d6e 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -6,6 +6,8 @@ use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; +use Psr\Log\LogLevel; +use Yiisoft\Log\Logger; use Yiisoft\Auth\IdentityInterface; use Yiisoft\Auth\IdentityRepositoryInterface; use Yiisoft\Yii\Web\User\User; @@ -37,6 +39,12 @@ class AutoLoginMiddlewareTest extends TestCase * @var IdentityInterface */ private $identityInterfaceMock; + + /** + * @var Logger + */ + private $loggerMock; + /** * @var User */ @@ -59,7 +67,6 @@ public function testProcessOK(): void ->method('handle') ->willReturn($response); - $this->assertEquals($this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock), $response); } @@ -74,8 +81,13 @@ public function testProcessErrorLogin(): void ->method('login') ->willReturn(false); - $this->expectException(\Exception::class); + $memory = memory_get_usage(); + $this->loggerMock->setTraceLevel(3); + $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); + + $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); + $this->assertEquals($messages[0][1], 'Unable to authenticate used by cookie.'); } public function testProcessCookieEmpty(): void @@ -84,8 +96,13 @@ public function testProcessCookieEmpty(): void $this->mockDataCookie([]); $this->mockFindIdentity(); - $this->expectException(\Exception::class); + $memory = memory_get_usage(); + $this->loggerMock->setTraceLevel(3); + $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); + + $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); + $this->assertEquals($messages[0][1], 'Unable to authenticate used by cookie.'); } public function testProcessCookieWithInvalidParams(): void @@ -94,8 +111,13 @@ public function testProcessCookieWithInvalidParams(): void $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60, "paramInvalid"])]); $this->mockFindIdentity(); - $this->expectException(\Exception::class); + $memory = memory_get_usage(); + $this->loggerMock->setTraceLevel(3); + $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); + + $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); + $this->assertEquals($messages[0][1], 'Unable to authenticate used by cookie.'); } private function mockDataRequest(): void @@ -103,8 +125,13 @@ private function mockDataRequest(): void $this->requestHandlerMock = $this->createMock(RequestHandlerInterface::class); $this->userMock = $this->createMock(User::class); $this->identityInterfaceMock = $this->createMock(IdentityInterface::class); + + $this->loggerMock = $this->getMockBuilder(Logger::class) + ->onlyMethods(['dispatch']) + ->getMock(); + $this->identityRepositoryInterfaceMock = $this->createMock(IdentityRepositoryInterface::class); - $this->autoLoginMiddlewareMock = new AutoLoginMiddleware($this->userMock, $this->identityRepositoryInterfaceMock); + $this->autoLoginMiddlewareMock = new AutoLoginMiddleware($this->userMock, $this->identityRepositoryInterfaceMock, $this->loggerMock); $this->requestMock = $this->createMock(ServerRequestInterface::class); } @@ -123,4 +150,27 @@ private function mockFindIdentity(): void ->method('findIdentity') ->willReturn($this->identityInterfaceMock); } + + /** + * Gets an inaccessible object property. + * @param $object + * @param $propertyName + * @param bool $revoke whether to make property inaccessible after getting + * @return mixed + * @throws \ReflectionException + */ + protected function getInaccessibleProperty($object, $propertyName, bool $revoke = true) + { + $class = new \ReflectionClass($object); + while (!$class->hasProperty($propertyName)) { + $class = $class->getParentClass(); + } + $property = $class->getProperty($propertyName); + $property->setAccessible(true); + $result = $property->getValue($object); + if ($revoke) { + $property->setAccessible(false); + } + return $result; + } } From 61632e173c4682a2b2872b4075a6516bf21cd7a0 Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Wed, 8 Apr 2020 19:27:07 +0000 Subject: [PATCH 07/35] Cookie login #218 --- src/User/AutoLoginMiddleware.php | 7 +++- src/User/User.php | 45 ++++++++++++++++++++++++++ tests/User/AutoLoginMiddlewareTest.php | 29 +++++++++++++++-- 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index ce115fc96..61b6edfdb 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -58,12 +58,17 @@ private function parseCredentials(ServerRequestInterface $request): array return []; } - [$id, , $duration] = $data; + [$id, $authKey, $duration] = $data; $identity = $this->identityRepository->findIdentity($id); if ($identity === null) { return []; } + if (!$this->user->validateAuthKey($authKey)) { + $this->logger->warning('Unable to authenticate used by cookie. Invalid auth key.'); + return []; + } + return ['identity' => $identity, 'duration' => $duration]; } diff --git a/src/User/User.php b/src/User/User.php index 7cceb1349..00577920e 100644 --- a/src/User/User.php +++ b/src/User/User.php @@ -2,10 +2,12 @@ namespace Yiisoft\Yii\Web\User; +use Nyholm\Psr7\Response; use Psr\EventDispatcher\EventDispatcherInterface; use Yiisoft\Access\AccessCheckerInterface; use Yiisoft\Auth\IdentityInterface; use Yiisoft\Auth\IdentityRepositoryInterface; +use Yiisoft\Yii\Web\Cookie; use Yiisoft\Yii\Web\Session\SessionInterface; use Yiisoft\Yii\Web\User\Event\AfterLogin; use Yiisoft\Yii\Web\User\Event\AfterLogout; @@ -111,6 +113,48 @@ public function setIdentity(IdentityInterface $identity): void $this->identity = $identity; } + /** + * Return the auth key value + * + * @return string Auth key value + */ + public function getAuthKey(): string + { + return 'ABCD1234'; + } + + /** + * Validate auth key + * + * @param String $authKey Auth key to validate + * @return bool True if is valid + */ + public function validateAuthKey($authKey): bool + { + return $authKey === 'ABCD1234'; + } + + /** + * Sends an identity cookie. + * + * @param IdentityInterface $identity + * @param int $duration number of seconds that the user can remain in logged-in status. + */ + protected function sendIdentityCookie(IdentityInterface $identity, int $duration): void + { + $data = json_encode([ + $identity->getId(), + $this->getAuthKey(), + $duration, + ], + JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE + ); + + $cookieIdentity = (new Cookie('remember', $data))->expireAt(time() + $duration); + $response = new Response(); + $cookieIdentity->addToResponse($response); + } + /** * Logs in a user. * @@ -135,6 +179,7 @@ public function login(IdentityInterface $identity, int $duration = 0): bool if ($this->beforeLogin($identity, $duration)) { $this->switchIdentity($identity); $this->afterLogin($identity, $duration); + $this->sendIdentityCookie($identity, $duration); } return !$this->isGuest(); } diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index 6d5bd5d6e..3640436d4 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -53,9 +53,14 @@ class AutoLoginMiddlewareTest extends TestCase public function testProcessOK(): void { $this->mockDataRequest(); - $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60])]); + $this->mockDataCookie(["remember" => json_encode(['1', 'ABCD1234', 60])]); $this->mockFindIdentity(); + $this->userMock + ->expects($this->once()) + ->method('validateAuthKey') + ->willReturn(true); + $this->userMock ->expects($this->once()) ->method('login') @@ -73,9 +78,14 @@ public function testProcessOK(): void public function testProcessErrorLogin(): void { $this->mockDataRequest(); - $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60])]); + $this->mockDataCookie(["remember" => json_encode(['1', 'ABCD1234', 60])]); $this->mockFindIdentity(); + $this->userMock + ->expects($this->once()) + ->method('validateAuthKey') + ->willReturn(true); + $this->userMock ->expects($this->once()) ->method('login') @@ -90,6 +100,21 @@ public function testProcessErrorLogin(): void $this->assertEquals($messages[0][1], 'Unable to authenticate used by cookie.'); } + public function testProcessInvalidAuthKey(): void + { + $this->mockDataRequest(); + $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60])]); + $this->mockFindIdentity(); + + $memory = memory_get_usage(); + $this->loggerMock->setTraceLevel(3); + + $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); + + $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); + $this->assertEquals($messages[0][1], 'Unable to authenticate used by cookie. Invalid auth key.'); + } + public function testProcessCookieEmpty(): void { $this->mockDataRequest(); From a07ecbc7bca7b8727d2bacfcc15c19ab5c949c2f Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Wed, 8 Apr 2020 19:34:54 +0000 Subject: [PATCH 08/35] Fix style #218 --- src/User/User.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/User/User.php b/src/User/User.php index 00577920e..6c2091d07 100644 --- a/src/User/User.php +++ b/src/User/User.php @@ -142,7 +142,8 @@ public function validateAuthKey($authKey): bool */ protected function sendIdentityCookie(IdentityInterface $identity, int $duration): void { - $data = json_encode([ + $data = json_encode( + [ $identity->getId(), $this->getAuthKey(), $duration, From e7de628adbd17d3958fae934c974bc910a2ee6f5 Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Sat, 11 Apr 2020 13:29:17 +0000 Subject: [PATCH 09/35] Remove cookie remeber in logout #218 --- src/User/User.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/User/User.php b/src/User/User.php index 6c2091d07..8226b7250 100644 --- a/src/User/User.php +++ b/src/User/User.php @@ -9,12 +9,13 @@ use Yiisoft\Auth\IdentityRepositoryInterface; use Yiisoft\Yii\Web\Cookie; use Yiisoft\Yii\Web\Session\SessionInterface; +use Yiisoft\Yii\Web\User\AuthenticationKeyInterface; use Yiisoft\Yii\Web\User\Event\AfterLogin; use Yiisoft\Yii\Web\User\Event\AfterLogout; use Yiisoft\Yii\Web\User\Event\BeforeLogin; use Yiisoft\Yii\Web\User\Event\BeforeLogout; -class User +class User implements AuthenticationKeyInterface { private const SESSION_AUTH_ID = '__auth_id'; private const SESSION_AUTH_EXPIRE = '__auth_expire'; @@ -151,7 +152,9 @@ protected function sendIdentityCookie(IdentityInterface $identity, int $duration JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE ); - $cookieIdentity = (new Cookie('remember', $data))->expireAt(time() + $duration); + $expireDateTime = new \DateTime(); + $expireDateTime->setTimestamp(time() + $duration); + $cookieIdentity = (new Cookie('remember', $data))->expireAt($expireDateTime); $response = new Response(); $cookieIdentity->addToResponse($response); } @@ -225,6 +228,12 @@ public function logout($destroySession = true): bool if ($destroySession && $this->session) { $this->session->destroy(); } + + // Remove the cookie + $expireDateTime = new \DateTime(); + $expireDateTime->modify("-1 day"); + (new Cookie('remember', ""))->expireAt($expireDateTime); + $this->afterLogout($identity); } return $this->isGuest(); From 80442d2479e20d36cd3c3f4f67b345c94a0d2546 Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Sun, 12 Apr 2020 09:50:30 +0000 Subject: [PATCH 10/35] Change to DateTimeImmutable and fix tests #218 --- src/User/User.php | 4 ++-- tests/User/AutoLoginMiddlewareTest.php | 19 ++++++------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/User/User.php b/src/User/User.php index 8226b7250..d543243da 100644 --- a/src/User/User.php +++ b/src/User/User.php @@ -152,7 +152,7 @@ protected function sendIdentityCookie(IdentityInterface $identity, int $duration JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE ); - $expireDateTime = new \DateTime(); + $expireDateTime = new \DateTimeImmutable(); $expireDateTime->setTimestamp(time() + $duration); $cookieIdentity = (new Cookie('remember', $data))->expireAt($expireDateTime); $response = new Response(); @@ -230,7 +230,7 @@ public function logout($destroySession = true): bool } // Remove the cookie - $expireDateTime = new \DateTime(); + $expireDateTime = new \DateTimeImmutable(); $expireDateTime->modify("-1 day"); (new Cookie('remember', ""))->expireAt($expireDateTime); diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index 3640436d4..c7c157c26 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -54,7 +54,6 @@ public function testProcessOK(): void { $this->mockDataRequest(); $this->mockDataCookie(["remember" => json_encode(['1', 'ABCD1234', 60])]); - $this->mockFindIdentity(); $this->userMock ->expects($this->once()) @@ -79,7 +78,6 @@ public function testProcessErrorLogin(): void { $this->mockDataRequest(); $this->mockDataCookie(["remember" => json_encode(['1', 'ABCD1234', 60])]); - $this->mockFindIdentity(); $this->userMock ->expects($this->once()) @@ -104,7 +102,6 @@ public function testProcessInvalidAuthKey(): void { $this->mockDataRequest(); $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60])]); - $this->mockFindIdentity(); $memory = memory_get_usage(); $this->loggerMock->setTraceLevel(3); @@ -119,7 +116,6 @@ public function testProcessCookieEmpty(): void { $this->mockDataRequest(); $this->mockDataCookie([]); - $this->mockFindIdentity(); $memory = memory_get_usage(); $this->loggerMock->setTraceLevel(3); @@ -134,7 +130,6 @@ public function testProcessCookieWithInvalidParams(): void { $this->mockDataRequest(); $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60, "paramInvalid"])]); - $this->mockFindIdentity(); $memory = memory_get_usage(); $this->loggerMock->setTraceLevel(3); @@ -156,6 +151,12 @@ private function mockDataRequest(): void ->getMock(); $this->identityRepositoryInterfaceMock = $this->createMock(IdentityRepositoryInterface::class); + + $this->identityRepositoryInterfaceMock + ->expects($this->any()) + ->method('findIdentity') + ->willReturn($this->identityInterfaceMock); + $this->autoLoginMiddlewareMock = new AutoLoginMiddleware($this->userMock, $this->identityRepositoryInterfaceMock, $this->loggerMock); $this->requestMock = $this->createMock(ServerRequestInterface::class); } @@ -168,14 +169,6 @@ private function mockDataCookie(array $cookie): void ->willReturn($cookie); } - private function mockFindIdentity(): void - { - $this->identityRepositoryInterfaceMock - ->expects($this->any()) - ->method('findIdentity') - ->willReturn($this->identityInterfaceMock); - } - /** * Gets an inaccessible object property. * @param $object From a1841aa12534716e6c9b366cb3b13a8a729cff77 Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Sun, 12 Apr 2020 10:30:31 +0000 Subject: [PATCH 11/35] Improvements tests #218 --- tests/User/AutoLoginMiddlewareTest.php | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index c7c157c26..b98ca1eeb 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -89,9 +89,6 @@ public function testProcessErrorLogin(): void ->method('login') ->willReturn(false); - $memory = memory_get_usage(); - $this->loggerMock->setTraceLevel(3); - $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); @@ -103,9 +100,6 @@ public function testProcessInvalidAuthKey(): void $this->mockDataRequest(); $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60])]); - $memory = memory_get_usage(); - $this->loggerMock->setTraceLevel(3); - $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); @@ -117,9 +111,6 @@ public function testProcessCookieEmpty(): void $this->mockDataRequest(); $this->mockDataCookie([]); - $memory = memory_get_usage(); - $this->loggerMock->setTraceLevel(3); - $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); @@ -131,9 +122,6 @@ public function testProcessCookieWithInvalidParams(): void $this->mockDataRequest(); $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60, "paramInvalid"])]); - $memory = memory_get_usage(); - $this->loggerMock->setTraceLevel(3); - $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); From 02558b90a1c910028f3d1168d34f53ee11a396f3 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 16 Apr 2020 23:48:34 +0300 Subject: [PATCH 12/35] Fix typos in exception messages, remove unused import --- src/User/AutoLoginMiddleware.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index 61b6edfdb..194d2570a 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -10,7 +10,6 @@ use Psr\Http\Server\MiddlewareInterface; use Psr\Log\LoggerInterface; use Yiisoft\Auth\IdentityRepositoryInterface; -use Yiisoft\Yii\Web\User\User; /** * AutoLoginMiddleware automatically logs user in based on "remember me" cookie @@ -34,7 +33,7 @@ public function __construct( public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { if (!$this->authenticateUserFromRequest($request)) { - $this->logger->warning('Unable to authenticate used by cookie.'); + $this->logger->warning('Unable to authenticate user by cookie.'); } return $handler->handle($request); @@ -65,7 +64,7 @@ private function parseCredentials(ServerRequestInterface $request): array } if (!$this->user->validateAuthKey($authKey)) { - $this->logger->warning('Unable to authenticate used by cookie. Invalid auth key.'); + $this->logger->warning('Unable to authenticate user by cookie. Invalid auth key.'); return []; } From 47969ef4023088122ff5b53a9f5134605edc28c5 Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Sat, 18 Apr 2020 11:59:23 +0000 Subject: [PATCH 13/35] Response cookie #218 --- src/User/AutoLoginMiddleware.php | 10 +++++---- src/User/User.php | 31 ++++++++++++++++++-------- tests/User/AutoLoginMiddlewareTest.php | 8 +++---- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index 194d2570a..b64947b38 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -32,11 +32,12 @@ public function __construct( public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - if (!$this->authenticateUserFromRequest($request)) { + $response = $handler->handle($request); + if (!$this->authenticateUserFromRequest($request, $response)) { $this->logger->warning('Unable to authenticate user by cookie.'); } - return $handler->handle($request); + return $response; } /** @@ -74,9 +75,10 @@ private function parseCredentials(ServerRequestInterface $request): array /** * Check if the user can authenticate and if everything is ok, authenticate * @param ServerRequestInterface $request Request to handle + * @param ResponseInterface $response Response to handle * @return bool */ - private function authenticateUserFromRequest(ServerRequestInterface $request): bool + private function authenticateUserFromRequest(ServerRequestInterface $request, ResponseInterface $response): bool { $data = $this->parseCredentials($request); @@ -84,6 +86,6 @@ private function authenticateUserFromRequest(ServerRequestInterface $request): b return false; } - return $this->user->login($data['identity'], $data['duration']); + return $this->user->login($data['identity'], $data['duration'], $response); } } diff --git a/src/User/User.php b/src/User/User.php index d543243da..fc774c8ac 100644 --- a/src/User/User.php +++ b/src/User/User.php @@ -2,20 +2,19 @@ namespace Yiisoft\Yii\Web\User; -use Nyholm\Psr7\Response; use Psr\EventDispatcher\EventDispatcherInterface; +use Psr\Http\Message\ResponseInterface; use Yiisoft\Access\AccessCheckerInterface; use Yiisoft\Auth\IdentityInterface; use Yiisoft\Auth\IdentityRepositoryInterface; use Yiisoft\Yii\Web\Cookie; use Yiisoft\Yii\Web\Session\SessionInterface; -use Yiisoft\Yii\Web\User\AuthenticationKeyInterface; use Yiisoft\Yii\Web\User\Event\AfterLogin; use Yiisoft\Yii\Web\User\Event\AfterLogout; use Yiisoft\Yii\Web\User\Event\BeforeLogin; use Yiisoft\Yii\Web\User\Event\BeforeLogout; -class User implements AuthenticationKeyInterface +class User implements IdentityInterface { private const SESSION_AUTH_ID = '__auth_id'; private const SESSION_AUTH_EXPIRE = '__auth_expire'; @@ -24,6 +23,7 @@ class User implements AuthenticationKeyInterface private IdentityRepositoryInterface $identityRepository; private EventDispatcherInterface $eventDispatcher; + private string $identityCookie = 'remember'; private ?AccessCheckerInterface $accessChecker = null; private ?IdentityInterface $identity = null; private ?SessionInterface $session = null; @@ -140,8 +140,9 @@ public function validateAuthKey($authKey): bool * * @param IdentityInterface $identity * @param int $duration number of seconds that the user can remain in logged-in status. + * @param ResponseInterface $response Response to handle */ - protected function sendIdentityCookie(IdentityInterface $identity, int $duration): void + protected function sendIdentityCookie(IdentityInterface $identity, int $duration, ResponseInterface $response): void { $data = json_encode( [ @@ -154,8 +155,7 @@ protected function sendIdentityCookie(IdentityInterface $identity, int $duration $expireDateTime = new \DateTimeImmutable(); $expireDateTime->setTimestamp(time() + $duration); - $cookieIdentity = (new Cookie('remember', $data))->expireAt($expireDateTime); - $response = new Response(); + $cookieIdentity = (new Cookie($this->identityCookie, $data))->expireAt($expireDateTime); $cookieIdentity->addToResponse($response); } @@ -176,14 +176,15 @@ protected function sendIdentityCookie(IdentityInterface $identity, int $duration * * @param IdentityInterface $identity the user identity (which should already be authenticated) * @param int $duration number of seconds that the user can remain in logged-in status, defaults to `0` + * @param ResponseInterface $response Response to handle * @return bool whether the user is logged in */ - public function login(IdentityInterface $identity, int $duration = 0): bool + public function login(IdentityInterface $identity, int $duration = 0, ResponseInterface $response): bool { if ($this->beforeLogin($identity, $duration)) { $this->switchIdentity($identity); $this->afterLogin($identity, $duration); - $this->sendIdentityCookie($identity, $duration); + $this->sendIdentityCookie($identity, $duration, $response); } return !$this->isGuest(); } @@ -232,7 +233,7 @@ public function logout($destroySession = true): bool // Remove the cookie $expireDateTime = new \DateTimeImmutable(); $expireDateTime->modify("-1 day"); - (new Cookie('remember', ""))->expireAt($expireDateTime); + (new Cookie($this->identityCookie, ""))->expireAt($expireDateTime); $this->afterLogout($identity); } @@ -260,6 +261,18 @@ public function getId(): ?string return $this->getIdentity()->getId(); } + /** + * Set the name of the cookie identity + * @param string $name New name of the cookie + * @return this + */ + public function setIdentityCookie(string $name): this + { + $new = clone $this; + $new->identityCookie = $name; + return $new; + } + /** * This method is called before logging in a user. * The default implementation will trigger the [[EVENT_BEFORE_LOGIN]] event. diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index b98ca1eeb..62d96f789 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -92,7 +92,7 @@ public function testProcessErrorLogin(): void $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); - $this->assertEquals($messages[0][1], 'Unable to authenticate used by cookie.'); + $this->assertEquals($messages[0][1], 'Unable to authenticate user by cookie.'); } public function testProcessInvalidAuthKey(): void @@ -103,7 +103,7 @@ public function testProcessInvalidAuthKey(): void $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); - $this->assertEquals($messages[0][1], 'Unable to authenticate used by cookie. Invalid auth key.'); + $this->assertEquals($messages[0][1], 'Unable to authenticate user by cookie. Invalid auth key.'); } public function testProcessCookieEmpty(): void @@ -114,7 +114,7 @@ public function testProcessCookieEmpty(): void $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); - $this->assertEquals($messages[0][1], 'Unable to authenticate used by cookie.'); + $this->assertEquals($messages[0][1], 'Unable to authenticate user by cookie.'); } public function testProcessCookieWithInvalidParams(): void @@ -125,7 +125,7 @@ public function testProcessCookieWithInvalidParams(): void $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); - $this->assertEquals($messages[0][1], 'Unable to authenticate used by cookie.'); + $this->assertEquals($messages[0][1], 'Unable to authenticate user by cookie.'); } private function mockDataRequest(): void From b6ccd4a7d9721eab74bdbbb7c5559efb06a9f36b Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Sat, 18 Apr 2020 14:26:31 +0000 Subject: [PATCH 14/35] Fix type hinting #218 --- src/User/User.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/User/User.php b/src/User/User.php index fc774c8ac..b86be9648 100644 --- a/src/User/User.php +++ b/src/User/User.php @@ -266,7 +266,7 @@ public function getId(): ?string * @param string $name New name of the cookie * @return this */ - public function setIdentityCookie(string $name): this + public function setIdentityCookie(string $name): self { $new = clone $this; $new->identityCookie = $name; From 6a33a34bd0bdd7ad3c715fe8fbb53dddd55a5416 Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Sat, 18 Apr 2020 16:29:34 +0000 Subject: [PATCH 15/35] Fix remove cookie #218 --- src/User/User.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/User/User.php b/src/User/User.php index b86be9648..cd3008d0c 100644 --- a/src/User/User.php +++ b/src/User/User.php @@ -231,9 +231,7 @@ public function logout($destroySession = true): bool } // Remove the cookie - $expireDateTime = new \DateTimeImmutable(); - $expireDateTime->modify("-1 day"); - (new Cookie($this->identityCookie, ""))->expireAt($expireDateTime); + (new Cookie($this->identityCookie, ""))->expire(1); $this->afterLogout($identity); } From 24a3a6f69196dca15fe4978bbfdbe85f975ce552 Mon Sep 17 00:00:00 2001 From: Martin Peveri Date: Sat, 25 Apr 2020 10:54:43 +0000 Subject: [PATCH 16/35] Remove IdentityInterface from User #218 --- src/User/User.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/User/User.php b/src/User/User.php index cd3008d0c..e5a1bb584 100644 --- a/src/User/User.php +++ b/src/User/User.php @@ -14,7 +14,7 @@ use Yiisoft\Yii\Web\User\Event\BeforeLogin; use Yiisoft\Yii\Web\User\Event\BeforeLogout; -class User implements IdentityInterface +class User { private const SESSION_AUTH_ID = '__auth_id'; private const SESSION_AUTH_EXPIRE = '__auth_expire'; From f46eeb8b4245bc24c5f65c9b66a1f42c03fe49e4 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Tue, 5 May 2020 21:44:18 +0300 Subject: [PATCH 17/35] Rename AuthenticationKeyInterface -> AutoLoginIdentityInterface --- ...tionKeyInterface.php => AutoLoginIdentityInterface.php} | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) rename src/User/{AuthenticationKeyInterface.php => AutoLoginIdentityInterface.php} (92%) diff --git a/src/User/AuthenticationKeyInterface.php b/src/User/AutoLoginIdentityInterface.php similarity index 92% rename from src/User/AuthenticationKeyInterface.php rename to src/User/AutoLoginIdentityInterface.php index 8fe86dbe6..f70d63724 100644 --- a/src/User/AuthenticationKeyInterface.php +++ b/src/User/AutoLoginIdentityInterface.php @@ -2,10 +2,9 @@ namespace Yiisoft\Yii\Web\User; -/** - * TODO: seriously need better name for it :( - */ -interface AuthenticationKeyInterface +use Yiisoft\Auth\IdentityInterface; + +interface AutoLoginIdentityInterface extends IdentityInterface { /** * Returns a key that can be used to check the validity of a given identity ID. From be8f65b1d4662264aab2bf20395ac3fdf718bc69 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Tue, 5 May 2020 21:44:54 +0300 Subject: [PATCH 18/35] Lay out feature structure --- src/User/AutoLogin.php | 68 +++++++++++ src/User/AutoLoginIdentityInterface.php | 11 +- src/User/AutoLoginMiddleware.php | 58 +++++---- src/User/Event/AfterLogin.php | 9 +- src/User/Event/BeforeLogin.php | 9 +- src/User/User.php | 155 +++++------------------- 6 files changed, 142 insertions(+), 168 deletions(-) create mode 100644 src/User/AutoLogin.php diff --git a/src/User/AutoLogin.php b/src/User/AutoLogin.php new file mode 100644 index 000000000..8b931938b --- /dev/null +++ b/src/User/AutoLogin.php @@ -0,0 +1,68 @@ +cookieName = $name; + return $new; + } + + /** + * Sends an identity cookie. + * TODO: do it on event? + * TODO: make duration a property of the service? + * + * @param AutoLoginIdentityInterface $identity + * @param int $duration number of seconds that the user can remain in logged-in status. + * @param ResponseInterface $response Response to handle + */ + public function sendCookie(AutoLoginIdentityInterface $identity, int $duration, ResponseInterface $response): void + { + $data = json_encode( + [ + $identity->getId(), + $identity->getAuthKey(), + // $duration, TODO: should we set/check duration separately from cookie expiration? + ], + JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE + ); + + $expireDateTime = new \DateTimeImmutable(); + $expireDateTime->setTimestamp(time() + $duration); + $cookieIdentity = (new Cookie($this->cookieName, $data))->expireAt($expireDateTime); + $cookieIdentity->addToResponse($response); + } + + /** + * Remove auto-login cookie so user is not logged in automatically anymore. + * TODO: trigger on logout? + */ + public function removeCookie(): void + { + // Remove the cookie + // TODO: use new cookie methods + (new Cookie($this->cookieName, ""))->expire(1); + } + + public function getCookieName(): string + { + return $this->cookieName; + } +} diff --git a/src/User/AutoLoginIdentityInterface.php b/src/User/AutoLoginIdentityInterface.php index f70d63724..be0e593c5 100644 --- a/src/User/AutoLoginIdentityInterface.php +++ b/src/User/AutoLoginIdentityInterface.php @@ -4,6 +4,12 @@ use Yiisoft\Auth\IdentityInterface; +/** + * AutoLoginIdentityInterface should be implemented in order to automatically log user in based on a cookie. + * + * @see AutoLogin + * @see AutoLoginMiddleware + */ interface AutoLoginIdentityInterface extends IdentityInterface { /** @@ -14,8 +20,8 @@ interface AutoLoginIdentityInterface extends IdentityInterface * * The space of such keys should be big enough to defeat potential identity attacks. * - * This is required if [[User::enableAutoLogin]] is enabled. The returned key will be stored on the - * client side as a cookie and will be used to authenticate user even if PHP session has been expired. + * The returned key will be stored on the client side as part of a cookie and will be used to authenticate user even + * if PHP session has been expired. * * Make sure to invalidate earlier issued authKeys when you implement force user logout, password change and * other scenarios, that require forceful access revocation for old sessions. @@ -28,7 +34,6 @@ public function getAuthKey(): string; /** * Validates the given auth key. * - * This is required if [[User::enableAutoLogin]] is enabled. * @param string $authKey the given auth key * @return bool whether the given auth key is valid. * @see getAuthKey() diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index b64947b38..f9f261ec3 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -9,83 +9,93 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Log\LoggerInterface; +use Yiisoft\Auth\IdentityInterface; use Yiisoft\Auth\IdentityRepositoryInterface; /** - * AutoLoginMiddleware automatically logs user in based on "remember me" cookie + * AutoLoginMiddleware automatically logs user in based on cookie. */ final class AutoLoginMiddleware implements MiddlewareInterface { private User $user; private IdentityRepositoryInterface $identityRepository; private LoggerInterface $logger; + private AutoLogin $autoLogin; public function __construct( User $user, IdentityRepositoryInterface $identityRepository, - LoggerInterface $logger + LoggerInterface $logger, + AutoLogin $autoLogin ) { $this->user = $user; $this->identityRepository = $identityRepository; $this->logger = $logger; + $this->autoLogin = $autoLogin; } public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - $response = $handler->handle($request); - if (!$this->authenticateUserFromRequest($request, $response)) { + if (!$this->authenticateUserFromRequest($request)) { $this->logger->warning('Unable to authenticate user by cookie.'); } - return $response; + return $handler->handle($request); } /** - * Parse and determines if an identity cookie has a valid format. - * @param ServerRequestInterface $request Request to handle - * @return array Returns an array of 'identity' and 'duration' if valid, otherwise []. + * Parse request and try to create identity out of data present. + * + * @param ServerRequestInterface $request Request to parse. + * @return IdentityInterface|null Identity created or null if request data isn't valid. */ - private function parseCredentials(ServerRequestInterface $request): array + private function getIdentityFromRequest(ServerRequestInterface $request): ?IdentityInterface { try { $cookies = $request->getCookieParams(); - $data = json_decode($cookies['remember'], true, 512, JSON_THROW_ON_ERROR); + $cookieName = $this->autoLogin->getCookieName(); + $data = json_decode($cookies[$cookieName], true, 512, JSON_THROW_ON_ERROR); } catch (\Exception $e) { - return []; + return null; } - if (!is_array($data) || count($data) !== 3) { - return []; + if (!is_array($data) || count($data) !== 2) { + return null; } - [$id, $authKey, $duration] = $data; + [$id, $authKey] = $data; $identity = $this->identityRepository->findIdentity($id); if ($identity === null) { - return []; + return null; + } + + if (!$identity instanceof AutoLoginIdentityInterface) { + // TODO: throw or write log? + return null; } - if (!$this->user->validateAuthKey($authKey)) { + if (!$identity->validateAuthKey($authKey)) { $this->logger->warning('Unable to authenticate user by cookie. Invalid auth key.'); - return []; + return null; } - return ['identity' => $identity, 'duration' => $duration]; + return $identity; } /** - * Check if the user can authenticate and if everything is ok, authenticate + * Authenticate user if there is data to do so in request. + * * @param ServerRequestInterface $request Request to handle - * @param ResponseInterface $response Response to handle * @return bool */ - private function authenticateUserFromRequest(ServerRequestInterface $request, ResponseInterface $response): bool + private function authenticateUserFromRequest(ServerRequestInterface $request): bool { - $data = $this->parseCredentials($request); + $identity = $this->getIdentityFromRequest($request); - if ($data === []) { + if ($identity === null) { return false; } - return $this->user->login($data['identity'], $data['duration'], $response); + return $this->user->login($identity); } } diff --git a/src/User/Event/AfterLogin.php b/src/User/Event/AfterLogin.php index 42e9cc0a7..90ef57ba3 100644 --- a/src/User/Event/AfterLogin.php +++ b/src/User/Event/AfterLogin.php @@ -7,21 +7,14 @@ class AfterLogin { private IdentityInterface $identity; - private int $duration; - public function __construct(IdentityInterface $identity, int $duration) + public function __construct(IdentityInterface $identity) { $this->identity = $identity; - $this->duration = $duration; } public function getIdentity(): IdentityInterface { return $this->identity; } - - public function getDuration(): int - { - return $this->duration; - } } diff --git a/src/User/Event/BeforeLogin.php b/src/User/Event/BeforeLogin.php index f721af7ec..5aa272ce4 100644 --- a/src/User/Event/BeforeLogin.php +++ b/src/User/Event/BeforeLogin.php @@ -7,13 +7,11 @@ class BeforeLogin { private IdentityInterface $identity; - private int $duration; private bool $isValid = true; - public function __construct(IdentityInterface $identity, int $duration) + public function __construct(IdentityInterface $identity) { $this->identity = $identity; - $this->duration = $duration; } public function invalidate(): void @@ -30,9 +28,4 @@ public function getIdentity(): IdentityInterface { return $this->identity; } - - public function getDuration(): int - { - return $this->duration; - } } diff --git a/src/User/User.php b/src/User/User.php index e5a1bb584..c32c0ebb0 100644 --- a/src/User/User.php +++ b/src/User/User.php @@ -3,17 +3,16 @@ namespace Yiisoft\Yii\Web\User; use Psr\EventDispatcher\EventDispatcherInterface; -use Psr\Http\Message\ResponseInterface; use Yiisoft\Access\AccessCheckerInterface; use Yiisoft\Auth\IdentityInterface; use Yiisoft\Auth\IdentityRepositoryInterface; -use Yiisoft\Yii\Web\Cookie; use Yiisoft\Yii\Web\Session\SessionInterface; use Yiisoft\Yii\Web\User\Event\AfterLogin; use Yiisoft\Yii\Web\User\Event\AfterLogout; use Yiisoft\Yii\Web\User\Event\BeforeLogin; use Yiisoft\Yii\Web\User\Event\BeforeLogout; +// TODO: declare it final? class User { private const SESSION_AUTH_ID = '__auth_id'; @@ -23,7 +22,6 @@ class User private IdentityRepositoryInterface $identityRepository; private EventDispatcherInterface $eventDispatcher; - private string $identityCookie = 'remember'; private ?AccessCheckerInterface $accessChecker = null; private ?IdentityInterface $identity = null; private ?SessionInterface $session = null; @@ -39,22 +37,16 @@ public function __construct( /** * @var int|null the number of seconds in which the user will be logged out automatically if he * remains inactive. If this property is not set, the user will be logged out after - * the current session expires (c.f. [[Session::timeout]]). + * the current session expires. */ public ?int $authTimeout = null; /** * @var int|null the number of seconds in which the user will be logged out automatically * regardless of activity. - * Note that this will not work if [[enableAutoLogin]] is `true`. */ public ?int $absoluteAuthTimeout = null; - /** - * @var array MIME types for which this component should redirect to the [[loginUrl]]. - */ - public array $acceptableRedirectTypes = ['text/html', 'application/xhtml+xml']; - /** * Set session to persist authentication status across multiple requests. * If not set, authentication has to be performed on each request, which is often the case @@ -74,10 +66,9 @@ public function setAccessChecker(AccessCheckerInterface $accessChecker): void /** * Returns the identity object associated with the currently logged-in user. - * When [[enableSession]] is true, this method may attempt to read the user's authentication data + * This method read the user's authentication data * stored in session and reconstruct the corresponding identity object, if it has not done so before. * @param bool $autoRenew whether to automatically renew authentication status if it has not been done so before. - * This is only useful when [[enableSession]] is true. * @return IdentityInterface the identity object associated with the currently logged-in user. * @throws \Throwable * @see logout() @@ -103,7 +94,7 @@ public function getIdentity($autoRenew = true): IdentityInterface /** * Sets the user identity object. * - * Note that this method does not deal with session or cookie. You should usually use [[switchIdentity()]] + * Note that this method does not deal with session. You should usually use {@see switchIdentity()} * to change the identity of the current user. * * @param IdentityInterface|null $identity the identity object associated with the currently logged user. @@ -114,91 +105,36 @@ public function setIdentity(IdentityInterface $identity): void $this->identity = $identity; } - /** - * Return the auth key value - * - * @return string Auth key value - */ - public function getAuthKey(): string - { - return 'ABCD1234'; - } - - /** - * Validate auth key - * - * @param String $authKey Auth key to validate - * @return bool True if is valid - */ - public function validateAuthKey($authKey): bool - { - return $authKey === 'ABCD1234'; - } - - /** - * Sends an identity cookie. - * - * @param IdentityInterface $identity - * @param int $duration number of seconds that the user can remain in logged-in status. - * @param ResponseInterface $response Response to handle - */ - protected function sendIdentityCookie(IdentityInterface $identity, int $duration, ResponseInterface $response): void - { - $data = json_encode( - [ - $identity->getId(), - $this->getAuthKey(), - $duration, - ], - JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE - ); - - $expireDateTime = new \DateTimeImmutable(); - $expireDateTime->setTimestamp(time() + $duration); - $cookieIdentity = (new Cookie($this->identityCookie, $data))->expireAt($expireDateTime); - $cookieIdentity->addToResponse($response); - } - /** * Logs in a user. * * After logging in a user: - * - the user's identity information is obtainable from the [[identity]] property - * - * If [[enableSession]] is `true`: - * - the identity information will be stored in session and be available in the next requests - * - in case of `$duration == 0`: as long as the session remains active or till the user closes the browser - * - in case of `$duration > 0`: as long as the session remains active or as long as the cookie - * remains valid by it's `$duration` in seconds when [[enableAutoLogin]] is set `true`. - * - * If [[enableSession]] is `false`: - * - the `$duration` parameter will be ignored + * - the user's identity information is obtainable from the {@see getIdentity()} + * - the identity information will be stored in session and be available in the next requests as long as the session + * remains active or till the user closes the browser. Some browsers, such as Chrome, are keeping session when + * browser is re-opened. * * @param IdentityInterface $identity the user identity (which should already be authenticated) - * @param int $duration number of seconds that the user can remain in logged-in status, defaults to `0` - * @param ResponseInterface $response Response to handle * @return bool whether the user is logged in */ - public function login(IdentityInterface $identity, int $duration = 0, ResponseInterface $response): bool + public function login(IdentityInterface $identity): bool { - if ($this->beforeLogin($identity, $duration)) { + if ($this->beforeLogin($identity)) { $this->switchIdentity($identity); - $this->afterLogin($identity, $duration); - $this->sendIdentityCookie($identity, $duration, $response); + $this->afterLogin($identity); } return !$this->isGuest(); } /** * Logs in a user by the given access token. - * This method will first authenticate the user by calling [[IdentityInterface::findIdentityByAccessToken()]] - * with the provided access token. If successful, it will call [[login()]] to log in the authenticated user. - * If authentication fails or [[login()]] is unsuccessful, it will return null. + * This method will first authenticate the user by calling {@see IdentityInterface::findIdentityByToken()} + * with the provided access token. If successful, it will call {@see login()} to log in the authenticated user. + * If authentication fails or {@see login()} is unsuccessful, it will return null. * @param string $token the access token * @param string $type the type of the token. The value of this parameter depends on the implementation. - * For example, [[\yii\filters\auth\HttpBearerAuth]] will set this parameter to be `yii\filters\auth\HttpBearerAuth`. * @return IdentityInterface|null the identity associated with the given access token. Null is returned if - * the access token is invalid or [[login()]] is unsuccessful. + * the access token is invalid or {@see login()} is unsuccessful. */ public function loginByAccessToken(string $token, string $type = null): ?IdentityInterface { @@ -214,7 +150,6 @@ public function loginByAccessToken(string $token, string $type = null): ?Identit * This will remove authentication-related session data. * If `$destroySession` is true, all session data will be removed. * @param bool $destroySession whether to destroy the whole session. Defaults to true. - * This parameter is ignored if [[enableSession]] is false. * @return bool whether the user is logged out * @throws \Throwable */ @@ -230,9 +165,6 @@ public function logout($destroySession = true): bool $this->session->destroy(); } - // Remove the cookie - (new Cookie($this->identityCookie, ""))->expire(1); - $this->afterLogout($identity); } return $this->isGuest(); @@ -259,58 +191,36 @@ public function getId(): ?string return $this->getIdentity()->getId(); } - /** - * Set the name of the cookie identity - * @param string $name New name of the cookie - * @return this - */ - public function setIdentityCookie(string $name): self - { - $new = clone $this; - $new->identityCookie = $name; - return $new; - } - /** * This method is called before logging in a user. - * The default implementation will trigger the [[EVENT_BEFORE_LOGIN]] event. + * The default implementation will trigger the {@see BeforeLogin} event. * If you override this method, make sure you call the parent implementation * so that the event is triggered. * @param IdentityInterface $identity the user identity information - * @param int $duration number of seconds that the user can remain in logged-in status. - * If 0, it means login till the user closes the browser or the session is manually destroyed. * @return bool whether the user should continue to be logged in */ - protected function beforeLogin(IdentityInterface $identity, int $duration): bool + private function beforeLogin(IdentityInterface $identity): bool { - $event = new BeforeLogin($identity, $duration); + $event = new BeforeLogin($identity); $this->eventDispatcher->dispatch($event); return $event->isValid(); } /** * This method is called after the user is successfully logged in. - * The default implementation will trigger the [[EVENT_AFTER_LOGIN]] event. - * If you override this method, make sure you call the parent implementation - * so that the event is triggered. * @param IdentityInterface $identity the user identity information - * @param int $duration number of seconds that the user can remain in logged-in status. - * If 0, it means login till the user closes the browser or the session is manually destroyed. */ - protected function afterLogin(IdentityInterface $identity, int $duration): void + private function afterLogin(IdentityInterface $identity): void { - $this->eventDispatcher->dispatch(new AfterLogin($identity, $duration)); + $this->eventDispatcher->dispatch(new AfterLogin($identity)); } /** - * This method is invoked when calling [[logout()]] to log out a user. - * The default implementation will trigger the [[EVENT_BEFORE_LOGOUT]] event. - * If you override this method, make sure you call the parent implementation - * so that the event is triggered. + * This method is invoked when calling {@see logout()} to log out a user. * @param IdentityInterface $identity the user identity information * @return bool whether the user should continue to be logged out */ - protected function beforeLogout(IdentityInterface $identity): bool + private function beforeLogout(IdentityInterface $identity): bool { $event = new BeforeLogout($identity); $this->eventDispatcher->dispatch($event); @@ -318,13 +228,10 @@ protected function beforeLogout(IdentityInterface $identity): bool } /** - * This method is invoked right after a user is logged out via [[logout()]]. - * The default implementation will trigger the [[EVENT_AFTER_LOGOUT]] event. - * If you override this method, make sure you call the parent implementation - * so that the event is triggered. + * This method is invoked right after a user is logged out via {@see logout()}. * @param IdentityInterface $identity the user identity information */ - protected function afterLogout(IdentityInterface $identity): void + private function afterLogout(IdentityInterface $identity): void { $this->eventDispatcher->dispatch(new AfterLogout($identity)); } @@ -332,10 +239,10 @@ protected function afterLogout(IdentityInterface $identity): void /** * Switches to a new identity for the current user. * - * When [[enableSession]] is true, this method may use session and/or cookie to store the user identity information, - * according to the value of `$duration`. Please refer to [[login()]] for more details. + * This method use session to store the user identity information. + * Please refer to {@see login()} for more details. * - * This method is mainly called by [[login()]], [[logout()]] and [[loginByCookie()]] + * This method is mainly called by {@see login()} and {@see logout()} * when the current user needs to be associated with the corresponding identity information. * * @param IdentityInterface $identity the identity information to be associated with the current user. @@ -366,17 +273,15 @@ public function switchIdentity(IdentityInterface $identity): void } /** - * Updates the authentication status using the information from session and cookie. + * Updates the authentication status using the information from session. * * This method will try to determine the user identity using a session variable. * - * If [[authTimeout]] is set, this method will refresh the timer. + * If {@see authTimeout} is set, this method will refresh the timer. * - * If the user identity cannot be determined by session, this method will try to [[loginByCookie()|login by cookie]] - * if [[enableAutoLogin]] is true. * @throws \Throwable */ - protected function renewAuthStatus(): void + private function renewAuthStatus(): void { $id = $this->session->get(self::SESSION_AUTH_ID); From 9e111ec96af29f5dd64925c55bca0c64a6b27ae1 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Tue, 5 May 2020 22:26:20 +0300 Subject: [PATCH 19/35] Adjust AutoLogin to use new cookie methods --- src/User/AutoLogin.php | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/User/AutoLogin.php b/src/User/AutoLogin.php index 8b931938b..fb67d1a72 100644 --- a/src/User/AutoLogin.php +++ b/src/User/AutoLogin.php @@ -25,7 +25,9 @@ public function cookieName(string $name): self } /** - * Sends an identity cookie. + * Add auto-login cookie to response so the user is logged in automatically based on cookie even if session + * is expired. + * * TODO: do it on event? * TODO: make duration a property of the service? * @@ -33,32 +35,26 @@ public function cookieName(string $name): self * @param int $duration number of seconds that the user can remain in logged-in status. * @param ResponseInterface $response Response to handle */ - public function sendCookie(AutoLoginIdentityInterface $identity, int $duration, ResponseInterface $response): void + public function addCookie(AutoLoginIdentityInterface $identity, int $duration, ResponseInterface $response): void { - $data = json_encode( - [ - $identity->getId(), - $identity->getAuthKey(), - // $duration, TODO: should we set/check duration separately from cookie expiration? - ], - JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE - ); + $data = json_encode([ + $identity->getId(), + $identity->getAuthKey() + ], JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); $expireDateTime = new \DateTimeImmutable(); $expireDateTime->setTimestamp(time() + $duration); - $cookieIdentity = (new Cookie($this->cookieName, $data))->expireAt($expireDateTime); + $cookieIdentity = (new Cookie($this->cookieName, $data))->withExpires($expireDateTime); $cookieIdentity->addToResponse($response); } /** - * Remove auto-login cookie so user is not logged in automatically anymore. + * Expire auto-login cookie so user is not logged in automatically anymore. * TODO: trigger on logout? */ - public function removeCookie(): void + public function expireCookie(ResponseInterface $response): void { - // Remove the cookie - // TODO: use new cookie methods - (new Cookie($this->cookieName, ""))->expire(1); + (new Cookie($this->cookieName))->expire()->addToResponse($response); } public function getCookieName(): string From 35d24270f84a95e71118f62d8dd3310015464e0c Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Tue, 5 May 2020 23:38:39 +0300 Subject: [PATCH 20/35] Adjust cookie name --- src/User/AutoLogin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/User/AutoLogin.php b/src/User/AutoLogin.php index fb67d1a72..2e2c4d630 100644 --- a/src/User/AutoLogin.php +++ b/src/User/AutoLogin.php @@ -15,7 +15,7 @@ */ class AutoLogin { - private string $cookieName = 'remember'; + private string $cookieName = 'autoLogin'; public function cookieName(string $name): self { From 7a1be2bd0796476b43cc920c7eddf7855ef7db0f Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Tue, 5 May 2020 23:40:43 +0300 Subject: [PATCH 21/35] Rename variable --- src/User/AutoLogin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/User/AutoLogin.php b/src/User/AutoLogin.php index 2e2c4d630..97e49c565 100644 --- a/src/User/AutoLogin.php +++ b/src/User/AutoLogin.php @@ -44,8 +44,8 @@ public function addCookie(AutoLoginIdentityInterface $identity, int $duration, R $expireDateTime = new \DateTimeImmutable(); $expireDateTime->setTimestamp(time() + $duration); - $cookieIdentity = (new Cookie($this->cookieName, $data))->withExpires($expireDateTime); - $cookieIdentity->addToResponse($response); + $cookie = (new Cookie($this->cookieName, $data))->withExpires($expireDateTime); + $cookie->addToResponse($response); } /** From bba7ab692deabfb816be95830fea39d431ac9575 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Tue, 5 May 2020 23:46:35 +0300 Subject: [PATCH 22/35] Throw exception if repository does not return an interface needed --- src/User/AutoLoginMiddleware.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index f9f261ec3..406b83243 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -70,8 +70,7 @@ private function getIdentityFromRequest(ServerRequestInterface $request): ?Ident } if (!$identity instanceof AutoLoginIdentityInterface) { - // TODO: throw or write log? - return null; + throw new \RuntimeException('Identity repository must return an instance of \Yiisoft\Yii\Web\User\AutoLoginIdentityInterface in order for auto-login to function.'); } if (!$identity->validateAuthKey($authKey)) { From 9a82efd6609d5416368ded9cf68d31dbf4efb3b9 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Wed, 6 May 2020 02:18:04 +0300 Subject: [PATCH 23/35] Cleanup, adjust tests --- src/User/AutoLoginMiddleware.php | 53 ++--- tests/User/AutoLoginIdentity.php | 29 +++ tests/User/AutoLoginMiddlewareTest.php | 288 ++++++++++++++++--------- 3 files changed, 241 insertions(+), 129 deletions(-) create mode 100644 tests/User/AutoLoginIdentity.php diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index 406b83243..199c009b8 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -9,7 +9,6 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Log\LoggerInterface; -use Yiisoft\Auth\IdentityInterface; use Yiisoft\Auth\IdentityRepositoryInterface; /** @@ -36,37 +35,42 @@ public function __construct( public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - if (!$this->authenticateUserFromRequest($request)) { - $this->logger->warning('Unable to authenticate user by cookie.'); - } - + $this->authenticateUserByCookieFromRequest($request); return $handler->handle($request); } /** - * Parse request and try to create identity out of data present. + * Authenticate user by auto login cookie from request. * - * @param ServerRequestInterface $request Request to parse. - * @return IdentityInterface|null Identity created or null if request data isn't valid. + * @param ServerRequestInterface $request Request instance containing auto login cookie. */ - private function getIdentityFromRequest(ServerRequestInterface $request): ?IdentityInterface + private function authenticateUserByCookieFromRequest(ServerRequestInterface $request): void { + $cookieName = $this->autoLogin->getCookieName(); + $cookies = $request->getCookieParams(); + + if (!array_key_exists($cookieName, $cookies)) { + return; + } + try { - $cookies = $request->getCookieParams(); - $cookieName = $this->autoLogin->getCookieName(); $data = json_decode($cookies[$cookieName], true, 512, JSON_THROW_ON_ERROR); } catch (\Exception $e) { - return null; + $this->logger->warning('Unable to authenticate user by cookie. Invalid cookie.'); + return; } if (!is_array($data) || count($data) !== 2) { - return null; + $this->logger->warning('Unable to authenticate user by cookie. Invalid cookie.'); + return; } [$id, $authKey] = $data; $identity = $this->identityRepository->findIdentity($id); + if ($identity === null) { - return null; + $this->logger->warning("Unable to authenticate user by cookie. Identity \"$id\" not found."); + return; } if (!$identity instanceof AutoLoginIdentityInterface) { @@ -75,26 +79,11 @@ private function getIdentityFromRequest(ServerRequestInterface $request): ?Ident if (!$identity->validateAuthKey($authKey)) { $this->logger->warning('Unable to authenticate user by cookie. Invalid auth key.'); - return null; + return; } - return $identity; - } - - /** - * Authenticate user if there is data to do so in request. - * - * @param ServerRequestInterface $request Request to handle - * @return bool - */ - private function authenticateUserFromRequest(ServerRequestInterface $request): bool - { - $identity = $this->getIdentityFromRequest($request); - - if ($identity === null) { - return false; + if ($identity !== null) { + $this->user->login($identity); } - - return $this->user->login($identity); } } diff --git a/tests/User/AutoLoginIdentity.php b/tests/User/AutoLoginIdentity.php new file mode 100644 index 000000000..1f90bc7ea --- /dev/null +++ b/tests/User/AutoLoginIdentity.php @@ -0,0 +1,29 @@ +getAuthKey(); + } + + public function getId(): ?string + { + return self::ID; + } +} diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index 62d96f789..709aa65e0 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -2,159 +2,253 @@ namespace Yiisoft\Yii\Web\Tests\User; -use Nyholm\Psr7\Response; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; -use Psr\Log\LogLevel; -use Yiisoft\Log\Logger; use Yiisoft\Auth\IdentityInterface; use Yiisoft\Auth\IdentityRepositoryInterface; +use Yiisoft\Log\Logger; +use Yiisoft\Yii\Web\User\AutoLogin; use Yiisoft\Yii\Web\User\User; use Yiisoft\Yii\Web\User\AutoLoginMiddleware; -class AutoLoginMiddlewareTest extends TestCase +final class AutoLoginMiddlewareTest extends TestCase { - /** - * @var RequestHandlerInterface - */ - private $requestHandlerMock; + private Logger $logger; - /** - * @var ServerRequestInterface - */ - private $requestMock; + protected function setUp(): void + { + $this->logger = $this->getMockBuilder(Logger::class) + ->onlyMethods(['dispatch']) + ->getMock(); + } - /** - * @var AutoLoginMiddleware - */ - private $autoLoginMiddlewareMock; + private function getLastLogMessage(): ?string + { + $messages = $this->getInaccessibleProperty($this->logger, 'messages'); + return $messages[0][1] ?? null; + } - /** - * @var IdentityRepositoryInterface - */ - private $identityRepositoryInterfaceMock; + public function testCorrectLogin(): void + { + $user = $this->getUserWithLoginExpected(); - /** - * @var IdentityInterface - */ - private $identityInterfaceMock; + $autoLogin = $this->getAutoLogin(); + $middleware = new AutoLoginMiddleware( + $user, + $this->getAutoLoginIdentityRepository(), + $this->logger, + $autoLogin + ); + $request = $this->getRequestWithAutoLoginCookie(AutoLoginIdentity::ID, AutoLoginIdentity::AUTH_KEY_CORRECT); - /** - * @var Logger - */ - private $loggerMock; + $middleware->process($request, $this->getRequestHandler()); - /** - * @var User - */ - private $userMock; + $this->assertNull($this->getLastLogMessage()); + } - public function testProcessOK(): void + public function testInvalidAuthKey(): void { - $this->mockDataRequest(); - $this->mockDataCookie(["remember" => json_encode(['1', 'ABCD1234', 60])]); + $user = $this->getUserWithoutLoginExpected(); - $this->userMock - ->expects($this->once()) - ->method('validateAuthKey') - ->willReturn(true); + $autoLogin = $this->getAutoLogin(); + $middleware = new AutoLoginMiddleware( + $user, + $this->getAutoLoginIdentityRepository(), + $this->logger, + $autoLogin + ); + $request = $this->getRequestWithAutoLoginCookie(AutoLoginIdentity::ID, AutoLoginIdentity::AUTH_KEY_INCORRECT); - $this->userMock - ->expects($this->once()) - ->method('login') - ->willReturn(true); + $middleware->process($request, $this->getRequestHandler()); - $response = new Response(); - $this->requestHandlerMock - ->expects($this->once()) - ->method('handle') - ->willReturn($response); + $this->assertSame('Unable to authenticate user by cookie. Invalid auth key.', $this->getLastLogMessage()); + } + + public function testNoCookie(): void + { + $user = $this->getUserWithoutLoginExpected(); + + $autoLogin = $this->getAutoLogin(); + $middleware = new AutoLoginMiddleware( + $user, + $this->getAutoLoginIdentityRepository(), + $this->logger, + $autoLogin + ); + $request = $this->getRequestWithCookies([]); + + $middleware->process($request, $this->getRequestHandler()); - $this->assertEquals($this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock), $response); + $this->assertNull($this->getLastLogMessage()); } - public function testProcessErrorLogin(): void + public function testEmptyCookie(): void { - $this->mockDataRequest(); - $this->mockDataCookie(["remember" => json_encode(['1', 'ABCD1234', 60])]); + $user = $this->getUserWithoutLoginExpected(); - $this->userMock - ->expects($this->once()) - ->method('validateAuthKey') - ->willReturn(true); + $autoLogin = $this->getAutoLogin(); + $middleware = new AutoLoginMiddleware( + $user, + $this->getAutoLoginIdentityRepository(), + $this->logger, + $autoLogin + ); + $request = $this->getRequestWithCookies(['autoLogin' => '']); - $this->userMock - ->expects($this->once()) - ->method('login') - ->willReturn(false); + $middleware->process($request, $this->getRequestHandler()); - $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); + $this->assertSame('Unable to authenticate user by cookie. Invalid cookie.', $this->getLastLogMessage()); + } - $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); - $this->assertEquals($messages[0][1], 'Unable to authenticate user by cookie.'); + public function testInvalidCookie(): void + { + $user = $this->getUserWithoutLoginExpected(); + + $autoLogin = $this->getAutoLogin(); + $middleware = new AutoLoginMiddleware( + $user, + $this->getAutoLoginIdentityRepository(), + $this->logger, + $autoLogin + ); + $request = $this->getRequestWithCookies( + [ + 'autoLogin' => json_encode([AutoLoginIdentity::ID, AutoLoginIdentity::AUTH_KEY_CORRECT, 'weird stuff']) + ] + ); + + $middleware->process($request, $this->getRequestHandler()); + + $this->assertSame('Unable to authenticate user by cookie. Invalid cookie.', $this->getLastLogMessage()); } - public function testProcessInvalidAuthKey(): void + public function testIncorrectIdentity(): void { - $this->mockDataRequest(); - $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60])]); + $user = $this->getUserWithoutLoginExpected(); + + $middleware = new AutoLoginMiddleware( + $user, + $this->getIncorrectIdentityRepository(), + $this->logger, + $this->getAutoLogin() + ); + + $request = $this->getRequestWithAutoLoginCookie(AutoLoginIdentity::ID, AutoLoginIdentity::AUTH_KEY_CORRECT); - $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Identity repository must return an instance of \Yiisoft\Yii\Web\User\AutoLoginIdentityInterface in order for auto-login to function.'); - $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); - $this->assertEquals($messages[0][1], 'Unable to authenticate user by cookie. Invalid auth key.'); + $middleware->process($request, $this->getRequestHandlerThatIsNotCalled()); } - public function testProcessCookieEmpty(): void + public function testIdentityNotFound(): void { - $this->mockDataRequest(); - $this->mockDataCookie([]); + $user = $this->getUserWithoutLoginExpected(); + + $middleware = new AutoLoginMiddleware( + $user, + $this->getEmptyIdentityRepository(), + $this->logger, + $this->getAutoLogin() + ); + + $identityId = AutoLoginIdentity::ID; + $request = $this->getRequestWithAutoLoginCookie($identityId, AutoLoginIdentity::AUTH_KEY_CORRECT); - $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); + $middleware->process($request, $this->getRequestHandler()); - $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); - $this->assertEquals($messages[0][1], 'Unable to authenticate user by cookie.'); + $this->assertSame("Unable to authenticate user by cookie. Identity \"$identityId\" not found.", $this->getLastLogMessage()); } - public function testProcessCookieWithInvalidParams(): void + private function getRequestHandler(): RequestHandlerInterface { - $this->mockDataRequest(); - $this->mockDataCookie(["remember" => json_encode(['1', '123456', 60, "paramInvalid"])]); + $requestHandler = $this->createMock(RequestHandlerInterface::class); - $this->autoLoginMiddlewareMock->process($this->requestMock, $this->requestHandlerMock); + $requestHandler + ->expects($this->once()) + ->method('handle'); - $messages = $this->getInaccessibleProperty($this->loggerMock, 'messages'); - $this->assertEquals($messages[0][1], 'Unable to authenticate user by cookie.'); + return $requestHandler; } - private function mockDataRequest(): void + private function getRequestHandlerThatIsNotCalled(): RequestHandlerInterface { - $this->requestHandlerMock = $this->createMock(RequestHandlerInterface::class); - $this->userMock = $this->createMock(User::class); - $this->identityInterfaceMock = $this->createMock(IdentityInterface::class); + $requestHandler = $this->createMock(RequestHandlerInterface::class); - $this->loggerMock = $this->getMockBuilder(Logger::class) - ->onlyMethods(['dispatch']) - ->getMock(); + $requestHandler + ->expects($this->never()) + ->method('handle'); + + return $requestHandler; + } - $this->identityRepositoryInterfaceMock = $this->createMock(IdentityRepositoryInterface::class); + private function getIncorrectIdentityRepository(): IdentityRepositoryInterface + { + return $this->getIdentityRepository($this->createMock(IdentityInterface::class)); + } + + private function getAutoLoginIdentityRepository(): IdentityRepositoryInterface + { + return $this->getIdentityRepository(new AutoLoginIdentity()); + } + + private function getEmptyIdentityRepository(): IdentityRepositoryInterface + { + return $this->createMock(IdentityRepositoryInterface::class); + } + + private function getIdentityRepository(IdentityInterface $identity): IdentityRepositoryInterface + { - $this->identityRepositoryInterfaceMock + $identityRepository = $this->createMock(IdentityRepositoryInterface::class); + + $identityRepository ->expects($this->any()) ->method('findIdentity') - ->willReturn($this->identityInterfaceMock); + ->willReturn($identity); - $this->autoLoginMiddlewareMock = new AutoLoginMiddleware($this->userMock, $this->identityRepositoryInterfaceMock, $this->loggerMock); - $this->requestMock = $this->createMock(ServerRequestInterface::class); + return $identityRepository; } - private function mockDataCookie(array $cookie): void + private function getRequestWithAutoLoginCookie(string $userId, string $authKey): ServerRequestInterface { - $this->requestMock + return $this->getRequestWithCookies(['autoLogin' => json_encode([$userId, $authKey])]); + } + + private function getRequestWithCookies(array $cookies): ServerRequestInterface + { + $request = $this->createMock(ServerRequestInterface::class); + + $request ->expects($this->any()) ->method('getCookieParams') - ->willReturn($cookie); + ->willReturn($cookies); + + return $request; + } + + private function getUserWithoutLoginExpected(): User + { + $user = $this->createMock(User::class); + $user->expects($this->never())->method('login'); + return $user; + } + + private function getUserWithLoginExpected(): User + { + $user = $this->createMock(User::class); + $user + ->expects($this->once()) + ->method('login') + ->willReturn(true); + + return $user; + } + + private function getAutoLogin(): AutoLogin + { + return new AutoLogin(); } /** @@ -165,7 +259,7 @@ private function mockDataCookie(array $cookie): void * @return mixed * @throws \ReflectionException */ - protected function getInaccessibleProperty($object, $propertyName, bool $revoke = true) + private function getInaccessibleProperty($object, $propertyName, bool $revoke = true) { $class = new \ReflectionClass($object); while (!$class->hasProperty($propertyName)) { From 9f1dc6ae1332caf7de299cfaafe8fc189609a1a5 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Wed, 6 May 2020 08:49:14 +0300 Subject: [PATCH 24/35] Use better names --- src/User/AutoLogin.php | 2 +- src/User/AutoLoginIdentityInterface.php | 16 ++++++++-------- src/User/AutoLoginMiddleware.php | 6 +++--- tests/User/AutoLoginIdentity.php | 12 ++++++------ tests/User/AutoLoginMiddlewareTest.php | 14 +++++++------- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/User/AutoLogin.php b/src/User/AutoLogin.php index 97e49c565..671665390 100644 --- a/src/User/AutoLogin.php +++ b/src/User/AutoLogin.php @@ -39,7 +39,7 @@ public function addCookie(AutoLoginIdentityInterface $identity, int $duration, R { $data = json_encode([ $identity->getId(), - $identity->getAuthKey() + $identity->getAutoLoginKey() ], JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); $expireDateTime = new \DateTimeImmutable(); diff --git a/src/User/AutoLoginIdentityInterface.php b/src/User/AutoLoginIdentityInterface.php index be0e593c5..f41ddfaa4 100644 --- a/src/User/AutoLoginIdentityInterface.php +++ b/src/User/AutoLoginIdentityInterface.php @@ -23,20 +23,20 @@ interface AutoLoginIdentityInterface extends IdentityInterface * The returned key will be stored on the client side as part of a cookie and will be used to authenticate user even * if PHP session has been expired. * - * Make sure to invalidate earlier issued authKeys when you implement force user logout, password change and + * Make sure to invalidate earlier issued keys when you implement force user logout, password change and * other scenarios, that require forceful access revocation for old sessions. * * @return string a key that is used to check the validity of a given identity ID. - * @see validateAuthKey() + * @see validateAutoLoginKey() */ - public function getAuthKey(): string; + public function getAutoLoginKey(): string; /** - * Validates the given auth key. + * Validates the given key. * - * @param string $authKey the given auth key - * @return bool whether the given auth key is valid. - * @see getAuthKey() + * @param string $key the given key + * @return bool whether the given key is valid. + * @see getAutoLoginKey() */ - public function validateAuthKey(string $authKey): bool; + public function validateAutoLoginKey(string $key): bool; } diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index 199c009b8..f9dd02157 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -65,7 +65,7 @@ private function authenticateUserByCookieFromRequest(ServerRequestInterface $req return; } - [$id, $authKey] = $data; + [$id, $key] = $data; $identity = $this->identityRepository->findIdentity($id); if ($identity === null) { @@ -77,8 +77,8 @@ private function authenticateUserByCookieFromRequest(ServerRequestInterface $req throw new \RuntimeException('Identity repository must return an instance of \Yiisoft\Yii\Web\User\AutoLoginIdentityInterface in order for auto-login to function.'); } - if (!$identity->validateAuthKey($authKey)) { - $this->logger->warning('Unable to authenticate user by cookie. Invalid auth key.'); + if (!$identity->validateAutoLoginKey($key)) { + $this->logger->warning('Unable to authenticate user by cookie. Invalid key.'); return; } diff --git a/tests/User/AutoLoginIdentity.php b/tests/User/AutoLoginIdentity.php index 1f90bc7ea..6fb8c88a0 100644 --- a/tests/User/AutoLoginIdentity.php +++ b/tests/User/AutoLoginIdentity.php @@ -9,17 +9,17 @@ final class AutoLoginIdentity implements AutoLoginIdentityInterface { public const ID = '42'; - public const AUTH_KEY_CORRECT = 'auth-key-correct'; - public const AUTH_KEY_INCORRECT = 'auth-key-incorrect'; + public const KEY_CORRECT = 'auto-login-key-correct'; + public const KEY_INCORRECT = 'auto-login-key-incorrect'; - public function getAuthKey(): string + public function getAutoLoginKey(): string { - return self::AUTH_KEY_CORRECT; + return self::KEY_CORRECT; } - public function validateAuthKey(string $authKey): bool + public function validateAutoLoginKey(string $key): bool { - return $authKey === $this->getAuthKey(); + return $key === $this->getAutoLoginKey(); } public function getId(): ?string diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index 709aa65e0..e0466408f 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -40,14 +40,14 @@ public function testCorrectLogin(): void $this->logger, $autoLogin ); - $request = $this->getRequestWithAutoLoginCookie(AutoLoginIdentity::ID, AutoLoginIdentity::AUTH_KEY_CORRECT); + $request = $this->getRequestWithAutoLoginCookie(AutoLoginIdentity::ID, AutoLoginIdentity::KEY_CORRECT); $middleware->process($request, $this->getRequestHandler()); $this->assertNull($this->getLastLogMessage()); } - public function testInvalidAuthKey(): void + public function testInvalidKey(): void { $user = $this->getUserWithoutLoginExpected(); @@ -58,11 +58,11 @@ public function testInvalidAuthKey(): void $this->logger, $autoLogin ); - $request = $this->getRequestWithAutoLoginCookie(AutoLoginIdentity::ID, AutoLoginIdentity::AUTH_KEY_INCORRECT); + $request = $this->getRequestWithAutoLoginCookie(AutoLoginIdentity::ID, AutoLoginIdentity::KEY_INCORRECT); $middleware->process($request, $this->getRequestHandler()); - $this->assertSame('Unable to authenticate user by cookie. Invalid auth key.', $this->getLastLogMessage()); + $this->assertSame('Unable to authenticate user by cookie. Invalid key.', $this->getLastLogMessage()); } public function testNoCookie(): void @@ -114,7 +114,7 @@ public function testInvalidCookie(): void ); $request = $this->getRequestWithCookies( [ - 'autoLogin' => json_encode([AutoLoginIdentity::ID, AutoLoginIdentity::AUTH_KEY_CORRECT, 'weird stuff']) + 'autoLogin' => json_encode([AutoLoginIdentity::ID, AutoLoginIdentity::KEY_CORRECT, 'weird stuff']) ] ); @@ -134,7 +134,7 @@ public function testIncorrectIdentity(): void $this->getAutoLogin() ); - $request = $this->getRequestWithAutoLoginCookie(AutoLoginIdentity::ID, AutoLoginIdentity::AUTH_KEY_CORRECT); + $request = $this->getRequestWithAutoLoginCookie(AutoLoginIdentity::ID, AutoLoginIdentity::KEY_CORRECT); $this->expectException(\RuntimeException::class); $this->expectExceptionMessage('Identity repository must return an instance of \Yiisoft\Yii\Web\User\AutoLoginIdentityInterface in order for auto-login to function.'); @@ -154,7 +154,7 @@ public function testIdentityNotFound(): void ); $identityId = AutoLoginIdentity::ID; - $request = $this->getRequestWithAutoLoginCookie($identityId, AutoLoginIdentity::AUTH_KEY_CORRECT); + $request = $this->getRequestWithAutoLoginCookie($identityId, AutoLoginIdentity::KEY_CORRECT); $middleware->process($request, $this->getRequestHandler()); From d3b128f1adb4fe0a120bfa57952b22ed7641642d Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Wed, 6 May 2020 08:52:07 +0300 Subject: [PATCH 25/35] Fixes --- src/User/AutoLogin.php | 3 +-- src/User/AutoLoginMiddleware.php | 4 +--- tests/User/AutoLoginIdentity.php | 1 - tests/User/AutoLoginMiddlewareTest.php | 1 - 4 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/User/AutoLogin.php b/src/User/AutoLogin.php index 671665390..d6a4d1bfb 100644 --- a/src/User/AutoLogin.php +++ b/src/User/AutoLogin.php @@ -3,7 +3,6 @@ namespace Yiisoft\Yii\Web\User; - use Psr\Http\Message\ResponseInterface; use Yiisoft\Yii\Web\Cookie; @@ -17,7 +16,7 @@ class AutoLogin { private string $cookieName = 'autoLogin'; - public function cookieName(string $name): self + public function withCookieName(string $name): self { $new = clone $this; $new->cookieName = $name; diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index f9dd02157..fd5e697b5 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -82,8 +82,6 @@ private function authenticateUserByCookieFromRequest(ServerRequestInterface $req return; } - if ($identity !== null) { - $this->user->login($identity); - } + $this->user->login($identity); } } diff --git a/tests/User/AutoLoginIdentity.php b/tests/User/AutoLoginIdentity.php index 6fb8c88a0..1f101a7e1 100644 --- a/tests/User/AutoLoginIdentity.php +++ b/tests/User/AutoLoginIdentity.php @@ -3,7 +3,6 @@ namespace Yiisoft\Yii\Web\Tests\User; - use Yiisoft\Yii\Web\User\AutoLoginIdentityInterface; final class AutoLoginIdentity implements AutoLoginIdentityInterface diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index e0466408f..8d172ec0d 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -200,7 +200,6 @@ private function getEmptyIdentityRepository(): IdentityRepositoryInterface private function getIdentityRepository(IdentityInterface $identity): IdentityRepositoryInterface { - $identityRepository = $this->createMock(IdentityRepositoryInterface::class); $identityRepository From 9386ec54255db4d5dbccfffd92899a2d7af705d2 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Wed, 6 May 2020 11:30:21 +0300 Subject: [PATCH 26/35] Make duration required argument of AutoLogin --- src/User/AutoLogin.php | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/User/AutoLogin.php b/src/User/AutoLogin.php index d6a4d1bfb..1223cd593 100644 --- a/src/User/AutoLogin.php +++ b/src/User/AutoLogin.php @@ -3,6 +3,7 @@ namespace Yiisoft\Yii\Web\User; +use DateInterval; use Psr\Http\Message\ResponseInterface; use Yiisoft\Yii\Web\Cookie; @@ -15,6 +16,12 @@ class AutoLogin { private string $cookieName = 'autoLogin'; + private DateInterval $duration; + + public function __construct(DateInterval $duration) + { + $this->duration = $duration; + } public function withCookieName(string $name): self { @@ -27,33 +34,29 @@ public function withCookieName(string $name): self * Add auto-login cookie to response so the user is logged in automatically based on cookie even if session * is expired. * - * TODO: do it on event? - * TODO: make duration a property of the service? - * * @param AutoLoginIdentityInterface $identity - * @param int $duration number of seconds that the user can remain in logged-in status. * @param ResponseInterface $response Response to handle */ - public function addCookie(AutoLoginIdentityInterface $identity, int $duration, ResponseInterface $response): void + public function addCookie(AutoLoginIdentityInterface $identity, ResponseInterface $response): void { $data = json_encode([ $identity->getId(), $identity->getAutoLoginKey() ], JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); - $expireDateTime = new \DateTimeImmutable(); - $expireDateTime->setTimestamp(time() + $duration); - $cookie = (new Cookie($this->cookieName, $data))->withExpires($expireDateTime); - $cookie->addToResponse($response); + (new Cookie($this->cookieName, $data)) + ->withMaxAge($this->duration) + ->addToResponse($response); } /** * Expire auto-login cookie so user is not logged in automatically anymore. - * TODO: trigger on logout? */ public function expireCookie(ResponseInterface $response): void { - (new Cookie($this->cookieName))->expire()->addToResponse($response); + (new Cookie($this->cookieName)) + ->expire() + ->addToResponse($response); } public function getCookieName(): string From ecb4587ed3960128ddd4a1f21204cb04863485f4 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Wed, 6 May 2020 11:31:00 +0300 Subject: [PATCH 27/35] Automatically deal with Cookie in AutoLoginMiddleware --- src/User/AutoLoginMiddleware.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/User/AutoLoginMiddleware.php b/src/User/AutoLoginMiddleware.php index fd5e697b5..58b874f3c 100644 --- a/src/User/AutoLoginMiddleware.php +++ b/src/User/AutoLoginMiddleware.php @@ -36,7 +36,19 @@ public function __construct( public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { $this->authenticateUserByCookieFromRequest($request); - return $handler->handle($request); + $guestBeforeHandle = $this->user->isGuest(); + $response = $handler->handle($request); + $guestAfterHandle = $this->user->isGuest(); + + if ($guestBeforeHandle && !$guestAfterHandle) { + $this->autoLogin->addCookie($this->user->getIdentity(false), $response); + } + + if (!$guestBeforeHandle && $guestAfterHandle) { + $this->autoLogin->expireCookie($response); + } + + return $response; } /** From 6dfc9809f966ce63ae7f4246129831f4a1662d86 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Wed, 6 May 2020 11:33:04 +0300 Subject: [PATCH 28/35] Fix tests --- tests/User/AutoLoginMiddlewareTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index 8d172ec0d..faaca94d1 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -247,7 +247,7 @@ private function getUserWithLoginExpected(): User private function getAutoLogin(): AutoLogin { - return new AutoLogin(); + return new AutoLogin(new \DateInterval('P1W')); } /** From af47ab71bdd7a70ac649911b1a80007ba47717e9 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Wed, 6 May 2020 22:12:31 +0300 Subject: [PATCH 29/35] Add AutoLogin tests, fix code --- src/User/AutoLogin.php | 11 +++---- src/User/User.php | 1 - tests/User/AutoLoginTest.php | 60 ++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 tests/User/AutoLoginTest.php diff --git a/src/User/AutoLogin.php b/src/User/AutoLogin.php index 1223cd593..f824f31e8 100644 --- a/src/User/AutoLogin.php +++ b/src/User/AutoLogin.php @@ -33,18 +33,15 @@ public function withCookieName(string $name): self /** * Add auto-login cookie to response so the user is logged in automatically based on cookie even if session * is expired. - * - * @param AutoLoginIdentityInterface $identity - * @param ResponseInterface $response Response to handle */ - public function addCookie(AutoLoginIdentityInterface $identity, ResponseInterface $response): void + public function addCookie(AutoLoginIdentityInterface $identity, ResponseInterface $response): ResponseInterface { $data = json_encode([ $identity->getId(), $identity->getAutoLoginKey() ], JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); - (new Cookie($this->cookieName, $data)) + return (new Cookie($this->cookieName, urlencode($data))) ->withMaxAge($this->duration) ->addToResponse($response); } @@ -52,9 +49,9 @@ public function addCookie(AutoLoginIdentityInterface $identity, ResponseInterfac /** * Expire auto-login cookie so user is not logged in automatically anymore. */ - public function expireCookie(ResponseInterface $response): void + public function expireCookie(ResponseInterface $response): ResponseInterface { - (new Cookie($this->cookieName)) + return (new Cookie($this->cookieName)) ->expire() ->addToResponse($response); } diff --git a/src/User/User.php b/src/User/User.php index c32c0ebb0..ce6c83a6b 100644 --- a/src/User/User.php +++ b/src/User/User.php @@ -12,7 +12,6 @@ use Yiisoft\Yii\Web\User\Event\BeforeLogin; use Yiisoft\Yii\Web\User\Event\BeforeLogout; -// TODO: declare it final? class User { private const SESSION_AUTH_ID = '__auth_id'; diff --git a/tests/User/AutoLoginTest.php b/tests/User/AutoLoginTest.php new file mode 100644 index 000000000..f4dd4c1b3 --- /dev/null +++ b/tests/User/AutoLoginTest.php @@ -0,0 +1,60 @@ +addCookie($identity, $response); + + $this->assertMatchesRegularExpression('#autoLogin=%5B%2242%22%2C%22auto-login-key-correct%22%5D; Expires=.*?; Max-Age=604800; Path=/; Secure; HttpOnly; SameSite=Lax#', $response->getHeaderLine('Set-Cookie')); + } + + public function testRemoveCookie(): void + { + $autoLogin = new AutoLogin(new \DateInterval('P1W')); + + $response = new Response(); + $response = $autoLogin->expireCookie($response); + + $this->assertMatchesRegularExpression('#autoLogin=; Expires=.*?; Max-Age=-31622400; Path=/; Secure; HttpOnly; SameSite=Lax#', $response->getHeaderLine('Set-Cookie')); + } + + public function testAddCookieWithCustomName(): void + { + $cookieName = 'testName'; + $autoLogin = (new AutoLogin(new \DateInterval('P1W'))) + ->withCookieName($cookieName); + + $identity = new AutoLoginIdentity(); + + $response = new Response(); + $response = $autoLogin->addCookie($identity, $response); + + $this->assertMatchesRegularExpression('#' . $cookieName . '=%5B%2242%22%2C%22auto-login-key-correct%22%5D; Expires=.*?; Max-Age=604800; Path=/; Secure; HttpOnly; SameSite=Lax#', $response->getHeaderLine('Set-Cookie')); + } + + public function testRemoveCookieWithCustomName(): void + { + $cookieName = 'testName'; + $autoLogin = (new AutoLogin(new \DateInterval('P1W'))) + ->withCookieName($cookieName); + + $response = new Response(); + $response = $autoLogin->expireCookie($response); + + $this->assertMatchesRegularExpression('#' . $cookieName . '=; Expires=.*?; Max-Age=-31622400; Path=/; Secure; HttpOnly; SameSite=Lax#', $response->getHeaderLine('Set-Cookie')); + } +} From 53c0d0329afbcb1e93ab5e85bef25a168a72cb06 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Wed, 6 May 2020 22:14:47 +0300 Subject: [PATCH 30/35] Fix code style --- tests/User/AutoLoginTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/User/AutoLoginTest.php b/tests/User/AutoLoginTest.php index f4dd4c1b3..e9c50deca 100644 --- a/tests/User/AutoLoginTest.php +++ b/tests/User/AutoLoginTest.php @@ -3,7 +3,6 @@ namespace Yiisoft\Yii\Web\Tests\User; - use Nyholm\Psr7\Response; use PHPUnit\Framework\TestCase; use Yiisoft\Yii\Web\User\AutoLogin; From d70bde17a49fb3df91044141c000315908565e31 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Wed, 6 May 2020 22:17:26 +0300 Subject: [PATCH 31/35] Add strict types --- src/User/AutoLogin.php | 2 +- src/User/AutoLoginIdentityInterface.php | 1 + src/User/AutoLoginMiddleware.php | 1 - src/User/User.php | 1 + tests/User/AutoLoginIdentity.php | 2 +- tests/User/AutoLoginMiddlewareTest.php | 1 + tests/User/AutoLoginTest.php | 2 +- 7 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/User/AutoLogin.php b/src/User/AutoLogin.php index f824f31e8..e4638542a 100644 --- a/src/User/AutoLogin.php +++ b/src/User/AutoLogin.php @@ -1,5 +1,5 @@ Date: Wed, 6 May 2020 22:44:42 +0300 Subject: [PATCH 32/35] Finalize AutoLogin, add test stubs --- src/User/AutoLogin.php | 3 ++- src/User/AutoLoginIdentityInterface.php | 1 + src/User/AutoLoginMiddleware.php | 1 + src/User/GuestIdentity.php | 2 ++ src/User/User.php | 1 + tests/User/AutoLoginIdentity.php | 1 + tests/User/AutoLoginMiddlewareTest.php | 11 +++++++++++ tests/User/AutoLoginTest.php | 1 + 8 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/User/AutoLogin.php b/src/User/AutoLogin.php index e4638542a..63aef7bf6 100644 --- a/src/User/AutoLogin.php +++ b/src/User/AutoLogin.php @@ -1,4 +1,5 @@ assertSame("Unable to authenticate user by cookie. Identity \"$identityId\" not found.", $this->getLastLogMessage()); } + public function testAddCookieAfterLogin(): void + { + + } + + public function testRemoveCookieAfterLogout(): void + { + + } + private function getRequestHandler(): RequestHandlerInterface { $requestHandler = $this->createMock(RequestHandlerInterface::class); diff --git a/tests/User/AutoLoginTest.php b/tests/User/AutoLoginTest.php index 717aaef9f..36e6aa57e 100644 --- a/tests/User/AutoLoginTest.php +++ b/tests/User/AutoLoginTest.php @@ -1,4 +1,5 @@ Date: Wed, 6 May 2020 23:02:02 +0300 Subject: [PATCH 33/35] Remove tests stubs I have no idea on how to implement :( --- tests/User/AutoLoginMiddlewareTest.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/User/AutoLoginMiddlewareTest.php b/tests/User/AutoLoginMiddlewareTest.php index b8797853c..62b6fc355 100644 --- a/tests/User/AutoLoginMiddlewareTest.php +++ b/tests/User/AutoLoginMiddlewareTest.php @@ -163,16 +163,6 @@ public function testIdentityNotFound(): void $this->assertSame("Unable to authenticate user by cookie. Identity \"$identityId\" not found.", $this->getLastLogMessage()); } - public function testAddCookieAfterLogin(): void - { - - } - - public function testRemoveCookieAfterLogout(): void - { - - } - private function getRequestHandler(): RequestHandlerInterface { $requestHandler = $this->createMock(RequestHandlerInterface::class); From 72d71f105f5a0c71d3d96097568d498ccafa23d9 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Wed, 6 May 2020 23:50:41 +0300 Subject: [PATCH 34/35] Do not encode cookie manually --- src/User/AutoLogin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/User/AutoLogin.php b/src/User/AutoLogin.php index 63aef7bf6..4bdbb254b 100644 --- a/src/User/AutoLogin.php +++ b/src/User/AutoLogin.php @@ -42,7 +42,7 @@ public function addCookie(AutoLoginIdentityInterface $identity, ResponseInterfac $identity->getAutoLoginKey() ], JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); - return (new Cookie($this->cookieName, urlencode($data))) + return (new Cookie($this->cookieName, $data)) ->withMaxAge($this->duration) ->addToResponse($response); } From e89d374cf0b9d2dc838ac449f46e9dde510d963a Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Wed, 6 May 2020 23:52:57 +0300 Subject: [PATCH 35/35] Add strict types --- src/User/Event/AfterLogin.php | 2 ++ src/User/Event/AfterLogout.php | 2 ++ src/User/Event/BeforeLogin.php | 2 ++ src/User/Event/BeforeLogout.php | 2 ++ 4 files changed, 8 insertions(+) diff --git a/src/User/Event/AfterLogin.php b/src/User/Event/AfterLogin.php index 90ef57ba3..94b3db3eb 100644 --- a/src/User/Event/AfterLogin.php +++ b/src/User/Event/AfterLogin.php @@ -1,5 +1,7 @@