From 85a026c37b43b10900d34d8c777a33b0eeaafa78 Mon Sep 17 00:00:00 2001 From: Marcin Kurczewski Date: Mon, 8 Sep 2014 13:06:32 +0200 Subject: [PATCH] Added e-mail confirmation and password reset --- data/config.ini | 6 + public_html/index.html | 1 - public_html/js/Presenters/LoginPresenter.js | 11 +- .../js/Presenters/PasswordResetPresenter.js | 32 ----- .../js/Presenters/RegistrationPresenter.js | 7 +- .../js/Presenters/UserActivationPresenter.js | 98 ++++++++++++++- public_html/js/Router.js | 8 +- public_html/templates/user-query-form.tpl | 19 +++ src/Controllers/UserController.php | 60 +++++++--- src/Controllers/ViewProxies/UserViewProxy.php | 1 + src/Dao/UserDao.php | 8 ++ src/Entities/Token.php | 2 + src/Entities/User.php | 1 + src/Services/AuthService.php | 27 +++-- src/Services/EmailService.php | 100 ++++++++++++++++ src/Services/PasswordService.php | 21 ++++ src/Services/PrivilegeService.php | 11 ++ src/Services/TokenService.php | 14 +-- src/Services/UserService.php | 113 +++++++++++++++--- tests/Services/AuthServiceTest.php | 77 +++++++++++- tests/Services/PasswordServiceTest.php | 47 ++++++++ tests/Services/PrivilegeServiceTest.php | 17 ++- tests/Services/UserServiceTest.php | 45 ++++++- 23 files changed, 619 insertions(+), 107 deletions(-) delete mode 100644 public_html/js/Presenters/PasswordResetPresenter.js create mode 100644 public_html/templates/user-query-form.tpl create mode 100644 tests/Services/PasswordServiceTest.php diff --git a/data/config.ini b/data/config.ini index f47f84d6..26b77f1f 100644 --- a/data/config.ini +++ b/data/config.ini @@ -1,3 +1,8 @@ +[basic] +serviceName = szuru2 +serviceBaseUrl = http://localhost/ +emailAddress = noreply@localhost + [database] host = localhost port = 27017 @@ -6,6 +11,7 @@ name = booru-dev [security] secret = change minPasswordLength = 5 +needEmailActivationToRegister = 1 [security.privileges] register = anonymous diff --git a/public_html/index.html b/public_html/index.html index 1302f5c7..e307c765 100644 --- a/public_html/index.html +++ b/public_html/index.html @@ -53,7 +53,6 @@ - diff --git a/public_html/js/Presenters/LoginPresenter.js b/public_html/js/Presenters/LoginPresenter.js index 276e47f1..63f1a777 100644 --- a/public_html/js/Presenters/LoginPresenter.js +++ b/public_html/js/Presenters/LoginPresenter.js @@ -40,12 +40,19 @@ App.Presenters.LoginPresenter = function( var password = $el.find('[name=password]').val(); var remember = $el.find('[name=remember]').val(); - //todo: client side error reporting + if (userName.length == 0) { + messagePresenter.showError($messages, 'User name cannot be empty.'); + return false; + } + + if (password.length == 0) { + messagePresenter.showError($messages, 'Password cannot be empty.'); + return false; + } auth.loginFromCredentials(userName, password, remember) .then(function(response) { router.navigateToMainPage(); - //todo: "redirect" to main page }).fail(function(response) { messagePresenter.showError($messages, response.json && response.json.error || response); }); diff --git a/public_html/js/Presenters/PasswordResetPresenter.js b/public_html/js/Presenters/PasswordResetPresenter.js deleted file mode 100644 index b5380e03..00000000 --- a/public_html/js/Presenters/PasswordResetPresenter.js +++ /dev/null @@ -1,32 +0,0 @@ -var App = App || {}; -App.Presenters = App.Presenters || {}; - -App.Presenters.PasswordResetPresenter = function( - jQuery, - topNavigationPresenter, - messagePresenter) { - - var $el = jQuery('#content'); - var $messages = $el; - - function init(args) { - topNavigationPresenter.select('login'); - if (args.token) { - alert('Got token'); - } else { - render(); - } - } - - function render() { - $el.html('Password reset placeholder'); - }; - - return { - init: init, - render: render, - }; - -}; - -App.DI.register('passwordResetPresenter', App.Presenters.PasswordResetPresenter); diff --git a/public_html/js/Presenters/RegistrationPresenter.js b/public_html/js/Presenters/RegistrationPresenter.js index 810641d1..1baf51ef 100644 --- a/public_html/js/Presenters/RegistrationPresenter.js +++ b/public_html/js/Presenters/RegistrationPresenter.js @@ -50,10 +50,13 @@ App.Presenters.RegistrationPresenter = function( } function registrationSuccess(apiResponse) { - //todo: tell user if it turned out that he needs to confirm his e-mail $el.find('form').slideUp(function() { var message = 'Registration complete! '; - message += 'Click here to login.'; + if (!apiResponse.json.confirmed) { + message += '
Check your inbox for activation e-mail.
If e-mail doesn\'t show up, check your spam folder.'; + } else { + message += 'Click here to login.'; + } messagePresenter.showInfo($messages, message); }); } diff --git a/public_html/js/Presenters/UserActivationPresenter.js b/public_html/js/Presenters/UserActivationPresenter.js index 58accd67..90688a80 100644 --- a/public_html/js/Presenters/UserActivationPresenter.js +++ b/public_html/js/Presenters/UserActivationPresenter.js @@ -3,21 +3,111 @@ App.Presenters = App.Presenters || {}; App.Presenters.UserActivationPresenter = function( jQuery, - topNavigationPresenter) { + promise, + util, + auth, + api, + router, + topNavigationPresenter, + messagePresenter) { var $el = jQuery('#content'); + var $messages = $el; + var template; + var formHidden = false; + var operation; function init(args) { + if (auth.isLoggedIn()) { + router.navigateToMainPage(); + return; + } + topNavigationPresenter.select('login'); - render(); + reinit(args); + } + + function reinit(args) { + operation = args.operation; + console.log(operation); + promise.wait(util.promiseTemplate('user-query-form')).then(function(html) { + template = _.template(html); + if (args.token) { + hideForm(); + confirmToken(args.token); + } else { + showForm(); + } + render(); + }); } function render() { - $el.html('Account activation placeholder'); - }; + $el.html(template()); + $messages = $el.find('.messages'); + if (formHidden) + $el.find('form').hide(); + $el.find('form').submit(userQueryFormSubmitted); + } + + function hideForm() { + formHidden = true; + } + + function showForm() { + formHidden = false; + } + + function userQueryFormSubmitted(e) { + e.preventDefault(); + messagePresenter.hideMessages($messages); + + var userNameOrEmail = $el.find('form input[name=user]').val(); + if (userNameOrEmail.length == 0) { + messagePresenter.showError($messages, 'Field cannot be blank.'); + return; + } + var url = operation == 'passwordReset' + ? '/password-reset/' + userNameOrEmail + : '/activation/' + userNameOrEmail; + + api.post(url).then(function(response) { + var message = operation == 'passwordReset' + ? 'Password reset request sent.' + : 'Activation e-mail resent.'; + message += ' Check your inbox.
If e-mail doesn\'t show up, check your spam folder.'; + + $el.find('#user-query-form').slideUp(function() { + messagePresenter.showInfo($messages, message); + }); + }).fail(function(response) { + messagePresenter.showError($messages, response.json && response.json.error || response); + }); + } + + function confirmToken(token) { + messagePresenter.hideMessages($messages); + + var url = operation == 'passwordReset' + ? '/finish-password-reset/' + token + : '/finish-activation/' + token; + + api.post(url).then(function(response) { + var message = operation == 'passwordReset' + ? 'Your new password is ' + response.json.newPassword + '.' + : 'E-mail activation successful.'; + + $el.find('#user-query-form').slideUp(function() { + messagePresenter.showInfo($messages, message); + }); + }).fail(function(response) { + messagePresenter.showError($messages, response.json && response.json.error || response); + }); + } return { init: init, + reinit: reinit, render: render, }; diff --git a/public_html/js/Router.js b/public_html/js/Router.js index 4bdebb31..0faa6203 100644 --- a/public_html/js/Router.js +++ b/public_html/js/Router.js @@ -24,8 +24,8 @@ App.Router = function(jQuery, util, appState) { inject('#/logout', 'logoutPresenter'); inject('#/register', 'registrationPresenter'); inject('#/upload', 'postUploadPresenter'); - inject('#/password-reset(/:token)', 'passwordResetPresenter'); - inject('#/activate(/:token)', 'userActivationPresenter'); + inject('#/password-reset(/:token)', 'userActivationPresenter', {operation: 'passwordReset'}); + inject('#/activate(/:token)', 'userActivationPresenter', {operation: 'activation'}); inject('#/users(/:searchArgs)', 'userListPresenter'); inject('#/user/:userName(/:tab)', 'userPresenter'); inject('#/posts(/:searchArgs)', 'postListPresenter'); @@ -40,9 +40,9 @@ App.Router = function(jQuery, util, appState) { Path.root(newRoot); }; - function inject(path, presenterName) { + function inject(path, presenterName, additionalParams) { Path.map(path).to(function() { - util.initContentPresenter(presenterName, this.params); + util.initContentPresenter(presenterName, _.extend(this.params, additionalParams)); }); }; diff --git a/public_html/templates/user-query-form.tpl b/public_html/templates/user-query-form.tpl new file mode 100644 index 00000000..5557daf7 --- /dev/null +++ b/public_html/templates/user-query-form.tpl @@ -0,0 +1,19 @@ +
+ +
+
+
+ +
+ +
+
+ +
+ +
+ +
+
+
+
diff --git a/src/Controllers/UserController.php b/src/Controllers/UserController.php index e215d1cd..1fa972da 100644 --- a/src/Controllers/UserController.php +++ b/src/Controllers/UserController.php @@ -24,14 +24,18 @@ final class UserController extends AbstractController { $router->post('/api/users', [$this, 'createUser']); $router->get('/api/users', [$this, 'getFiltered']); - $router->get('/api/users/:userName', [$this, 'getByName']); - $router->put('/api/users/:userName', [$this, 'updateUser']); - $router->delete('/api/users/:userName', [$this, 'deleteUser']); + $router->get('/api/users/:userNameOrEmail', [$this, 'getByNameOrEmail']); + $router->put('/api/users/:userNameOrEmail', [$this, 'updateUser']); + $router->delete('/api/users/:userNameOrEmail', [$this, 'deleteUser']); + $router->post('/api/password-reset/:userNameOrEmail', [$this, 'passwordReset']); + $router->post('/api/finish-password-reset/:tokenName', [$this, 'finishPasswordReset']); + $router->post('/api/activation/:userNameOrEmail', [$this, 'activation']); + $router->post('/api/finish-activation/:tokenName', [$this, 'finishActivation']); } - public function getByName($userName) + public function getByNameOrEmail($userNameOrEmail) { - $user = $this->userService->getByName($userName); + $user = $this->userService->getByNameOrEmail($userNameOrEmail); return $this->userViewProxy->fromEntity($user); } @@ -53,17 +57,18 @@ final class UserController extends AbstractController $this->privilegeService->assertPrivilege(\Szurubooru\Privilege::REGISTER); $formData = new \Szurubooru\FormData\RegistrationFormData($this->inputReader); $user = $this->userService->createUser($formData); - return $this->userViewProxy->fromEntity($user); + return array_merge((array) $this->userViewProxy->fromEntity($user), ['confirmed' => $user->emailUnconfirmed == null]); } - public function updateUser($userName) + public function updateUser($userNameOrEmail) { + $user = $this->userService->getByNameOrEmail($userNameOrEmail); $formData = new \Szurubooru\FormData\UserEditFormData($this->inputReader); if ($formData->avatarStyle !== null) { $this->privilegeService->assertPrivilege( - $this->privilegeService->isLoggedIn($userName) + $this->privilegeService->isLoggedIn($userNameOrEmail) ? \Szurubooru\Privilege::CHANGE_OWN_AVATAR_STYLE : \Szurubooru\Privilege::CHANGE_ALL_AVATAR_STYLES); } @@ -71,7 +76,7 @@ final class UserController extends AbstractController if ($formData->userName !== null) { $this->privilegeService->assertPrivilege( - $this->privilegeService->isLoggedIn($userName) + $this->privilegeService->isLoggedIn($userNameOrEmail) ? \Szurubooru\Privilege::CHANGE_OWN_NAME : \Szurubooru\Privilege::CHANGE_ALL_NAMES); } @@ -79,7 +84,7 @@ final class UserController extends AbstractController if ($formData->password !== null) { $this->privilegeService->assertPrivilege( - $this->privilegeService->isLoggedIn($userName) + $this->privilegeService->isLoggedIn($userNameOrEmail) ? \Szurubooru\Privilege::CHANGE_OWN_PASSWORD : \Szurubooru\Privilege::CHANGE_ALL_PASSWORDS); } @@ -87,7 +92,7 @@ final class UserController extends AbstractController if ($formData->email !== null) { $this->privilegeService->assertPrivilege( - $this->privilegeService->isLoggedIn($userName) + $this->privilegeService->isLoggedIn($userNameOrEmail) ? \Szurubooru\Privilege::CHANGE_OWN_EMAIL_ADDRESS : \Szurubooru\Privilege::CHANGE_ALL_EMAIL_ADDRESSES); } @@ -99,20 +104,43 @@ final class UserController extends AbstractController if ($formData->browsingSettings) { - $this->privilegeService->assertLoggedIn($userName); + $this->privilegeService->assertLoggedIn($userNameOrEmail); } - $user = $this->userService->updateUser($userName, $formData); + $user = $this->userService->updateUser($user, $formData); return $this->userViewProxy->fromEntity($user); } - public function deleteUser($userName) + public function deleteUser($userNameOrEmail) { $this->privilegeService->assertPrivilege( - $this->privilegeService->isLoggedIn($userName) + $this->privilegeService->isLoggedIn($userNameOrEmail) ? \Szurubooru\Privilege::DELETE_OWN_ACCOUNT : \Szurubooru\Privilege::DELETE_ACCOUNTS); - return $this->userService->deleteUserByName($userName); + $user = $this->userService->getByNameOrEmail($userNameOrEmail); + return $this->userService->deleteUser($user); + } + + public function passwordReset($userNameOrEmail) + { + $user = $this->userService->getByNameOrEmail($userNameOrEmail); + return $this->userService->sendPasswordResetEmail($user); + } + + public function activation($userNameOrEmail) + { + $user = $this->userService->getByNameOrEmail($userNameOrEmail, true); + return $this->userService->sendActivationEmail($user); + } + + public function finishPasswordReset($tokenName) + { + return ['newPassword' => $this->userService->finishPasswordReset($tokenName)]; + } + + public function finishActivation($tokenName) + { + $this->userService->finishActivation($tokenName); } } diff --git a/src/Controllers/ViewProxies/UserViewProxy.php b/src/Controllers/ViewProxies/UserViewProxy.php index 5b8104d8..e7f90a17 100644 --- a/src/Controllers/ViewProxies/UserViewProxy.php +++ b/src/Controllers/ViewProxies/UserViewProxy.php @@ -31,6 +31,7 @@ class UserViewProxy extends AbstractViewProxy $this->privilegeService->isLoggedIn($user)) { $result->email = $user->email; + $result->emailUnconfirmed = $user->emailUnconfirmed; } } return $result; diff --git a/src/Dao/UserDao.php b/src/Dao/UserDao.php index f2c8b5fa..5cef016c 100644 --- a/src/Dao/UserDao.php +++ b/src/Dao/UserDao.php @@ -15,6 +15,14 @@ class UserDao extends AbstractDao implements ICrudDao return $this->entityConverter->toEntity($arrayEntity); } + public function getByEmail($userEmail, $allowUnconfirmed = false) + { + $arrayEntity = $this->collection->findOne(['email' => $userEmail]); + if (!$arrayEntity and $allowUnconfirmed) + $arrayEntity = $this->collection->findOne(['emailUnconfirmed' => $userEmail]); + return $this->entityConverter->toEntity($arrayEntity); + } + public function hasAnyUsers() { return (bool) $this->collection->findOne(); diff --git a/src/Entities/Token.php b/src/Entities/Token.php index 138af4cc..cec002fc 100644 --- a/src/Entities/Token.php +++ b/src/Entities/Token.php @@ -4,6 +4,8 @@ namespace Szurubooru\Entities; final class Token extends Entity { const PURPOSE_LOGIN = 'login'; + const PURPOSE_ACTIVATE = 'activate'; + const PURPOSE_PASSWORD_RESET = 'passwordReset'; public $name; public $purpose; diff --git a/src/Entities/User.php b/src/Entities/User.php index dd8c5797..fb313d38 100644 --- a/src/Entities/User.php +++ b/src/Entities/User.php @@ -16,6 +16,7 @@ final class User extends Entity public $name; public $email; + public $emailUnconfirmed; public $passwordHash; public $accessRank; public $registrationTime; diff --git a/src/Services/AuthService.php b/src/Services/AuthService.php index fbd3d493..e4cc37b8 100644 --- a/src/Services/AuthService.php +++ b/src/Services/AuthService.php @@ -6,20 +6,20 @@ class AuthService private $loggedInUser = null; private $loginToken = null; - private $validator; + private $config; private $passwordService; private $timeService; private $userService; private $tokenService; public function __construct( - \Szurubooru\Validator $validator, + \Szurubooru\Config $config, \Szurubooru\Services\PasswordService $passwordService, \Szurubooru\Services\TimeService $timeService, \Szurubooru\Services\TokenService $tokenService, \Szurubooru\Services\UserService $userService) { - $this->validator = $validator; + $this->config = $config; $this->passwordService = $passwordService; $this->timeService = $timeService; $this->tokenService = $tokenService; @@ -43,12 +43,10 @@ class AuthService return $this->loginToken; } - public function loginFromCredentials($userName, $password) + public function loginFromCredentials($userNameOrEmail, $password) { - $this->validator->validateUserName($userName); - $this->validator->validatePassword($password); - - $user = $this->userService->getByName($userName); + $user = $this->userService->getByNameOrEmail($userNameOrEmail); + $this->validateUser($user); $passwordHash = $this->passwordService->getHash($password); if ($user->passwordHash != $passwordHash) @@ -61,13 +59,12 @@ class AuthService public function loginFromToken($loginTokenName) { - $this->validator->validateToken($loginTokenName); - $loginToken = $this->tokenService->getByName($loginTokenName); if ($loginToken->purpose != \Szurubooru\Entities\Token::PURPOSE_LOGIN) throw new \Exception('This token is not a login token.'); $user = $this->userService->getById($loginToken->additionalData); + $this->validateUser($user); $this->loginToken = $loginToken; $this->loggedInUser = $user; @@ -93,12 +90,18 @@ class AuthService if (!$this->isLoggedIn()) throw new \Exception('Not logged in.'); - $this->tokenService->invalidateByToken($this->loginToken); + $this->tokenService->invalidateByName($this->loginToken); $this->loginToken = null; } private function createAndSaveLoginToken(\Szurubooru\Entities\User $user) { - return $this->tokenService->createAndSaveToken($user, \Szurubooru\Entities\Token::PURPOSE_LOGIN); + return $this->tokenService->createAndSaveToken($user->id, \Szurubooru\Entities\Token::PURPOSE_LOGIN); + } + + private function validateUser($user) + { + if (!$user->email and $this->config->security->needEmailActivationToRegister) + throw new \DomainException('User didn\'t confirm mail yet.'); } } diff --git a/src/Services/EmailService.php b/src/Services/EmailService.php index f6a5a488..956ac328 100644 --- a/src/Services/EmailService.php +++ b/src/Services/EmailService.php @@ -3,4 +3,104 @@ namespace Szurubooru\Services; class EmailService { + private $config; + + public function __construct(\Szurubooru\Config $config) + { + $this->config = $config; + } + + public function sendPasswordResetEmail(\Szurubooru\Entities\User $user, \Szurubooru\Entities\Token $token) + { + if (!$user->email) + throw new \BadMethodCall('An activated e-mail addreses is needed to reset the password.'); + + $recipientEmail = $user->email; + $senderName = $this->config->basic->serviceName . ' bot'; + $subject = $this->config->basic->serviceName . ' password reset'; + + $body = + 'Hello,' . + PHP_EOL . PHP_EOL . + 'Someone (probably you) requested to reset password for an account at ' . $this->config->basic->serviceName . '. ' . + 'In order to proceed, please click this link or paste it in your browser address bar: ' . + PHP_EOL . PHP_EOL . + $this->config->basic->serviceBaseUrl . '#/password-reset/' . $token->name . + PHP_EOL . PHP_EOL . + 'Otherwise please ignore this mail.' . + $this->getFooter(); + + $this->sendEmail($senderName, $recipientEmail, $subject, $body); + } + + public function sendActivationEmail(\Szurubooru\Entities\User $user, \Szurubooru\Entities\Token $token) + { + if (!$user->emailUnconfirmed) + { + throw new \BadMethodCallException( + $user->email + ? 'E-mail for this account is already confirmed.' + : 'An e-mail address is needed to activate the account.'); + } + + $recipientEmail = $user->emailUnconfirmed; + $senderName = $this->config->basic->serviceName . ' bot'; + $subject = $this->config->basic->serviceName . ' account activation'; + + $body = + 'Hello,' . + PHP_EOL . PHP_EOL . + 'Someone (probably you) registered at ' . $this->config->basic->serviceName . ' an account with this e-mail address. ' . + 'In order to finish activation, please click this link or paste it in your browser address bar: ' . + PHP_EOL . PHP_EOL . + $this->config->basic->serviceBaseUrl . '#/activate/' . $token->name . + PHP_EOL . PHP_EOL . + 'Otherwise please ignore this mail.' . + $this->getFooter(); + + $this->sendEmail($senderName, $recipientEmail, $subject, $body); + } + + private function sendEmail($senderName, $recipientEmail, $subject, $body) + { + $senderEmail = $this->config->basic->emailAddress; + $domain = substr($senderEmail, strpos($senderEmail, '@') + 1); + + $clientIp = isset($_SERVER['SERVER_ADDR']) + ? $_SERVER['SERVER_ADDR'] + : ''; + + $body = wordwrap($body, 70); + if (empty($recipientEmail)) + throw new \InvalidArgumentException('Destination e-mail address was not found'); + + $messageId = sha1(date('r') . uniqid()) . '@' . $domain; + + $headers = []; + $headers []= sprintf('MIME-Version: 1.0'); + $headers []= sprintf('Content-Transfer-Encoding: 7bit'); + $headers []= sprintf('Date: %s', date('r')); + $headers []= sprintf('Message-ID: <%s>', $messageId); + $headers []= sprintf('From: %s <%s>', $senderName, $senderEmail); + $headers []= sprintf('Reply-To: %s', $senderEmail); + $headers []= sprintf('Return-Path: %s', $senderEmail); + $headers []= sprintf('Subject: %s', $subject); + $headers []= sprintf('Content-Type: text/plain; charset=utf-8'); + $headers []= sprintf('X-Mailer: PHP/%s', phpversion()); + $headers []= sprintf('X-Originating-IP: %s', $clientIp); + + $senderEmail = $this->config->basic->emailAddress; + $encodedSubject = '=?UTF-8?B?' . base64_encode($subject) . '?='; + + $arguments = [$recipientEmail, $encodedSubject, $body, implode("\r\n", $headers), '-f' . $senderEmail]; + //throw new \RuntimeException(htmlentities(print_r($arguments, true))); + call_user_func_array('mail', $arguments); + } + + private function getFooter() + { + return PHP_EOL . PHP_EOL . + 'Thank you and have a nice day,' . PHP_EOL . + $this->config->basic->serviceName . ' registration bot'; + } } diff --git a/src/Services/PasswordService.php b/src/Services/PasswordService.php index fdbf6b7e..6ebc775e 100644 --- a/src/Services/PasswordService.php +++ b/src/Services/PasswordService.php @@ -4,14 +4,35 @@ namespace Szurubooru\Services; class PasswordService { private $config; + private $alphabet; + private $pattern; public function __construct(\Szurubooru\Config $config) { $this->config = $config; + $this->alphabet = + [ + 'c' => str_split('bcdfghjklmnpqrstvwxyz'), + 'v' => str_split('aeiou'), + 'n' => str_split('0123456789'), + ]; + $this->pattern = str_split('cvcvnncvcv'); } public function getHash($password) { return hash('sha256', $this->config->security->secret . '/' . $password); } + + public function getRandomPassword() + { + $password = ''; + foreach ($this->pattern as $token) + { + $subAlphabet = $this->alphabet[$token]; + $character = $subAlphabet[mt_rand(0, count($subAlphabet) - 1)]; + $password .= $character; + } + return $password; + } } diff --git a/src/Services/PrivilegeService.php b/src/Services/PrivilegeService.php index 251dce0b..8b6c29c1 100644 --- a/src/Services/PrivilegeService.php +++ b/src/Services/PrivilegeService.php @@ -57,10 +57,21 @@ class PrivilegeService { $loggedInUser = $this->authService->getLoggedInUser(); if ($userIdentifier instanceof \Szurubooru\Entities\User) + { return $loggedInUser->name == $userIdentifier->name; + } elseif (is_string($userIdentifier)) + { + if ($loggedInUser->email) + { + if ($loggedInUser->email == $userIdentifier) + return true; + } return $loggedInUser->name == $userIdentifier; + } else + { throw new \InvalidArgumentException('Invalid user identifier.'); + } } } diff --git a/src/Services/TokenService.php b/src/Services/TokenService.php index 985a48bc..6e644a1a 100644 --- a/src/Services/TokenService.php +++ b/src/Services/TokenService.php @@ -18,23 +18,23 @@ class TokenService return $token; } - public function invalidateByToken($tokenName) + public function invalidateByName($tokenName) { return $this->tokenDao->deleteByName($tokenName); } - public function invalidateByUser(\Szurubooru\Entities\User $user) + public function invalidateByAdditionalData($additionalData) { - return $this->tokenDao->deleteByAdditionalData($user->id); + return $this->tokenDao->deleteByAdditionalData($additionalData); } - public function createAndSaveToken(\Szurubooru\Entities\User $user, $tokenPurpose) + public function createAndSaveToken($additionalData, $tokenPurpose) { $token = new \Szurubooru\Entities\Token(); - $token->name = hash('sha256', $user->name . '/' . microtime(true)); - $token->additionalData = $user->id; + $token->name = sha1(date('r') . uniqid() . microtime(true)); + $token->additionalData = $additionalData; $token->purpose = $tokenPurpose; - $this->invalidateByUser($user); + $this->invalidateByAdditionalData($additionalData); $this->tokenDao->save($token); return $token; } diff --git a/src/Services/UserService.php b/src/Services/UserService.php index 885a00f0..5f4b5050 100644 --- a/src/Services/UserService.php +++ b/src/Services/UserService.php @@ -11,6 +11,7 @@ class UserService private $emailService; private $fileService; private $timeService; + private $tokenService; public function __construct( \Szurubooru\Config $config, @@ -20,7 +21,8 @@ class UserService \Szurubooru\Services\PasswordService $passwordService, \Szurubooru\Services\EmailService $emailService, \Szurubooru\Services\FileService $fileService, - \Szurubooru\Services\TimeService $timeService) + \Szurubooru\Services\TimeService $timeService, + \Szurubooru\Services\TokenService $tokenService) { $this->config = $config; $this->validator = $validator; @@ -30,6 +32,20 @@ class UserService $this->emailService = $emailService; $this->fileService = $fileService; $this->timeService = $timeService; + $this->tokenService = $tokenService; + } + + public function getByNameOrEmail($userNameOrEmail, $allowUnconfirmed = false) + { + $user = $this->userDao->getByName($userNameOrEmail); + if ($user) + return $user; + + $user = $this->userDao->getByEmail($userNameOrEmail, $allowUnconfirmed); + if ($user) + return $user; + + throw new \InvalidArgumentException('User "' . $userNameOrEmail . '" was not found.'); } public function getByName($userName) @@ -62,12 +78,15 @@ class UserService $this->validator->validatePassword($formData->password); $this->validator->validateEmail($formData->email); + if ($formData->email and $this->userDao->getByEmail($formData->email)) + throw new \DomainException('User with this e-mail already exists.'); + if ($this->userDao->getByName($formData->userName)) throw new \DomainException('User with this name already exists.'); $user = new \Szurubooru\Entities\User(); $user->name = $formData->userName; - $user->email = $formData->email; + $user->emailUnconfirmed = $formData->email; $user->passwordHash = $this->passwordService->getHash($formData->password); $user->accessRank = $this->userDao->hasAnyUsers() ? \Szurubooru\Entities\User::ACCESS_RANK_REGULAR_USER @@ -76,15 +95,13 @@ class UserService $user->lastLoginTime = null; $user->avatarStyle = \Szurubooru\Entities\User::AVATAR_STYLE_GRAVATAR; - $this->sendActivationMailIfNeeded($user); + $this->sendActivationEmailIfNeeded($user); return $this->userDao->save($user); } - public function updateUser($userName, \Szurubooru\FormData\UserEditFormData $formData) + public function updateUser(\Szurubooru\Entities\User $user, \Szurubooru\FormData\UserEditFormData $formData) { - $user = $this->getByName($userName); - if ($formData->avatarStyle !== null) { $user->avatarStyle = \Szurubooru\Helpers\EnumHelper::avatarStyleFromString($formData->avatarStyle); @@ -95,7 +112,8 @@ class UserService if ($formData->userName !== null and $formData->userName != $user->name) { $this->validator->validateUserName($formData->userName); - if ($this->userDao->getByName($formData->userName)) + $userWithThisEmail = $this->userDao->getByName($formData->userName); + if ($userWithThisEmail and $userWithThisEmail->id != $user->id) throw new \DomainException('User with this name already exists.'); $user->name = $formData->userName; @@ -107,10 +125,13 @@ class UserService $user->passwordHash = $this->passwordService->getHash($formData->password); } - if ($formData->email !== null) + if ($formData->email !== null and $formData->email != $user->email) { $this->validator->validateEmail($formData->email); - $user->email = $formData->email; + if ($this->userDao->getByEmail($formData->email)) + throw new \DomainException('User with this e-mail already exists.'); + + $user->emailUnconfirmed = $formData->email; } if ($formData->accessRank !== null) @@ -128,17 +149,15 @@ class UserService } if ($formData->email !== null) - $this->sendActivationMailIfNeeded($user); + $this->sendActivationEmailIfNeeded($user); return $this->userDao->save($user); } - public function deleteUserByName($userName) + public function deleteUser(\Szurubooru\Entities\User $user) { - $user = $this->getByName($userName); - $this->userDao->deleteByName($userName); + $this->userDao->deleteById($user->id); $this->fileService->delete($this->getCustomAvatarSourcePath($user)); - return true; } public function getCustomAvatarSourcePath(\Szurubooru\Entities\User $user) @@ -157,8 +176,70 @@ class UserService $this->userDao->save($user); } - private function sendActivationMailIfNeeded(\Szurubooru\Entities\User &$user) + public function sendPasswordResetEmail(\Szurubooru\Entities\User $user) { - //todo + $token = $this->tokenService->createAndSaveToken($user->name, \Szurubooru\Entities\Token::PURPOSE_PASSWORD_RESET); + $this->emailService->sendPasswordResetEmail($user, $token); + } + + public function finishPasswordReset($tokenName) + { + $token = $this->tokenService->getByName($tokenName); + if ($token->purpose != \Szurubooru\Entities\Token::PURPOSE_PASSWORD_RESET) + throw new \Exception('This token is not a password reset token.'); + + $user = $this->getByName($token->additionalData); + $newPassword = $this->passwordService->getRandomPassword(); + $user->passwordHash = $this->passwordService->getHash($newPassword); + $this->userDao->save($user); + $this->tokenService->invalidateByName($token->name); + return $newPassword; + } + + public function sendActivationEmail(\Szurubooru\Entities\User $user) + { + $token = $this->tokenService->createAndSaveToken($user->name, \Szurubooru\Entities\Token::PURPOSE_ACTIVATE); + $this->emailService->sendActivationEmail($user, $token); + } + + public function finishActivation($tokenName) + { + $token = $this->tokenService->getByName($tokenName); + if ($token->purpose != \Szurubooru\Entities\Token::PURPOSE_ACTIVATE) + throw new \Exception('This token is not an activation token.'); + + $user = $this->getByName($token->additionalData); + $this->confirmEmail($user); + $this->tokenService->invalidateByName($token->name); + } + + private function sendActivationEmailIfNeeded(\Szurubooru\Entities\User &$user) + { + if ($user->accessRank == \Szurubooru\Entities\User::ACCESS_RANK_ADMINISTRATOR or !$this->config->security->needEmailActivationToRegister) + { + $this->confirmEmail($user); + } + else + { + $this->sendActivationEmail($user); + } + } + + private function confirmEmail(\Szurubooru\Entities\User &$user) + { + //security issue: + //1. two users set their unconfirmed mail to godzilla@empire.gov + //2. activation mail is sent to both of them + //3. first user confirms, ok + //4. second user confirms, ok + //5. two users share the same mail --> problem. + //by checking here again for users with such mail, this problem is solved with first-come first-serve approach: + //whoever confirms e-mail first, wins. + if ($this->userDao->getByEmail($user->emailUnconfirmed)) + throw new \DomainException('This e-mail was already confirmed by someone else in the meantime.'); + + $user->email = $user->emailUnconfirmed; + $user->emailUnconfirmed = null; + $this->userDao->save($user); } } diff --git a/tests/Services/AuthServiceTest.php b/tests/Services/AuthServiceTest.php index ba11256f..1e86b788 100644 --- a/tests/Services/AuthServiceTest.php +++ b/tests/Services/AuthServiceTest.php @@ -3,7 +3,7 @@ namespace Szurubooru\Tests\Services; class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase { - private $validatorMock; + private $configMock; private $passwordServiceMock; private $timeServiceMock; private $tokenServiceMock; @@ -11,7 +11,7 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase public function setUp() { - $this->validatorMock = $this->mock(\Szurubooru\Validator::class); + $this->configMock = $this->mockConfig(); $this->passwordServiceMock = $this->mock(\Szurubooru\Services\PasswordService::class); $this->timeServiceMock = $this->mock(\Szurubooru\Services\TimeService::class); $this->tokenServiceMock = $this->mock(\Szurubooru\Services\TokenService::class); @@ -20,12 +20,13 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase public function testInvalidPassword() { + $this->configMock->set('security/needEmailActivationToRegister', false); $this->passwordServiceMock->method('getHash')->willReturn('unmatchingHash'); $testUser = new \Szurubooru\Entities\User(); $testUser->name = 'dummy'; $testUser->passwordHash = 'hash'; - $this->userServiceMock->expects($this->once())->method('getByName')->willReturn($testUser); + $this->userServiceMock->expects($this->once())->method('getByNameOrEmail')->willReturn($testUser); $authService = $this->getAuthService(); $this->setExpectedException(\Exception::class, 'Specified password is invalid'); @@ -34,17 +35,19 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase public function testValidCredentials() { + $this->configMock->set('security/needEmailActivationToRegister', false); $this->passwordServiceMock->method('getHash')->willReturn('hash'); $testUser = new \Szurubooru\Entities\User(); + $testUser->id = 'an unusual database identifier'; $testUser->name = 'dummy'; $testUser->passwordHash = 'hash'; - $this->userServiceMock->expects($this->once())->method('getByName')->willReturn($testUser); + $this->userServiceMock->expects($this->once())->method('getByNameOrEmail')->willReturn($testUser); $testToken = new \Szurubooru\Entities\Token(); $testToken->name = 'mummy'; $this->tokenServiceMock->expects($this->once())->method('createAndSaveToken')->with( - $testUser, + $testUser->id, \Szurubooru\Entities\Token::PURPOSE_LOGIN)->willReturn($testToken); $authService = $this->getAuthService(); @@ -56,8 +59,28 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase $this->assertEquals('mummy', $authService->getLoginToken()->name); } + public function testValidCredentialsUnconfirmedEmail() + { + $this->configMock->set('security/needEmailActivationToRegister', true); + $this->passwordServiceMock->method('getHash')->willReturn('hash'); + + $testUser = new \Szurubooru\Entities\User(); + $testUser->name = 'dummy'; + $testUser->passwordHash = 'hash'; + $this->userServiceMock->expects($this->once())->method('getByNameOrEmail')->willReturn($testUser); + + $this->setExpectedException(\Exception::class, 'User didn\'t confirm mail yet'); + $authService = $this->getAuthService(); + $authService->loginFromCredentials('dummy', 'godzilla'); + + $this->assertFalse($authService->isLoggedIn()); + $this->assertNull($testUser, $authService->getLoggedInUser()); + $this->assertNull($authService->getLoginToken()); + } + public function testInvalidToken() { + $this->configMock->set('security/needEmailActivationToRegister', false); $this->tokenServiceMock->expects($this->once())->method('getByName')->willReturn(null); $this->setExpectedException(\Exception::class); @@ -67,6 +90,7 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase public function testValidToken() { + $this->configMock->set('security/needEmailActivationToRegister', false); $testUser = new \Szurubooru\Entities\User(); $testUser->id = 5; $testUser->name = 'dummy'; @@ -87,10 +111,51 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase $this->assertEquals('dummy_token', $authService->getLoginToken()->name); } + public function testValidTokenInvalidPurpose() + { + $this->configMock->set('security/needEmailActivationToRegister', false); + $testToken = new \Szurubooru\Entities\Token(); + $testToken->name = 'dummy_token'; + $testToken->additionalData = 'whatever'; + $testToken->purpose = null; + $this->tokenServiceMock->expects($this->once())->method('getByName')->willReturn($testToken); + + $this->setExpectedException(\Exception::class, 'This token is not a login token'); + $authService = $this->getAuthService(); + $authService->loginFromToken($testToken->name); + + $this->assertFalse($authService->isLoggedIn()); + $this->assertNull($authService->getLoggedInUser()); + $this->assertNull($authService->getLoginToken()); + } + + public function testValidTokenUnconfirmedEmail() + { + $this->configMock->set('security/needEmailActivationToRegister', true); + $testUser = new \Szurubooru\Entities\User(); + $testUser->id = 5; + $testUser->name = 'dummy'; + $this->userServiceMock->expects($this->once())->method('getById')->willReturn($testUser); + + $testToken = new \Szurubooru\Entities\Token(); + $testToken->name = 'dummy_token'; + $testToken->additionalData = $testUser->id; + $testToken->purpose = \Szurubooru\Entities\Token::PURPOSE_LOGIN; + $this->tokenServiceMock->expects($this->once())->method('getByName')->willReturn($testToken); + + $this->setExpectedException(\Exception::class, 'User didn\'t confirm mail yet'); + $authService = $this->getAuthService(); + $authService->loginFromToken($testToken->name); + + $this->assertFalse($authService->isLoggedIn()); + $this->assertNull($testUser, $authService->getLoggedInUser()); + $this->assertNull($authService->getLoginToken()); + } + private function getAuthService() { return new \Szurubooru\Services\AuthService( - $this->validatorMock, + $this->configMock, $this->passwordServiceMock, $this->timeServiceMock, $this->tokenServiceMock, diff --git a/tests/Services/PasswordServiceTest.php b/tests/Services/PasswordServiceTest.php new file mode 100644 index 00000000..0cc9a70e --- /dev/null +++ b/tests/Services/PasswordServiceTest.php @@ -0,0 +1,47 @@ +mockConfig(); + $passwordService = new \Szurubooru\Services\PasswordService($configMock); + + $sampleCount = 10000; + $distribution = []; + for ($i = 0; $i < $sampleCount; $i ++) + { + $password = $passwordService->getRandomPassword(); + for ($j = 0; $j < strlen($password); $j ++) + { + $c = $password{$j}; + if (!isset($distribution[$j])) + $distribution[$j] = [$c => 1]; + elseif (!isset($distribution[$j][$c])) + $distribution[$j][$c] = 1; + else + $distribution[$j][$c] ++; + } + } + + foreach ($distribution as $index => $characterDistribution) + { + $this->assertLessThan(10, $this->getRelativeStandardDeviation($characterDistribution)); + } + } + + private function getStandardDeviation($sample) + { + $mean = array_sum($sample) / count($sample); + foreach ($sample as $key => $num) + $devs[$key] = pow($num - $mean, 2); + return sqrt(array_sum($devs) / (count($devs))); + } + + private function getRelativeStandardDeviation($sample) + { + $mean = array_sum($sample) / count($sample); + return 100 * $this->getStandardDeviation($sample) / $mean; + } +} diff --git a/tests/Services/PrivilegeServiceTest.php b/tests/Services/PrivilegeServiceTest.php index 7d8af13a..2c60bb2f 100644 --- a/tests/Services/PrivilegeServiceTest.php +++ b/tests/Services/PrivilegeServiceTest.php @@ -27,7 +27,7 @@ class PrivilegeServiceTest extends \Szurubooru\Tests\AbstractTestCase $this->assertTrue($privilegeService->hasPrivilege($privilege)); } - public function testIsLoggedInByString() + public function testIsLoggedInByNameString() { $testUser1 = new \Szurubooru\Entities\User(); $testUser1->name = 'dummy'; @@ -40,6 +40,21 @@ class PrivilegeServiceTest extends \Szurubooru\Tests\AbstractTestCase $this->assertFalse($privilegeService->isLoggedIn($testUser2->name)); } + public function testIsLoggedInByEmailString() + { + $testUser1 = new \Szurubooru\Entities\User(); + $testUser1->name = 'user1'; + $testUser1->email = 'dummy'; + $testUser2 = new \Szurubooru\Entities\User(); + $testUser2->name = 'user2'; + $testUser2->email = 'godzilla'; + $this->authServiceMock->method('getLoggedInUser')->willReturn($testUser1); + + $privilegeService = $this->getPrivilegeService(); + $this->assertTrue($privilegeService->isLoggedIn($testUser1->email)); + $this->assertFalse($privilegeService->isLoggedIn($testUser2->email)); + } + public function testIsLoggedInByUser() { $testUser1 = new \Szurubooru\Entities\User(); diff --git a/tests/Services/UserServiceTest.php b/tests/Services/UserServiceTest.php index 9661e0d5..cb9527d0 100644 --- a/tests/Services/UserServiceTest.php +++ b/tests/Services/UserServiceTest.php @@ -11,6 +11,7 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase private $emailServiceMock; private $fileServiceMock; private $timeServiceMock; + private $tokenServiceMock; public function setUp() { @@ -22,6 +23,7 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase $this->emailServiceMock = $this->mock(\Szurubooru\Services\EmailService::class); $this->fileServiceMock = $this->mock(\Szurubooru\Services\FileService::class); $this->timeServiceMock = $this->mock(\Szurubooru\Services\TimeService::class); + $this->tokenServiceMock = $this->mock(\Szurubooru\Services\TokenService::class); } public function testGettingByName() @@ -78,23 +80,56 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase $this->assertEquals($expected, $actual); } - public function testValidRegistration() + public function testValidRegistrationWithoutMailActivation() { $formData = new \Szurubooru\FormData\RegistrationFormData; $formData->userName = 'user'; $formData->password = 'password'; - $formData->email = 'email'; + $formData->email = 'human@people.gov'; + $this->configMock->set('security/needEmailActivationToRegister', false); $this->passwordServiceMock->method('getHash')->willReturn('hash'); $this->timeServiceMock->method('getCurrentTime')->willReturn('now'); $this->userDaoMock->method('hasAnyUsers')->willReturn(true); $this->userDaoMock->method('save')->will($this->returnArgument(0)); + $this->emailServiceMock->expects($this->never())->method('sendActivationEmail'); $userService = $this->getUserService(); $savedUser = $userService->createUser($formData); $this->assertEquals('user', $savedUser->name); - $this->assertEquals('email', $savedUser->email); + $this->assertEquals('human@people.gov', $savedUser->email); + $this->assertNull($savedUser->emailUnconfirmed); + $this->assertEquals('hash', $savedUser->passwordHash); + $this->assertEquals(\Szurubooru\Entities\User::ACCESS_RANK_REGULAR_USER, $savedUser->accessRank); + $this->assertEquals('now', $savedUser->registrationTime); + } + + public function testValidRegistrationWithMailActivation() + { + $formData = new \Szurubooru\FormData\RegistrationFormData; + $formData->userName = 'user'; + $formData->password = 'password'; + $formData->email = 'human@people.gov'; + + $this->configMock->set('security/needEmailActivationToRegister', true); + $this->passwordServiceMock->method('getHash')->willReturn('hash'); + $this->timeServiceMock->method('getCurrentTime')->willReturn('now'); + $this->userDaoMock->method('hasAnyUsers')->willReturn(true); + $this->userDaoMock->method('save')->will($this->returnArgument(0)); + + $testToken = new \Szurubooru\Entities\Token(); + $this->tokenServiceMock->expects($this->once())->method('createAndSaveToken')->willReturn($testToken); + $this->emailServiceMock->expects($this->once())->method('sendActivationEmail')->with( + $this->anything(), + $testToken); + + $userService = $this->getUserService(); + $savedUser = $userService->createUser($formData); + + $this->assertEquals('user', $savedUser->name); + $this->assertNull($savedUser->email); + $this->assertEquals('human@people.gov', $savedUser->emailUnconfirmed); $this->assertEquals('hash', $savedUser->passwordHash); $this->assertEquals(\Szurubooru\Entities\User::ACCESS_RANK_REGULAR_USER, $savedUser->accessRank); $this->assertEquals('now', $savedUser->registrationTime); @@ -107,6 +142,7 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase $formData->password = 'password'; $formData->email = 'email'; + $this->configMock->set('security/needEmailActivationToRegister', false); $this->userDaoMock->method('hasAnyUsers')->willReturn(false); $this->userDaoMock->method('save')->will($this->returnArgument(0)); @@ -143,6 +179,7 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase $this->passwordServiceMock, $this->emailServiceMock, $this->fileServiceMock, - $this->timeServiceMock); + $this->timeServiceMock, + $this->tokenServiceMock); } }