From 6cafac1b6514287e2fffed845ab9d2e6b95db72c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=91cze=20Bence?= Date: Sun, 5 Jul 2020 13:32:15 +0200 Subject: [PATCH 1/8] MAPG-142 modify UserConfirmationRepository to return only one confirmation for user (it is not possible for an user to have multiple) --- src/Controller/UserController.php | 3 ++- src/Repository/UserConfirmationRepository.php | 6 ++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Controller/UserController.php b/src/Controller/UserController.php index 83f3c89..18b4dee 100644 --- a/src/Controller/UserController.php +++ b/src/Controller/UserController.php @@ -184,7 +184,8 @@ class UserController implements ISecured \Container::$dbConnection->startTransaction(); - foreach ($this->userConfirmationRepository->getByUser($user) as $userConfirmation) { + $userConfirmation = $this->userConfirmationRepository->getByUser($user); + if ($userConfirmation !== null) { $this->pdm->deleteFromDb($userConfirmation); } diff --git a/src/Repository/UserConfirmationRepository.php b/src/Repository/UserConfirmationRepository.php index 5c0723e..d7d80ad 100644 --- a/src/Repository/UserConfirmationRepository.php +++ b/src/Repository/UserConfirmationRepository.php @@ -1,8 +1,6 @@ pdm->selectFromDb($select, UserConfirmation::class); } - public function getByUser(User $user): Generator + public function getByUser(User $user): ?UserConfirmation { $select = new Select(\Container::$dbConnection); $select->where('user_id', '=', $user->getId()); - yield from $this->pdm->selectMultipleFromDb($select, UserConfirmation::class); + return $this->pdm->selectFromDb($select, UserConfirmation::class); } } From 7e3315fc88e9db18de356eb2590aa914a619c281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=91cze=20Bence?= Date: Sun, 5 Jul 2020 12:22:47 +0200 Subject: [PATCH 2/8] MAPG-142 implemenet confirmation mail resend --- .../20200705_0118_resend_activation.sql | 5 ++++ src/Controller/LoginController.php | 29 +++++++++++++++++-- src/PersistentData/Model/UserConfirmation.php | 26 ++++++++++++++++- 3 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 database/migrations/structure/20200705_0118_resend_activation.sql diff --git a/database/migrations/structure/20200705_0118_resend_activation.sql b/database/migrations/structure/20200705_0118_resend_activation.sql new file mode 100644 index 0000000..76e34d3 --- /dev/null +++ b/database/migrations/structure/20200705_0118_resend_activation.sql @@ -0,0 +1,5 @@ +UPDATE `user_confirmations` SET token=SUBSTRING(token, 1, 32); + +ALTER TABLE `user_confirmations` + ADD `last_sent` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP, + MODIFY `token` varchar(32) CHARACTER SET ascii NOT NULL; diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index ac0a25e..4328ee9 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -1,5 +1,6 @@ getActive()) { + $this->resendConfirmationEmail($user); + return new JsonContent([ 'error' => [ 'errorText' => 'User found with the given email address, but the account is not activated. ' . @@ -306,11 +309,12 @@ class LoginController $this->pdm->saveToDb($user); - $token = hash('sha256', serialize($user) . random_bytes(10) . microtime()); + $token = bin2hex(random_bytes(16)); $confirmation = new UserConfirmation(); $confirmation->setUser($user); $confirmation->setToken($token); + $confirmation->setLastSentDate(new DateTime()); $this->pdm->saveToDb($confirmation); @@ -377,7 +381,7 @@ class LoginController return new Redirect(\Container::$routeCollection->getRoute('index')->generateLink(), IRedirect::TEMPORARY); } - $confirmation = $this->userConfirmationRepository->getByToken($this->request->query('token')); + $confirmation = $this->userConfirmationRepository->getByToken(substr($this->request->query('token'), 0, 32)); if ($confirmation === null) { return new HtmlContent('login/activate'); @@ -405,7 +409,7 @@ class LoginController return new Redirect(\Container::$routeCollection->getRoute('index')->generateLink(), IRedirect::TEMPORARY); } - $confirmation = $this->userConfirmationRepository->getByToken($this->request->query('token')); + $confirmation = $this->userConfirmationRepository->getByToken(substr($this->request->query('token'), 0, 32)); if ($confirmation === null) { return new HtmlContent('login/cancel', ['success' => false]); @@ -445,6 +449,8 @@ class LoginController } if (!$user->getActive()) { + $this->resendConfirmationEmail($user); + return new JsonContent([ 'error' => [ 'errorText' => 'User found with the given email address, but the account is not activated. ' . @@ -533,6 +539,23 @@ class LoginController $mail->send(); } + private function resendConfirmationEmail(User $user): bool + { + $confirmation = $this->userConfirmationRepository->getByUser($user); + + if ($confirmation === null || (clone $confirmation->getLastSentDate())->add(new DateInterval('PT1H')) > new DateTime()) { + return false; + } + + $confirmation->setLastSentDate(new DateTime()); + + $this->pdm->saveToDb($confirmation); + + $this->sendConfirmationEmail($user->getEmail(), $confirmation->getToken()); + + return true; + } + private function sendWelcomeEmail(string $email): void { $mail = new Mail(); diff --git a/src/PersistentData/Model/UserConfirmation.php b/src/PersistentData/Model/UserConfirmation.php index fac2b2d..592fe46 100644 --- a/src/PersistentData/Model/UserConfirmation.php +++ b/src/PersistentData/Model/UserConfirmation.php @@ -1,10 +1,12 @@ User::class]; @@ -14,6 +16,8 @@ class UserConfirmation extends Model private string $token = ''; + private DateTime $lastSent; + public function setUser(User $user): void { $this->user = $user; @@ -29,6 +33,16 @@ class UserConfirmation extends Model $this->token = $token; } + public function setLastSentDate(DateTime $lastSent): void + { + $this->lastSent = $lastSent; + } + + public function setLastSent(string $lastSent): void + { + $this->lastSent = new DateTime($lastSent); + } + public function getUser(): ?User { return $this->user; @@ -43,4 +57,14 @@ class UserConfirmation extends Model { return $this->token; } + + public function getLastSentDate(): DateTime + { + return $this->lastSent; + } + + public function getLastSent(): string + { + return $this->lastSent->format('Y-m-d H:i:s'); + } } From ab23b37b97b1bbea5b23ec458972d482ac6833e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=91cze=20Bence?= Date: Sun, 5 Jul 2020 12:25:25 +0200 Subject: [PATCH 3/8] MAPG-142 redefine tokens and increase OAuth security with nonce --- .../20200705_0219_redefine_tokens.sql | 4 ++++ main.php | 2 +- src/Controller/LoginController.php | 17 +++++++++---- src/Controller/UserController.php | 11 +++++++-- src/Database/Query/Modify.php | 2 +- src/Interfaces/Session/ISessionHandler.php | 9 +++++++ src/OAuth/GoogleOAuth.php | 7 ++++-- src/Session/DatabaseSessionHandler.php | 24 +++++++++---------- tests/OAuth/GoogleOAuthTest.php | 12 ++++------ web.php | 7 +++++- 10 files changed, 63 insertions(+), 32 deletions(-) create mode 100644 database/migrations/structure/20200705_0219_redefine_tokens.sql create mode 100644 src/Interfaces/Session/ISessionHandler.php diff --git a/database/migrations/structure/20200705_0219_redefine_tokens.sql b/database/migrations/structure/20200705_0219_redefine_tokens.sql new file mode 100644 index 0000000..4f78b83 --- /dev/null +++ b/database/migrations/structure/20200705_0219_redefine_tokens.sql @@ -0,0 +1,4 @@ +UPDATE `sessions` SET id=SUBSTRING(id, 1, 32); + +ALTER TABLE `sessions` + MODIFY `id` varchar(32) CHARACTER SET ascii NOT NULL; diff --git a/main.php b/main.php index 401e86f..fbf78d9 100644 --- a/main.php +++ b/main.php @@ -16,7 +16,7 @@ class Container { static MapGuesser\Interfaces\Database\IConnection $dbConnection; static MapGuesser\Routing\RouteCollection $routeCollection; - static \SessionHandlerInterface $sessionHandler; + static MapGuesser\Interfaces\Session\ISessionHandler $sessionHandler; static MapGuesser\Interfaces\Request\IRequest $request; } diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index 4328ee9..86a73aa 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -53,13 +53,16 @@ class LoginController public function getGoogleLoginRedirect(): IRedirect { $state = bin2hex(random_bytes(16)); + $nonce = bin2hex(random_bytes(16)); $this->request->session()->set('oauth_state', $state); + $this->request->session()->set('oauth_nonce', $nonce); $oAuth = new GoogleOAuth(new Request()); $url = $oAuth->getDialogUrl( $state, - $this->request->getBase() . '/' . \Container::$routeCollection->getRoute('login-google-action')->generateLink() + $this->request->getBase() . '/' . \Container::$routeCollection->getRoute('login-google-action')->generateLink(), + $nonce ); return new Redirect($url, IRedirect::TEMPORARY); @@ -214,16 +217,20 @@ class LoginController } $jwtParser = new JwtParser($tokenData['id_token']); - $userData = $jwtParser->getPayload(); + $idToken = $jwtParser->getPayload(); - if (!$userData['email_verified']) { + if ($idToken['nonce'] !== $this->request->session()->get('oauth_nonce')) { return new HtmlContent('login/google_login'); } - $user = $this->userRepository->getByGoogleSub($userData['sub']); + if (!$idToken['email_verified']) { + return new HtmlContent('login/google_login'); + } + + $user = $this->userRepository->getByGoogleSub($idToken['sub']); if ($user === null) { - $this->request->session()->set('google_user_data', $userData); + $this->request->session()->set('google_user_data', ['sub' => $idToken['sub'], 'email' => $idToken['email']]); return new Redirect(\Container::$routeCollection->getRoute('signup-google')->generateLink(), IRedirect::TEMPORARY); } diff --git a/src/Controller/UserController.php b/src/Controller/UserController.php index 18b4dee..f285318 100644 --- a/src/Controller/UserController.php +++ b/src/Controller/UserController.php @@ -59,14 +59,17 @@ class UserController implements ISecured $user = $this->request->user(); $state = bin2hex(random_bytes(16)); + $nonce = bin2hex(random_bytes(16)); $this->request->session()->set('oauth_state', $state); + $this->request->session()->set('oauth_nonce', $nonce); $oAuth = new GoogleOAuth(new Request()); $url = $oAuth->getDialogUrl( $state, $this->request->getBase() . '/' . \Container::$routeCollection->getRoute('account.googleAuthenticate-action')->generateLink(), + $nonce, $user->getEmail() ); @@ -95,9 +98,13 @@ class UserController implements ISecured } $jwtParser = new JwtParser($tokenData['id_token']); - $userData = $jwtParser->getPayload(); + $idToken = $jwtParser->getPayload(); - if ($userData['sub'] !== $user->getGoogleSub()) { + if ($idToken['nonce'] !== $this->request->session()->get('oauth_nonce')) { + return new HtmlContent('account/google_authenticate', ['success' => false]); + } + + if ($idToken['sub'] !== $user->getGoogleSub()) { return new HtmlContent('account/google_authenticate', [ 'success' => false, 'errorText' => 'This Google account is not linked to your account.' diff --git a/src/Database/Query/Modify.php b/src/Database/Query/Modify.php index b0a6f82..321c78c 100755 --- a/src/Database/Query/Modify.php +++ b/src/Database/Query/Modify.php @@ -135,6 +135,6 @@ class Modify private function generateKey(): string { - return substr(hash('sha256', serialize($this->attributes) . random_bytes(10) . microtime()), 0, 7); + return substr(hash('sha256', serialize($this->attributes) . random_bytes(5) . microtime()), 0, 7); } } diff --git a/src/Interfaces/Session/ISessionHandler.php b/src/Interfaces/Session/ISessionHandler.php new file mode 100644 index 0000000..dbee79e --- /dev/null +++ b/src/Interfaces/Session/ISessionHandler.php @@ -0,0 +1,9 @@ +request = $request; } - public function getDialogUrl(string $state, string $redirectUrl, ?string $loginHint = null): string + public function getDialogUrl(string $state, string $redirectUrl, ?string $nonce = null, ?string $loginHint = null): string { $oauthParams = [ 'response_type' => 'code', @@ -23,9 +23,12 @@ class GoogleOAuth 'scope' => 'openid email', 'redirect_uri' => $redirectUrl, 'state' => $state, - 'nonce' => hash('sha256', random_bytes(10) . microtime()), ]; + if ($nonce !== null) { + $oauthParams['nonce'] = $nonce; + } + if ($loginHint !== null) { $oauthParams['login_hint'] = $loginHint; } diff --git a/src/Session/DatabaseSessionHandler.php b/src/Session/DatabaseSessionHandler.php index a50a668..49f7f6f 100644 --- a/src/Session/DatabaseSessionHandler.php +++ b/src/Session/DatabaseSessionHandler.php @@ -4,11 +4,9 @@ use DateTime; use MapGuesser\Database\Query\Modify; use MapGuesser\Database\Query\Select; use MapGuesser\Interfaces\Database\IResultSet; -use SessionHandlerInterface; -use SessionIdInterface; -use SessionUpdateTimestampHandlerInterface; +use MapGuesser\Interfaces\Session\ISessionHandler; -class DatabaseSessionHandler implements SessionHandlerInterface, SessionIdInterface, SessionUpdateTimestampHandlerInterface +class DatabaseSessionHandler implements ISessionHandler { private bool $exists = false; @@ -28,7 +26,7 @@ class DatabaseSessionHandler implements SessionHandlerInterface, SessionIdInterf { $select = new Select(\Container::$dbConnection, 'sessions'); $select->columns(['data']); - $select->whereId($id); + $select->whereId(substr($id, 0, 32)); $result = $select->execute()->fetch(IResultSet::FETCH_ASSOC); @@ -46,16 +44,16 @@ class DatabaseSessionHandler implements SessionHandlerInterface, SessionIdInterf $modify = new Modify(\Container::$dbConnection, 'sessions'); if ($this->exists) { - $modify->setId($id); + $modify->setId(substr($id, 0, 32)); } else { - $modify->setExternalId($id); + $modify->setExternalId(substr($id, 0, 32)); } $modify->set('data', $data); $modify->set('updated', (new DateTime())->format('Y-m-d H:i:s')); $modify->save(); - $written = true; + $this->written = true; return true; } @@ -63,9 +61,11 @@ class DatabaseSessionHandler implements SessionHandlerInterface, SessionIdInterf public function destroy($id): bool { $modify = new Modify(\Container::$dbConnection, 'sessions'); - $modify->setId($id); + $modify->setId(substr($id, 0, 32)); $modify->delete(); + $this->exists = false; + return true; } @@ -88,12 +88,12 @@ class DatabaseSessionHandler implements SessionHandlerInterface, SessionIdInterf public function create_sid(): string { - return hash('sha256', random_bytes(10) . microtime()); + return bin2hex(random_bytes(16)); } public function validateId($id): bool { - return preg_match('/^[a-f0-9]{64}$/', $id); + return preg_match('/^[a-f0-9]{32}$/', $id); } public function updateTimestamp($id, $data): bool @@ -104,7 +104,7 @@ class DatabaseSessionHandler implements SessionHandlerInterface, SessionIdInterf $modify = new Modify(\Container::$dbConnection, 'sessions'); - $modify->setId($id); + $modify->setId(substr($id, 0, 32)); $modify->set('updated', (new DateTime())->format('Y-m-d H:i:s')); $modify->save(); diff --git a/tests/OAuth/GoogleOAuthTest.php b/tests/OAuth/GoogleOAuthTest.php index a40fc66..301c6ce 100644 --- a/tests/OAuth/GoogleOAuthTest.php +++ b/tests/OAuth/GoogleOAuthTest.php @@ -13,6 +13,7 @@ final class GoogleOAuthTest extends TestCase { $_ENV['GOOGLE_OAUTH_CLIENT_ID'] = 'xyz'; $state = 'random_state_string'; + $nonce = 'random_nonce_string'; $redirectUrl = 'http://example.com/oauth'; $requestMock = $this->getMockBuilder(IRequest::class) @@ -20,7 +21,7 @@ final class GoogleOAuthTest extends TestCase ->getMock(); $googleOAuth = new GoogleOAuth($requestMock); - $dialogUrl = $googleOAuth->getDialogUrl($state, $redirectUrl); + $dialogUrl = $googleOAuth->getDialogUrl($state, $redirectUrl, $nonce); $dialogUrlParsed = explode('?', $dialogUrl); $this->assertEquals('https://accounts.google.com/o/oauth2/v2/auth', $dialogUrlParsed[0]); @@ -33,15 +34,10 @@ final class GoogleOAuthTest extends TestCase 'scope' => 'openid email', 'redirect_uri' => $redirectUrl, 'state' => $state, - 'nonce' => hash('sha256', random_bytes(10) . microtime()), + 'nonce' => $nonce, ]; - $this->assertEquals($expectedQueryParams['response_type'], $dialogUrlQueryParams['response_type']); - $this->assertEquals($expectedQueryParams['client_id'], $dialogUrlQueryParams['client_id']); - $this->assertEquals($expectedQueryParams['scope'], $dialogUrlQueryParams['scope']); - $this->assertEquals($expectedQueryParams['redirect_uri'], $dialogUrlQueryParams['redirect_uri']); - $this->assertEquals($expectedQueryParams['state'], $dialogUrlQueryParams['state']); - $this->assertMatchesRegularExpression('/^[a-f0-9]{64}$/', $dialogUrlQueryParams['nonce']); + $this->assertEquals($expectedQueryParams, $dialogUrlQueryParams); } public function testCanRequestToken(): void diff --git a/web.php b/web.php index 5576441..6ebfecf 100644 --- a/web.php +++ b/web.php @@ -71,6 +71,11 @@ if (isset($_COOKIE['COOKIES_CONSENT'])) { 'cookie_httponly' => true, 'cookie_samesite' => 'Lax' ]); + + // this is needed to handle old type of session IDs + if (!Container::$sessionHandler->validateId(session_id())) { + session_regenerate_id(true); + } } else { $_SESSION = []; } @@ -78,5 +83,5 @@ if (isset($_COOKIE['COOKIES_CONSENT'])) { Container::$request = new MapGuesser\Request\Request($_SERVER['REQUEST_SCHEME'] . '://' . $_SERVER['HTTP_HOST'], $_GET, $_POST, $_SESSION); if (!Container::$request->session()->has('anti_csrf_token')) { - Container::$request->session()->set('anti_csrf_token', hash('sha256', random_bytes(10) . microtime())); + Container::$request->session()->set('anti_csrf_token', bin2hex(random_bytes(16))); } From ef013b8b9edae7c6d8cb24f382ca99e1986d8a10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=91cze=20Bence?= Date: Sun, 5 Jul 2020 12:26:29 +0200 Subject: [PATCH 4/8] MAPG-142 rename app to MapGuesser in .env.example --- .env.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.env.example b/.env.example index 3a5e91e..ac43791 100644 --- a/.env.example +++ b/.env.example @@ -1,4 +1,4 @@ -APP_NAME=MapQuiz +APP_NAME=MapGuesser DEV=1 DB_HOST=mariadb DB_USER=mapguesser From 38885849de09f7e07b271d3325e05722f2754916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=91cze=20Bence?= Date: Sun, 5 Jul 2020 12:26:53 +0200 Subject: [PATCH 5/8] MAPG-142 remove unnecessary 'now' from DateTime constructor --- src/PersistentData/Model/Place.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PersistentData/Model/Place.php b/src/PersistentData/Model/Place.php index 033f05e..6a3b029 100644 --- a/src/PersistentData/Model/Place.php +++ b/src/PersistentData/Model/Place.php @@ -166,7 +166,7 @@ class Place extends Model } $this->panoIdCached = $panoId; - $this->panoIdCachedTimestamp = new DateTime('now'); + $this->panoIdCachedTimestamp = new DateTime(); (new PersistentDataManager())->saveToDb($this); From 32392590bdb8ce589f8c7c91a1f0da1d3ced8e91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=91cze=20Bence?= Date: Sun, 5 Jul 2020 12:49:02 +0200 Subject: [PATCH 6/8] MAPG-142 save user creation date --- .../structure/20200706_1236_user_created.sql | 2 ++ src/Cli/AddUserCommand.php | 2 ++ src/Controller/LoginController.php | 2 ++ src/PersistentData/Model/User.php | 25 ++++++++++++++++++- 4 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 database/migrations/structure/20200706_1236_user_created.sql diff --git a/database/migrations/structure/20200706_1236_user_created.sql b/database/migrations/structure/20200706_1236_user_created.sql new file mode 100644 index 0000000..c60b975 --- /dev/null +++ b/database/migrations/structure/20200706_1236_user_created.sql @@ -0,0 +1,2 @@ +ALTER TABLE `users` + ADD `created` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP; diff --git a/src/Cli/AddUserCommand.php b/src/Cli/AddUserCommand.php index 9ccebc8..212649e 100644 --- a/src/Cli/AddUserCommand.php +++ b/src/Cli/AddUserCommand.php @@ -1,5 +1,6 @@ setEmail($input->getArgument('email')); $user->setPlainPassword($input->getArgument('password')); $user->setActive(true); + $user->setCreatedDate(new DateTime()); if ($input->hasArgument('type')) { $user->setType($input->getArgument('type')); diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index 86a73aa..d7f1eb5 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -311,6 +311,7 @@ class LoginController $user = new User(); $user->setEmail($this->request->post('email')); $user->setPlainPassword($this->request->post('password')); + $user->setCreatedDate(new DateTime()); \Container::$dbConnection->startTransaction(); @@ -349,6 +350,7 @@ class LoginController $user = new User(); $user->setEmail($userData['email']); + $user->setCreatedDate(new DateTime()); } else { $sendWelcomeEmail = false; } diff --git a/src/PersistentData/Model/User.php b/src/PersistentData/Model/User.php index e13adfd..147362d 100644 --- a/src/PersistentData/Model/User.php +++ b/src/PersistentData/Model/User.php @@ -1,12 +1,13 @@ email = $email; @@ -52,6 +55,16 @@ class User extends Model implements IUser $this->googleSub = $googleSub; } + public function setCreatedDate(DateTime $created): void + { + $this->created = $created; + } + + public function setCreated(string $created): void + { + $this->created = new DateTime($created); + } + public function getEmail(): string { return $this->email; @@ -77,6 +90,16 @@ class User extends Model implements IUser return $this->googleSub; } + public function getCreatedData(): DateTime + { + return $this->created; + } + + public function getCreated(): string + { + return $this->created->format('Y-m-d H:i:s'); + } + public function hasPermission(int $permission): bool { switch ($permission) { From 37094e552bf5131ae283700ca06eb7b89f0b4a71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=91cze=20Bence?= Date: Sun, 5 Jul 2020 13:22:22 +0200 Subject: [PATCH 7/8] MAPG-142 modify UserPasswordResetterRepository to return only one resetter for user (it is not possible for an user to have multiple) --- src/Controller/UserController.php | 3 ++- src/Repository/UserPasswordResetterRepository.php | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Controller/UserController.php b/src/Controller/UserController.php index f285318..4b0e26f 100644 --- a/src/Controller/UserController.php +++ b/src/Controller/UserController.php @@ -196,7 +196,8 @@ class UserController implements ISecured $this->pdm->deleteFromDb($userConfirmation); } - foreach ($this->userPasswordResetterRepository->getByUser($user) as $userPasswordResetter) { + $userPasswordResetter = $this->userPasswordResetterRepository->getByUser($user); + if ($userPasswordResetter !== null) { $this->pdm->deleteFromDb($userPasswordResetter); } diff --git a/src/Repository/UserPasswordResetterRepository.php b/src/Repository/UserPasswordResetterRepository.php index 04a82af..466c3c8 100644 --- a/src/Repository/UserPasswordResetterRepository.php +++ b/src/Repository/UserPasswordResetterRepository.php @@ -1,6 +1,5 @@ pdm->selectFromDb($select, UserPasswordResetter::class); } - public function getByUser(User $user): Generator + public function getByUser(User $user): ?UserPasswordResetter { $select = new Select(\Container::$dbConnection); $select->where('user_id', '=', $user->getId()); - yield from $this->pdm->selectMultipleFromDb($select, UserPasswordResetter::class); + return $this->pdm->selectFromDb($select, UserPasswordResetter::class); } } From dd6bb5ef9ae05a399486f8732ad50209b37806de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=91cze=20Bence?= Date: Sun, 5 Jul 2020 13:23:53 +0200 Subject: [PATCH 8/8] MAPG-142 limit password reset query if the existing is not expired --- src/Controller/LoginController.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php index d7f1eb5..92d7a46 100644 --- a/src/Controller/LoginController.php +++ b/src/Controller/LoginController.php @@ -468,6 +468,16 @@ class LoginController ]); } + $existingResetter = $this->userPasswordResetterRepository->getByUser($user); + + if ($existingResetter !== null && $existingResetter->getExpiresDate() > new DateTime()) { + return new JsonContent([ + 'error' => [ + 'errorText' => 'Password reset was recently requested for this account. Please check your email, or try again later!' + ] + ]); + } + $token = bin2hex(random_bytes(16)); $expires = new DateTime('+1 hour'); @@ -476,8 +486,16 @@ class LoginController $passwordResetter->setToken($token); $passwordResetter->setExpiresDate($expires); + \Container::$dbConnection->startTransaction(); + + if ($existingResetter !== null) { + $this->pdm->deleteFromDb($existingResetter); + } + $this->pdm->saveToDb($passwordResetter); + \Container::$dbConnection->commit(); + $this->sendPasswordResetEmail($user->getEmail(), $token, $expires); return new JsonContent(['success' => true]);