Added IValidatable; moved validation to FormData

I still struggle to find out how to deal with arguments like
$userNameOrEmail. Should I trim() them in controllers, or in service?
If I do it in service, shouldn't all of such validation belong in there?
This commit is contained in:
Marcin Kurczewski 2014-09-09 19:38:16 +02:00
parent e6c7ed7e11
commit c117367974
16 changed files with 170 additions and 81 deletions

View file

@ -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(

View file

@ -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) {

View file

@ -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
{

View file

@ -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);
}
}

View file

@ -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;

View file

@ -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);
}
}
}

View file

@ -0,0 +1,21 @@
<?php
namespace Szurubooru\FormData;
class LoginFormData implements \Szurubooru\IValidatable
{
public $userNameOrEmail;
public $password;
public function __construct($inputReader = null)
{
if ($inputReader !== null)
{
$this->userNameOrEmail = trim($inputReader->userNameOrEmail);
$this->password = $inputReader->password;
}
}
public function validate(\Szurubooru\Validator $validator)
{
}
}

View file

@ -1,7 +1,7 @@
<?php
namespace Szurubooru\FormData;
class RegistrationFormData
class RegistrationFormData implements \Szurubooru\IValidatable
{
public $userName;
public $password;
@ -11,9 +11,16 @@ class RegistrationFormData
{
if ($inputReader !== null)
{
$this->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);
}
}

View file

@ -1,7 +1,7 @@
<?php
namespace Szurubooru\FormData;
class SearchFormData
class SearchFormData implements \Szurubooru\IValidatable
{
public $query;
public $order;
@ -11,9 +11,14 @@ class SearchFormData
{
if ($inputReader !== null)
{
$this->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);
}
}

View file

@ -1,7 +1,7 @@
<?php
namespace Szurubooru\FormData;
class UserEditFormData
class UserEditFormData implements \Szurubooru\IValidatable
{
public $userName;
public $email;
@ -23,4 +23,24 @@ class UserEditFormData
$this->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.');
}
}
}

7
src/IValidatable.php Normal file
View file

@ -0,0 +1,7 @@
<?php
namespace Szurubooru;
interface IValidatable
{
public function validate(\Szurubooru\Validator $validator);
}

View file

@ -43,12 +43,12 @@ class AuthService
return $this->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.');

View file

@ -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

View file

@ -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')

View file

@ -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);
}
}

View file

@ -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());