From bba4ad55da03862467d8a13f2ef3dcd986193b0a Mon Sep 17 00:00:00 2001 From: Yuri Kuznetsov Date: Sun, 29 Aug 2021 16:13:35 +0300 Subject: [PATCH] refactoring --- application/Espo/Controllers/App.php | 5 ++- application/Espo/Core/Api/Auth.php | 16 ++++++- application/Espo/Core/Api/Response.php | 11 +++-- application/Espo/Core/Api/ResponseWrapper.php | 7 +++ .../Core/Authentication/Authentication.php | 45 +++++++++++-------- .../Espo/Core/ORM/EntityManagerProxy.php | 6 +++ tests/integration/Core/Tester.php | 14 +++++- 7 files changed, 75 insertions(+), 29 deletions(-) diff --git a/application/Espo/Controllers/App.php b/application/Espo/Controllers/App.php index f471a85f22..1c258308c5 100644 --- a/application/Espo/Controllers/App.php +++ b/application/Espo/Controllers/App.php @@ -34,6 +34,7 @@ use Espo\Core\Exceptions\BadRequest; use Espo\Core\Authentication\Authentication; use Espo\Core\Di; use Espo\Core\Api\Request; +use Espo\Core\Api\Response; use StdClass; @@ -50,7 +51,7 @@ class App implements return (object) $this->serviceFactory->create('App')->getUserData(); } - public function postActionDestroyAuthToken(Request $request): bool + public function postActionDestroyAuthToken(Request $request, Response $response): bool { $data = $request->getParsedBody(); @@ -60,6 +61,6 @@ class App implements $auth = $this->injectableFactory->create(Authentication::class); - return $auth->destroyAuthToken($data->token, $request); + return $auth->destroyAuthToken($data->token, $request, $response); } } diff --git a/application/Espo/Core/Api/Auth.php b/application/Espo/Core/Api/Auth.php index 9266725163..548c14c365 100644 --- a/application/Espo/Core/Api/Auth.php +++ b/application/Espo/Core/Api/Auth.php @@ -127,7 +127,13 @@ class Auth ): ?AuthResult { try { - $result = $this->authentication->login($username, $password, $request, $authenticationMethod); + $result = $this->authentication->login( + $username, + $password, + $request, + $response, + $authenticationMethod + ); } catch (Exception $e) { $this->handleException($response, $e); @@ -153,7 +159,13 @@ class Auth $showDialog = $this->isEntryPoint; try { - $result = $this->authentication->login($username, $password, $request, $authenticationMethod); + $result = $this->authentication->login( + $username, + $password, + $request, + $response, + $authenticationMethod + ); } catch (Exception $e) { $this->handleException($response, $e); diff --git a/application/Espo/Core/Api/Response.php b/application/Espo/Core/Api/Response.php index d81df875f8..a18369e9e7 100644 --- a/application/Espo/Core/Api/Response.php +++ b/application/Espo/Core/Api/Response.php @@ -29,10 +29,8 @@ namespace Espo\Core\Api; -use Psr\Http\Message\{ - ResponseInterface as Psr7Response, - StreamInterface, -}; +use Psr\Http\Message\ResponseInterface as Psr7Response; +use Psr\Http\Message\StreamInterface; /** * Representation of an HTTP response. An instance is mutable. @@ -49,6 +47,11 @@ interface Response */ public function setHeader(string $name, string $value): self; + /** + * Add a specific header. + */ + public function addHeader(string $name, string $value): self; + /** * Get a header value. */ diff --git a/application/Espo/Core/Api/ResponseWrapper.php b/application/Espo/Core/Api/ResponseWrapper.php index 3c6c730a7e..2a32ac3921 100644 --- a/application/Espo/Core/Api/ResponseWrapper.php +++ b/application/Espo/Core/Api/ResponseWrapper.php @@ -68,6 +68,13 @@ class ResponseWrapper implements ApiResponse return $this; } + public function addHeader(string $name, string $value): Response + { + $this->response = $this->response->withAddedHeader($name, $value); + + return $this; + } + public function getHeader(string $name): ?string { if (!$this->response->hasHeader($name)) { diff --git a/application/Espo/Core/Authentication/Authentication.php b/application/Espo/Core/Authentication/Authentication.php index 8512b4bcce..ed597809ad 100644 --- a/application/Espo/Core/Authentication/Authentication.php +++ b/application/Espo/Core/Authentication/Authentication.php @@ -55,6 +55,7 @@ use Espo\Core\{ ApplicationState, ORM\EntityManagerProxy, Api\Request, + Api\Response, Utils\Log, }; @@ -136,6 +137,7 @@ class Authentication ?string $username, ?string $password, Request $request, + Response $response, ?string $authenticationMethod = null ): Result { @@ -262,8 +264,9 @@ class Authentication if (!$result->isSecondStepRequired() && $request->getHeader('Espo-Authorization')) { if (!$authToken) { - $authToken = $this->createAuthToken($user, $request); - } else { + $authToken = $this->createAuthToken($user, $request, $response); + } + else { $this->authTokenManager->renew($authToken); } @@ -281,7 +284,7 @@ class Authentication if ($authToken && !$authLogRecord && isset($authToken->id)) { $authLogRecord = $this->entityManager - ->getRepository('AuthLogRecord') + ->getRDBRepository('AuthLogRecord') ->select(['id']) ->where([ 'authTokenId' => $authToken->id @@ -291,7 +294,7 @@ class Authentication } if ($authLogRecord) { - $user->set('authLogRecordId', $authLogRecord->id); + $user->set('authLogRecordId', $authLogRecord->getId()); } return $result; @@ -452,14 +455,14 @@ class Authentication ]; $wasFailed = (bool) $this->entityManager - ->getRepository('AuthLogRecord') + ->getRDBRepository('AuthLogRecord') ->select(['id']) ->where($where) ->findOne(); if ($wasFailed) { $failAttemptCount = $this->entityManager - ->getRepository('AuthLogRecord') + ->getRDBRepository('AuthLogRecord') ->where($where) ->count(); } @@ -473,7 +476,7 @@ class Authentication } } - private function createAuthToken(User $user, Request $request): AuthToken + private function createAuthToken(User $user, Request $request, Response $response): AuthToken { $createSecret = $request->getHeader('Espo-Authorization-Create-Token-Secret') === 'true'; @@ -496,7 +499,7 @@ class Authentication ); if ($createSecret) { - $this->setSecretInCookie($authToken->getSecret()); + $this->setSecretInCookie($authToken->getSecret(), $response); } if ( @@ -504,7 +507,7 @@ class Authentication $authToken instanceof AuthTokenEntity ) { $concurrentAuthTokenList = $this->entityManager - ->getRepository('AuthToken') + ->getRDBRepository('AuthToken') ->select(['id']) ->where([ 'userId' => $user->id, @@ -523,7 +526,7 @@ class Authentication return $authToken; } - public function destroyAuthToken(string $token, Request $request): bool + public function destroyAuthToken(string $token, Request $request, Response $response): bool { $authToken = $this->authTokenManager->get($token); @@ -537,7 +540,7 @@ class Authentication $sentSecret = $request->getCookieParam('auth-token-secret'); if ($sentSecret === $authToken->getSecret()) { - $this->setSecretInCookie(null); + $this->setSecretInCookie(null, $response); } } @@ -603,15 +606,19 @@ class Authentication $this->entityManager->saveEntity($authLogRecord); } - private function setSecretInCookie(?string $secret): void + private function setSecretInCookie(?string $secret, Response $response): void { - $time = $secret ? strtotime('+1000 days') : -1; + $time = $secret ? strtotime('+1000 days') : 1; - setcookie('auth-token-secret', $secret, [ - 'expires' => $time, - 'path' => '/', - 'httponly' => true, - 'samesite' => 'Lax', - ]); + $value = $secret ? $secret : 'deleted'; + + $headerValue = + 'auth-token-secret=' . urlencode($value) . + '; path=/' . + '; expires=' . gmdate('D, d M Y H:i:s T', $time) . + '; HttpOnly' . + '; SameSite=Lax'; + + $response->addHeader('Set-Cookie', $headerValue); } } diff --git a/application/Espo/Core/ORM/EntityManagerProxy.php b/application/Espo/Core/ORM/EntityManagerProxy.php index c4b417dd4e..85a7578caa 100644 --- a/application/Espo/Core/ORM/EntityManagerProxy.php +++ b/application/Espo/Core/ORM/EntityManagerProxy.php @@ -32,6 +32,7 @@ namespace Espo\Core\ORM; use Espo\ORM\{ Entity, Repository\Repository, + Repository\RDBRepository, }; use Espo\Core\{ @@ -72,4 +73,9 @@ class EntityManagerProxy { return $this->getEntityManager()->getRepository($entityType); } + + public function getRDBRepository(string $entityType): RDBRepository + { + return $this->getEntityManager()->getRDBRepository($entityType); + } } diff --git a/tests/integration/Core/Tester.php b/tests/integration/Core/Tester.php index 8a8dd069c2..748327fda4 100644 --- a/tests/integration/Core/Tester.php +++ b/tests/integration/Core/Tester.php @@ -34,12 +34,14 @@ use Espo\Core\Authentication\Authentication; use Espo\Core\Application; use Espo\Core\Portal\Application as PortalApplication; use Espo\Core\Api\RequestWrapper; +use Espo\Core\Api\ResponseWrapper; use Espo\Core\ApplicationRunners\Rebuild; use Espo\Core\Utils\PasswordHash; use Espo\Entities\User; use Slim\Psr7\Factory\RequestFactory; +use Slim\Psr7\Response; class Tester { @@ -205,10 +207,18 @@ class Tester (new RequestFactory())->createRequest('POST', '') ); + $response = new ResponseWrapper(new Response()); + if (isset($this->userName) || $this->authenticationMethod) { $this->password = isset($this->password) ? $this->password : $this->defaultUserPassword; - $result = $auth->login($this->userName, $this->password, $request, $this->authenticationMethod); + $auth->login( + $this->userName, + $this->password, + $request, + $response, + $this->authenticationMethod + ); } else { $this->application->setupSystemUser(); @@ -469,7 +479,7 @@ class Tester } $application = $this->getApplication(); - + $entityManager = $application->getContainer()->get('entityManager'); $config = $application->getContainer()->get('config');