diff --git a/public_html/js/Auth.js b/public_html/js/Auth.js index 3b3f5044..aa514004 100644 --- a/public_html/js/Auth.js +++ b/public_html/js/Auth.js @@ -26,9 +26,9 @@ App.Auth = function(_, jQuery, util, api, appState, promise) { listTags: 'listTags', }; - function loginFromCredentials(userName, password, remember) { + function loginFromCredentials(userNameOrEmail, password, remember) { return promise.make(function(resolve, reject) { - promise.wait(api.post('/login', {userName: userName, password: password})) + promise.wait(api.post('/login', {userNameOrEmail: userNameOrEmail, password: password})) .then(function(response) { updateAppState(response); jQuery.cookie( diff --git a/public_html/js/Presenters/LoginPresenter.js b/public_html/js/Presenters/LoginPresenter.js index 466189b1..71df63c7 100644 --- a/public_html/js/Presenters/LoginPresenter.js +++ b/public_html/js/Presenters/LoginPresenter.js @@ -38,11 +38,11 @@ App.Presenters.LoginPresenter = function( e.preventDefault(); messagePresenter.hideMessages($messages); - var userName = $el.find('[name=user]').val(); + var userNameOrEmail = $el.find('[name=user]').val(); var password = $el.find('[name=password]').val(); var remember = $el.find('[name=remember]').val(); - if (userName.length === 0) { + if (userNameOrEmail.length === 0) { messagePresenter.showError($messages, 'User name cannot be empty.'); return false; } @@ -52,7 +52,7 @@ App.Presenters.LoginPresenter = function( return false; } - auth.loginFromCredentials(userName, password, remember) + auth.loginFromCredentials(userNameOrEmail, password, remember) .then(function(response) { router.navigateToMainPage(); }).fail(function(response) { diff --git a/src/Controllers/AuthController.php b/src/Controllers/AuthController.php index 85cd8ae5..01d250d4 100644 --- a/src/Controllers/AuthController.php +++ b/src/Controllers/AuthController.php @@ -4,6 +4,7 @@ namespace Szurubooru\Controllers; final class AuthController extends AbstractController { private $authService; + private $tokenService; private $privilegeService; private $inputReader; private $userViewProxy; @@ -11,12 +12,14 @@ final class AuthController extends AbstractController public function __construct( \Szurubooru\Services\AuthService $authService, + \Szurubooru\Services\TokenService $tokenService, \Szurubooru\Services\PrivilegeService $privilegeService, \Szurubooru\Helpers\InputReader $inputReader, \Szurubooru\Controllers\ViewProxies\UserViewProxy $userViewProxy, \Szurubooru\Controllers\ViewProxies\TokenViewProxy $tokenViewProxy) { $this->authService = $authService; + $this->tokenService = $tokenService; $this->privilegeService = $privilegeService; $this->inputReader = $inputReader; $this->userViewProxy = $userViewProxy; @@ -33,11 +36,13 @@ final class AuthController extends AbstractController { if (isset($this->inputReader->userName) and isset($this->inputReader->password)) { - $this->authService->loginFromCredentials($this->inputReader->userName, $this->inputReader->password); + $formData = new \Szurubooru\FormData\LoginFormData($this->inputReader); + $this->authService->loginFromCredentials($formData); } elseif (isset($this->inputReader->token)) { - $this->authService->loginFromToken($this->inputReader->token); + $token = $this->tokenService->getByName($this->inputReader->token); + $this->authService->loginFromToken($token); } else { diff --git a/src/Controllers/UserController.php b/src/Controllers/UserController.php index 885903ff..32d435aa 100644 --- a/src/Controllers/UserController.php +++ b/src/Controllers/UserController.php @@ -5,17 +5,20 @@ final class UserController extends AbstractController { private $privilegeService; private $userService; + private $tokenService; private $inputReader; private $userViewProxy; public function __construct( \Szurubooru\Services\PrivilegeService $privilegeService, \Szurubooru\Services\UserService $userService, + \Szurubooru\Services\TokenService $tokenService, \Szurubooru\Helpers\InputReader $inputReader, \Szurubooru\Controllers\ViewProxies\UserViewProxy $userViewProxy) { $this->privilegeService = $privilegeService; $this->userService = $userService; + $this->tokenService = $tokenService; $this->inputReader = $inputReader; $this->userViewProxy = $userViewProxy; } @@ -136,11 +139,13 @@ final class UserController extends AbstractController public function finishPasswordReset($tokenName) { - return ['newPassword' => $this->userService->finishPasswordReset($tokenName)]; + $token = $this->tokenService->getByName($tokenName); + return ['newPassword' => $this->userService->finishPasswordReset($token)]; } public function finishActivation($tokenName) { - $this->userService->finishActivation($tokenName); + $token = $this->tokenService->getByName($tokenName); + $this->userService->finishActivation($token); } } diff --git a/src/Dao/SearchFilter.php b/src/Dao/SearchFilter.php index 5788f4b8..a5189b30 100644 --- a/src/Dao/SearchFilter.php +++ b/src/Dao/SearchFilter.php @@ -10,7 +10,7 @@ class SearchFilter public function __construct($pageSize, \Szurubooru\FormData\SearchFormData $searchFormData = null) { - $this->pageSize = $pageSize; + $this->pageSize = intval($pageSize); if ($searchFormData) { $this->query = $searchFormData->query; diff --git a/src/Dispatcher.php b/src/Dispatcher.php index 06d019fa..79c99062 100644 --- a/src/Dispatcher.php +++ b/src/Dispatcher.php @@ -5,11 +5,13 @@ final class Dispatcher { private $router; private $authService; + private $tokenService; public function __construct( \Szurubooru\Router $router, \Szurubooru\Helpers\HttpHelper $httpHelper, \Szurubooru\Services\AuthService $authService, + \Szurubooru\Services\TokenService $tokenService, \Szurubooru\ControllerRepository $controllerRepository) { $this->router = $router; @@ -18,6 +20,7 @@ final class Dispatcher //if script fails prematurely, mark it as fail from advance $this->httpHelper->setResponseCode(500); $this->authService = $authService; + $this->tokenService = $tokenService; foreach ($controllerRepository->getControllers() as $controller) $controller->registerRoutes($router); @@ -54,8 +57,11 @@ final class Dispatcher private function authorizeFromRequestHeader() { - $loginToken = $this->httpHelper->getRequestHeader('X-Authorization-Token'); - if ($loginToken) - $this->authService->loginFromToken($loginToken); + $loginTokenName = $this->httpHelper->getRequestHeader('X-Authorization-Token'); + if ($loginTokenName) + { + $token = $this->tokenService->getByName($loginTokenName); + $this->authService->loginFromToken($token); + } } } diff --git a/src/FormData/LoginFormData.php b/src/FormData/LoginFormData.php new file mode 100644 index 00000000..8b86fa30 --- /dev/null +++ b/src/FormData/LoginFormData.php @@ -0,0 +1,21 @@ +userNameOrEmail = trim($inputReader->userNameOrEmail); + $this->password = $inputReader->password; + } + } + + public function validate(\Szurubooru\Validator $validator) + { + } +} diff --git a/src/FormData/RegistrationFormData.php b/src/FormData/RegistrationFormData.php index d355e750..06a7b318 100644 --- a/src/FormData/RegistrationFormData.php +++ b/src/FormData/RegistrationFormData.php @@ -1,7 +1,7 @@ userName = $inputReader->userName; + $this->userName = trim($inputReader->userName); $this->password = $inputReader->password; - $this->email = $inputReader->email; + $this->email = trim($inputReader->email); } } + + public function validate(\Szurubooru\Validator $validator) + { + $validator->validateUserName($this->userName); + $validator->validatePassword($this->password); + $validator->validateEmail($this->email); + } } diff --git a/src/FormData/SearchFormData.php b/src/FormData/SearchFormData.php index fc71e35b..53eaf253 100644 --- a/src/FormData/SearchFormData.php +++ b/src/FormData/SearchFormData.php @@ -1,7 +1,7 @@ query = $inputReader->query; - $this->order = $inputReader->order; - $this->pageNumber = $inputReader->page; + $this->query = trim($inputReader->query); + $this->order = trim($inputReader->order); + $this->pageNumber = intval($inputReader->page); } } + + public function validate(\Szurubooru\Validator $validator = null) + { + $validator->validateNumber($this->pageNumber); + } } diff --git a/src/FormData/UserEditFormData.php b/src/FormData/UserEditFormData.php index 4a016fe6..983d279d 100644 --- a/src/FormData/UserEditFormData.php +++ b/src/FormData/UserEditFormData.php @@ -1,7 +1,7 @@ browsingSettings = $inputReader->browsingSettings; } } + + public function validate(\Szurubooru\Validator $validator) + { + if ($this->userName !== null) + $this->validator->validateUserName($formData->userName); + + if ($formData->password !== null) + $this->validator->validatePassword($formData->password); + + if ($formData->email !== null) + $this->validator->validateEmail($formData->email); + + if ($formData->browsingSettings !== null) + { + if (!is_string($formData->browsingSettings)) + throw new \InvalidArgumentException('Browsing settings must be stringified JSON.'); + else if (strlen($formData->browsingSettings) > 2000) + throw new \InvalidArgumentException('Stringified browsing settings can have at most 2000 characters.'); + } + } } diff --git a/src/IValidatable.php b/src/IValidatable.php new file mode 100644 index 00000000..a61df0dc --- /dev/null +++ b/src/IValidatable.php @@ -0,0 +1,7 @@ +loginToken; } - public function loginFromCredentials($userNameOrEmail, $password) + public function loginFromCredentials($formData) { - $user = $this->userService->getByNameOrEmail($userNameOrEmail); - $this->validateUser($user); + $user = $this->userService->getByNameOrEmail($formData->userNameOrEmail); + $this->doFinalChecksOnUser($user); - $passwordHash = $this->passwordService->getHash($password); + $passwordHash = $this->passwordService->getHash($formData->password); if ($user->passwordHash !== $passwordHash) throw new \InvalidArgumentException('Specified password is invalid.'); @@ -57,16 +57,15 @@ class AuthService $this->userService->updateUserLastLoginTime($user); } - public function loginFromToken($loginTokenName) + public function loginFromToken(\Szurubooru\Entities\Token $token) { - $loginToken = $this->tokenService->getByName($loginTokenName); - if ($loginToken->purpose !== \Szurubooru\Entities\Token::PURPOSE_LOGIN) + if ($token->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); + $user = $this->userService->getById($token->additionalData); + $this->doFinalChecksOnUser($user); - $this->loginToken = $loginToken; + $this->loginToken = $token; $this->loggedInUser = $user; $this->userService->updateUserLastLoginTime($this->loggedInUser); } @@ -99,7 +98,7 @@ class AuthService return $this->tokenService->createAndSaveToken($user->id, \Szurubooru\Entities\Token::PURPOSE_LOGIN); } - private function validateUser($user) + private function doFinalChecksOnUser($user) { if (!$user->email and $this->config->security->needEmailActivationToRegister) throw new \DomainException('User didn\'t confirm mail yet.'); diff --git a/src/Services/UserService.php b/src/Services/UserService.php index 246346d0..a9dc9e0e 100644 --- a/src/Services/UserService.php +++ b/src/Services/UserService.php @@ -69,17 +69,14 @@ class UserService public function getFiltered(\Szurubooru\FormData\SearchFormData $formData) { - $pageSize = intval($this->config->users->usersPerPage); - $this->validator->validateNumber($formData->pageNumber); - $searchFilter = new \Szurubooru\Dao\SearchFilter($pageSize, $formData); + $this->validator->validate($formData); + $searchFilter = new \Szurubooru\Dao\SearchFilter($this->config->users->usersPerPage, $formData); return $this->userSearchService->getFiltered($searchFilter); } public function createUser(\Szurubooru\FormData\RegistrationFormData $formData) { - $this->validator->validateUserName($formData->userName); - $this->validator->validatePassword($formData->password); - $this->validator->validateEmail($formData->email); + $formData->validate($this->validator); if ($formData->email and $this->userDao->getByEmail($formData->email)) throw new \DomainException('User with this e-mail already exists.'); @@ -105,6 +102,8 @@ class UserService public function updateUser(\Szurubooru\Entities\User $user, \Szurubooru\FormData\UserEditFormData $formData) { + $this->validator->validate($formData); + if ($formData->avatarStyle !== null) { $user->avatarStyle = \Szurubooru\Helpers\EnumHelper::avatarStyleFromString($formData->avatarStyle); @@ -118,7 +117,6 @@ class UserService if ($formData->userName !== null and $formData->userName !== $user->name) { - $this->validator->validateUserName($formData->userName); $userWithThisEmail = $this->userDao->getByName($formData->userName); if ($userWithThisEmail and $userWithThisEmail->id !== $user->id) throw new \DomainException('User with this name already exists.'); @@ -128,13 +126,11 @@ class UserService if ($formData->password !== null) { - $this->validator->validatePassword($formData->password); $user->passwordHash = $this->passwordService->getHash($formData->password); } if ($formData->email !== null and $formData->email !== $user->email) { - $this->validator->validateEmail($formData->email); if ($this->userDao->getByEmail($formData->email)) throw new \DomainException('User with this e-mail already exists.'); @@ -149,16 +145,18 @@ class UserService if ($formData->browsingSettings !== null) { - if (!is_string($formData->browsingSettings)) - throw new \InvalidArgumentException('Browsing settings must be stringified JSON.'); - if (strlen($formData->browsingSettings) > 2000) - throw new \InvalidArgumentException('Stringified browsing settings can have at most 2000 characters.'); $user->browsingSettings = $formData->browsingSettings; } return $this->userDao->save($user); } + public function updateUserLastLoginTime(\Szurubooru\Entities\User $user) + { + $user->lastLoginTime = $this->timeService->getCurrentTime(); + $this->userDao->save($user); + } + public function deleteUser(\Szurubooru\Entities\User $user) { $this->userDao->deleteById($user->id); @@ -168,31 +166,14 @@ class UserService $this->thumbnailService->deleteUsedThumbnails($avatarSource); } - public function getCustomAvatarSourcePath(\Szurubooru\Entities\User $user) - { - return 'avatars' . DIRECTORY_SEPARATOR . $user->id; - } - - public function getBlankAvatarSourcePath() - { - return 'avatars' . DIRECTORY_SEPARATOR . 'blank.png'; - } - - public function updateUserLastLoginTime(\Szurubooru\Entities\User $user) - { - $user->lastLoginTime = $this->timeService->getCurrentTime(); - $this->userDao->save($user); - } - public function sendPasswordResetEmail(\Szurubooru\Entities\User $user) { $token = $this->tokenService->createAndSaveToken($user->name, \Szurubooru\Entities\Token::PURPOSE_PASSWORD_RESET); $this->emailService->sendPasswordResetEmail($user, $token); } - public function finishPasswordReset($tokenName) + public function finishPasswordReset(\Szurubooru\Entities\Token $token) { - $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.'); @@ -210,23 +191,32 @@ class UserService $this->emailService->sendActivationEmail($user, $token); } - public function finishActivation($tokenName) + public function finishActivation(\Szurubooru\Entities\Token $token) { - $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); - $user = $this->confirmEmail($user); + $user = $this->confirmUserEmail($user); $this->userDao->save($user); $this->tokenService->invalidateByName($token->name); } + public function getCustomAvatarSourcePath(\Szurubooru\Entities\User $user) + { + return 'avatars' . DIRECTORY_SEPARATOR . $user->id; + } + + public function getBlankAvatarSourcePath() + { + return 'avatars' . DIRECTORY_SEPARATOR . 'blank.png'; + } + private function sendActivationEmailIfNeeded(\Szurubooru\Entities\User $user) { if ($user->accessRank === \Szurubooru\Entities\User::ACCESS_RANK_ADMINISTRATOR or !$this->config->security->needEmailActivationToRegister) { - $user = $this->confirmEmail($user); + $user = $this->confirmUserEmail($user); } else { @@ -235,7 +225,7 @@ class UserService return $user; } - private function confirmEmail(\Szurubooru\Entities\User $user) + private function confirmUserEmail(\Szurubooru\Entities\User $user) { //security issue: //1. two users set their unconfirmed mail to godzilla@empire.gov diff --git a/src/Validator.php b/src/Validator.php index 194d2f70..079a1f7c 100644 --- a/src/Validator.php +++ b/src/Validator.php @@ -10,10 +10,15 @@ class Validator $this->config = $config; } + public function validate(\Szurubooru\IValidatable $validatable) + { + $validatable->validate($this); + } + public function validateNumber($subject) { if (!preg_match('/^-?[0-9]+$/', $subject)) - throw new \DomainException(subject . ' does not look like a number.'); + throw new \DomainException($subject . ' does not look like a number.'); } public function validateNonEmpty($subject, $subjectName = 'Object') diff --git a/tests/DispatcherTest.php b/tests/DispatcherTest.php index cbeffd43..0331282f 100644 --- a/tests/DispatcherTest.php +++ b/tests/DispatcherTest.php @@ -6,6 +6,7 @@ final class DispatcherTest extends \Szurubooru\Tests\AbstractTestCase private $routerMock; private $httpHelperMock; private $authServiceMock; + private $tokenServiceMock; private $controllerRepositoryMock; public function setUp() @@ -13,6 +14,7 @@ final class DispatcherTest extends \Szurubooru\Tests\AbstractTestCase $this->routerMock = $this->mock(\Szurubooru\Router::class); $this->httpHelperMock = $this->mock(\Szurubooru\Helpers\HttpHelper::class); $this->authServiceMock = $this->mock(\Szurubooru\Services\AuthService::class); + $this->tokenServiceMock = $this->mock(\Szurubooru\Services\TokenService::class); $this->controllerRepositoryMock = $this->mock(\Szurubooru\ControllerRepository::class); } @@ -50,12 +52,23 @@ final class DispatcherTest extends \Szurubooru\Tests\AbstractTestCase $this->assertEquals($expected, $actual); } + public function testAuthorization() + { + $this->httpHelperMock->expects($this->once())->method('getRequestHeader')->with($this->equalTo('X-Authorization-Token'))->willReturn('test'); + $this->tokenServiceMock->expects($this->once())->method('getByName'); + $this->controllerRepositoryMock->method('getControllers')->willReturn([]); + + $dispatcher = $this->getDispatcher(); + $dispatcher->run(); + } + private function getDispatcher() { return new \Szurubooru\Dispatcher( $this->routerMock, $this->httpHelperMock, $this->authServiceMock, + $this->tokenServiceMock, $this->controllerRepositoryMock); } } diff --git a/tests/Services/AuthServiceTest.php b/tests/Services/AuthServiceTest.php index 1e86b788..685ae1e6 100644 --- a/tests/Services/AuthServiceTest.php +++ b/tests/Services/AuthServiceTest.php @@ -28,9 +28,12 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase $testUser->passwordHash = 'hash'; $this->userServiceMock->expects($this->once())->method('getByNameOrEmail')->willReturn($testUser); - $authService = $this->getAuthService(); $this->setExpectedException(\Exception::class, 'Specified password is invalid'); - $authService->loginFromCredentials('dummy', 'godzilla'); + $authService = $this->getAuthService(); + $formData = new \Szurubooru\FormData\LoginFormData(); + $formData->userNameOrEmail = 'dummy'; + $formData->password = 'godzilla'; + $authService->loginFromCredentials($formData); } public function testValidCredentials() @@ -51,7 +54,10 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase \Szurubooru\Entities\Token::PURPOSE_LOGIN)->willReturn($testToken); $authService = $this->getAuthService(); - $authService->loginFromCredentials('dummy', 'godzilla'); + $formData = new \Szurubooru\FormData\LoginFormData(); + $formData->userNameOrEmail = 'dummy'; + $formData->password = 'godzilla'; + $authService->loginFromCredentials($formData); $this->assertTrue($authService->isLoggedIn()); $this->assertEquals($testUser, $authService->getLoggedInUser()); @@ -71,7 +77,10 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase $this->setExpectedException(\Exception::class, 'User didn\'t confirm mail yet'); $authService = $this->getAuthService(); - $authService->loginFromCredentials('dummy', 'godzilla'); + $formData = new \Szurubooru\FormData\LoginFormData(); + $formData->userNameOrEmail = 'dummy'; + $formData->password = 'godzilla'; + $authService->loginFromCredentials($formData); $this->assertFalse($authService->isLoggedIn()); $this->assertNull($testUser, $authService->getLoggedInUser()); @@ -81,11 +90,11 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase public function testInvalidToken() { $this->configMock->set('security/needEmailActivationToRegister', false); - $this->tokenServiceMock->expects($this->once())->method('getByName')->willReturn(null); $this->setExpectedException(\Exception::class); $authService = $this->getAuthService(); - $authService->loginFromToken(''); + $testToken = new \Szurubooru\Entities\Token(); + $authService->loginFromToken($testToken); } public function testValidToken() @@ -100,10 +109,9 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase $testToken->name = 'dummy_token'; $testToken->additionalData = $testUser->id; $testToken->purpose = \Szurubooru\Entities\Token::PURPOSE_LOGIN; - $this->tokenServiceMock->expects($this->once())->method('getByName')->willReturn($testToken); $authService = $this->getAuthService(); - $authService->loginFromToken($testToken->name); + $authService->loginFromToken($testToken); $this->assertTrue($authService->isLoggedIn()); $this->assertEquals($testUser, $authService->getLoggedInUser()); @@ -118,11 +126,10 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase $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); + $authService->loginFromToken($testToken); $this->assertFalse($authService->isLoggedIn()); $this->assertNull($authService->getLoggedInUser()); @@ -141,11 +148,10 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase $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); + $authService->loginFromToken($testToken); $this->assertFalse($authService->isLoggedIn()); $this->assertNull($testUser, $authService->getLoggedInUser());