From 143aac93e6deadad918e43bbb829595e1013ca08 Mon Sep 17 00:00:00 2001 From: nuxsmin Date: Mon, 23 Jul 2018 00:45:08 +0200 Subject: [PATCH] * [ADD] Unit testing. Work in progress * [MOD] Code refactoring --- lib/SP/Services/Service.php | 25 +++ lib/SP/Services/User/UserPassService.php | 44 ++--- test/Services/Track/TrackServiceTest.php | 3 - test/Services/User/UserPassServiceTest.php | 220 +++++++++++++++++++++ test/res/datasets/syspass_user.xml | 111 +++++++++++ 5 files changed, 378 insertions(+), 25 deletions(-) create mode 100644 test/Services/User/UserPassServiceTest.php create mode 100644 test/res/datasets/syspass_user.xml diff --git a/lib/SP/Services/Service.php b/lib/SP/Services/Service.php index fff2b069..f1427c02 100644 --- a/lib/SP/Services/Service.php +++ b/lib/SP/Services/Service.php @@ -28,6 +28,7 @@ use Defuse\Crypto\Exception\CryptoException; use DI\Container; use Psr\Container\ContainerInterface; use SP\Config\Config; +use SP\Core\Context\ContextException; use SP\Core\Context\ContextInterface; use SP\Core\Context\SessionContext; use SP\Core\Crypt\Session; @@ -143,4 +144,28 @@ abstract class Service throw new ServiceException(__u('Error ol obtener la clave maestra del contexto')); } } + + /** + * @param string $masterPass + * + * @throws ServiceException + */ + protected final function setMasterKeyInContext(string $masterPass) + { + try { + if ($this->context instanceof SessionContext) { + Session::saveSessionKey($masterPass, $this->context); + } else { + $this->context->setTrasientKey('_masterpass', $masterPass); + } + } catch (ContextException $e) { + debugLog($e->getMessage()); + + throw new ServiceException(__u('Error ol establecer la clave maestra en el contexto')); + } catch (CryptoException $e) { + debugLog($e->getMessage()); + + throw new ServiceException(__u('Error ol establecer la clave maestra en el contexto')); + } + } } \ No newline at end of file diff --git a/lib/SP/Services/User/UserPassService.php b/lib/SP/Services/User/UserPassService.php index 98a0d665..775fd8c3 100644 --- a/lib/SP/Services/User/UserPassService.php +++ b/lib/SP/Services/User/UserPassService.php @@ -29,9 +29,9 @@ use SP\Config\Config; use SP\Config\ConfigData; use SP\Core\Crypt\Crypt; use SP\Core\Crypt\Hash; -use SP\Core\Crypt\Session as CryptSession; use SP\Core\Exceptions\SPException; use SP\DataModel\UserLoginData; +use SP\Repositories\NoSuchItemException; use SP\Repositories\User\UserRepository; use SP\Services\Config\ConfigService; use SP\Services\Service; @@ -94,13 +94,13 @@ class UserPassService extends Service * Comprueba la clave maestra del usuario. * * @param UserLoginData $userLoginData - * @param string $key Clave de cifrado + * @param string $userPass Clave de cifrado * * @return UserPassResponse * @throws SPException * @throws \Psr\Container\ContainerExceptionInterface */ - public function loadUserMPass(UserLoginData $userLoginData, $key = null) + public function loadUserMPass(UserLoginData $userLoginData, $userPass = null) { $userLoginResponse = $userLoginData->getUserLoginResponse(); @@ -117,23 +117,22 @@ class UserPassService extends Service return new UserPassResponse(self::MPASS_CHANGED); } - // FIXME // if ($userLoginResponse->getIsMigrate() === 1) { -// return UpgradeUser::upgradeMasterKey($userLoginData, $this) ? new UserPassResponse(self::MPASS_OK) : new UserPassResponse(self::MPASS_WRONG); +// $key = $this->makeKeyForUserOld($userLoginData->getLoginUser(), $userPass ?: $userLoginData->getLoginPass()); // } - if ($key === null && $userLoginResponse->getIsChangedPass() === 1) { + if ($userPass === null && $userLoginResponse->getIsChangedPass() === 1) { return new UserPassResponse(self::MPASS_CHECKOLD); } try { - $key = self::makeKeyForUser($userLoginData->getLoginUser(), $key ?: $userLoginData->getLoginPass(), $this->configData); + $key = $this->makeKeyForUser($userLoginData->getLoginUser(), $userPass ?: $userLoginData->getLoginPass()); $clearMPass = Crypt::decrypt($userLoginResponse->getMPass(), $userLoginResponse->getMKey(), $key); // Comprobamos el hash de la clave del usuario con la guardada if (Hash::checkHashKey($clearMPass, $configHashMPass)) { - CryptSession::saveSessionKey($clearMPass, $this->context); + $this->setMasterKeyInContext($clearMPass); $response = new UserPassResponse(self::MPASS_OK, $clearMPass); $response->setCryptMasterPass($userLoginResponse->getMPass()); @@ -151,15 +150,19 @@ class UserPassService extends Service /** * Obtener una clave de cifrado basada en la clave del usuario y un salt. * - * @param string $userLogin - * @param string $userPass - * @param ConfigData $configData + * @param string $userLogin + * @param string $userPass * * @return string con la clave de cifrado */ - public static function makeKeyForUser($userLogin, $userPass, ConfigData $configData) + public function makeKeyForUser($userLogin, $userPass) { - return $userPass . $userLogin . $configData->getPasswordSalt(); + // Use always the most recent config data + if (Config::getTimeUpdated() > $this->configData->getConfigDate()) { + return trim($userPass . $userLogin . $this->config->getConfigData()->getPasswordSalt()); + } else { + return trim($userPass . $userLogin . $this->configData->getPasswordSalt()); + } } /** @@ -195,7 +198,7 @@ class UserPassService extends Service $this->userRepository->updateMasterPassById($userData->getId(), $response->getCryptMasterPass(), $response->getCryptSecuredKey()); - CryptSession::saveSessionKey($userMPass, $this->context); + $this->setMasterKeyInContext($userMPass); return $response; } @@ -216,12 +219,7 @@ class UserPassService extends Service */ public function createMasterPass($masterPass, $userLogin, $userPass) { - // Use always the most recent config data - if (Config::getTimeUpdated() > $this->configData->getConfigDate()) { - $key = self::makeKeyForUser($userLogin, $userPass, $this->config->getConfigData()); - } else { - $key = self::makeKeyForUser($userLogin, $userPass, $this->configData); - } + $key = $this->makeKeyForUser($userLogin, $userPass); $securedKey = Crypt::makeSecuredKey($key); $cryptMPass = Crypt::encrypt($masterPass, $securedKey, $key); @@ -245,13 +243,15 @@ class UserPassService extends Service * @param int $id * @param string $userPass * - * @return bool * @throws \SP\Core\Exceptions\ConstraintException * @throws \SP\Core\Exceptions\QueryException + * @throws NoSuchItemException */ public function migrateUserPassById($id, $userPass) { - return $this->userRepository->updatePassById($id, new UpdatePassRequest(Hash::hashKey($userPass))); + if ($this->userRepository->updatePassById($id, new UpdatePassRequest(Hash::hashKey($userPass))) === 0) { + throw new NoSuchItemException(__u('El usuario no existe')); + } } /** diff --git a/test/Services/Track/TrackServiceTest.php b/test/Services/Track/TrackServiceTest.php index 3cf3d9f0..2430b1b9 100644 --- a/test/Services/Track/TrackServiceTest.php +++ b/test/Services/Track/TrackServiceTest.php @@ -25,7 +25,6 @@ namespace SP\Tests\Services\Track; use SP\DataModel\TrackData; -use SP\Http\Request; use SP\Repositories\NoSuchItemException; use SP\Repositories\Track\TrackRequest; use SP\Services\Track\TrackService; @@ -40,7 +39,6 @@ use function SP\Test\setupContext; */ class TrackServiceTest extends DatabaseTestCase { - private static $request; /** * @var TrackService */ @@ -62,7 +60,6 @@ class TrackServiceTest extends DatabaseTestCase // Inicializar el servicio self::$service = $dic->get(TrackService::class); - self::$request = $dic->get(Request::class); } /** diff --git a/test/Services/User/UserPassServiceTest.php b/test/Services/User/UserPassServiceTest.php new file mode 100644 index 00000000..9c1df8d5 --- /dev/null +++ b/test/Services/User/UserPassServiceTest.php @@ -0,0 +1,220 @@ +. + */ + +namespace SP\Tests\Services\User; + +use SP\Core\Crypt\Crypt; +use SP\DataModel\UserLoginData; +use SP\Repositories\NoSuchItemException; +use SP\Services\User\UserLoginResponse; +use SP\Services\User\UserPassService; +use SP\Services\User\UserService; +use SP\Storage\Database\DatabaseConnectionData; +use SP\Test\DatabaseTestCase; +use function SP\Test\setupContext; + +/** + * Class UserPassServiceTest + * + * @package SP\Tests\Services\User + */ +class UserPassServiceTest extends DatabaseTestCase +{ + const CURRENT_MASTERPASS = '12345678900'; + const NEW_MASTERPASS = '00123456789'; + + /** + * @var \Closure + */ + private static $getUserLoginResponse; + + /** + * @var UserPassService + */ + private static $service; + + /** + * @throws \DI\NotFoundException + * @throws \SP\Core\Context\ContextException + * @throws \DI\DependencyException + * @throws \SP\Core\Exceptions\SPException + */ + public static function setUpBeforeClass() + { + $dic = setupContext(); + + self::$dataset = 'syspass_user.xml'; + + // Datos de conexión a la BBDD + self::$databaseConnectionData = $dic->get(DatabaseConnectionData::class); + + // Inicializar el servicio + self::$service = $dic->get(UserPassService::class); + + self::$getUserLoginResponse = function ($login) use ($dic) { + return UserService::mapUserLoginResponse($dic->get(UserService::class)->getByLogin($login)); + }; + } + + /** + * @throws \Defuse\Crypto\Exception\CryptoException + * @throws \SP\Core\Exceptions\SPException + */ + public function testCreateMasterPass() + { + $result = self::$service->createMasterPass(self::NEW_MASTERPASS, 'admin', 'test123'); + + $key = self::$service->makeKeyForUser('admin', 'test123'); + + $this->assertEquals(self::NEW_MASTERPASS, Crypt::decrypt($result->getCryptMasterPass(), $result->getCryptSecuredKey(), $key)); + } + + /** + * @throws \SP\Core\Exceptions\ConstraintException + * @throws \SP\Core\Exceptions\QueryException + * @throws \SP\Repositories\NoSuchItemException + */ + public function testMigrateUserPassById() + { + self::$service->migrateUserPassById(2, '123'); + + $this->expectException(NoSuchItemException::class); + + self::$service->migrateUserPassById(10, '123'); + } + + /** + * @throws \Defuse\Crypto\Exception\CryptoException + * @throws \SP\Core\Exceptions\SPException + */ + public function testUpdateMasterPassFromOldPass() + { + $data = new UserLoginData(); + $data->setLoginUser('admin'); + $data->setLoginPass('debian'); + $data->setUserLoginResponse(self::$getUserLoginResponse->call($this, $data->getLoginUser())); + + $result = self::$service->updateMasterPassFromOldPass('debian', $data); + + $this->assertEquals(UserPassService::MPASS_OK, $result->getStatus()); + + $result = self::$service->updateMasterPassFromOldPass('test123', $data); + + $this->assertEquals(UserPassService::MPASS_WRONG, $result->getStatus()); + } + + /** + * @throws \SP\Core\Exceptions\SPException + */ + public function testLoadUserMPass() + { + $data = new UserLoginData(); + $data->setLoginUser('admin'); + $data->setLoginPass('debian'); + $data->setUserLoginResponse(self::$getUserLoginResponse->call($this, $data->getLoginUser())); + + $result = self::$service->loadUserMPass($data); + + $this->assertEquals(UserPassService::MPASS_OK, $result->getStatus()); + + $result = self::$service->loadUserMPass($data, 'test123'); + + $this->assertEquals(UserPassService::MPASS_CHECKOLD, $result->getStatus()); + } + + /** + * @throws \SP\Core\Exceptions\SPException + */ + public function testLoadUserMPassOutdated() + { + $data = new UserLoginData(); + $data->setLoginUser('admin'); + $data->setLoginPass('debian'); + + /** @var UserLoginResponse $userData */ + $userData = self::$getUserLoginResponse->call($this, $data->getLoginUser()); + $userData->setLastUpdateMPass(1521887150); + + $data->setUserLoginResponse($userData); + + $result = self::$service->loadUserMPass($data); + + $this->assertEquals(UserPassService::MPASS_CHANGED, $result->getStatus()); + } + + /** + * @throws \SP\Core\Exceptions\SPException + */ + public function testLoadUserMPassChangedPass() + { + $data = new UserLoginData(); + $data->setLoginUser('admin'); + $data->setLoginPass('debian'); + + /** @var UserLoginResponse $userData */ + $userData = self::$getUserLoginResponse->call($this, $data->getLoginUser()); + $userData->setIsChangedPass(true); + + $data->setUserLoginResponse($userData); + + $result = self::$service->loadUserMPass($data); + + $this->assertEquals(UserPassService::MPASS_CHECKOLD, $result->getStatus()); + } + + /** + * @throws \SP\Core\Exceptions\SPException + */ + public function testLoadUserMPassNotSet() + { + $data = new UserLoginData(); + $data->setLoginUser('admin'); + $data->setLoginPass('debian'); + $data->setUserLoginResponse(new UserLoginResponse()); + + $result = self::$service->loadUserMPass($data); + + $this->assertEquals(UserPassService::MPASS_NOTSET, $result->getStatus()); + } + + /** + * @throws \Defuse\Crypto\Exception\CryptoException + * @throws \SP\Core\Exceptions\SPException + */ + public function testUpdateMasterPassOnLogin() + { + $data = new UserLoginData(); + $data->setLoginUser('admin'); + $data->setLoginPass('test123'); + $data->setUserLoginResponse(self::$getUserLoginResponse->call($this, $data->getLoginUser())); + + $result = self::$service->updateMasterPassOnLogin(self::CURRENT_MASTERPASS, $data); + + $this->assertEquals(UserPassService::MPASS_OK, $result->getStatus()); + + $result = self::$service->updateMasterPassOnLogin(self::NEW_MASTERPASS, $data); + + $this->assertEquals(UserPassService::MPASS_WRONG, $result->getStatus()); + } +} diff --git a/test/res/datasets/syspass_user.xml b/test/res/datasets/syspass_user.xml new file mode 100644 index 00000000..da63aaf2 --- /dev/null +++ b/test/res/datasets/syspass_user.xml @@ -0,0 +1,111 @@ + + + + + + 1 + sysPass Admin + 1 + admin + + 2432792431302432584B666F627854545234444E4A7956573748365165774153356E5234434E4B7748746A4A614362545333486D72316B37485A4E4F + 64656635303230306339373130623861363837613161346136323261333134613936303034326531646638643662323838326537383264636261653237326662346562386138326665613134386165666637343132663537363035663034363135623633623961616239303266333933613863323439386539613734343061356337333937326131653663333766326532306136643766356266383137653965376465363438633738663034323333386230303666353461643039363437 + 6465663130303030646566353032303033633166653138613634613233646466323436333064626636616335306634376539356630653063303335636231353938343362626638363932323138366536613330333839663735333832376532356464333436633035393730316433303539333531376233633434386535303539356666663630326263373037373564363330623439313539363862333263353262643961366564306165336439623764626564653633633163346536613330383936616230303639613139356431333736623231333232663766663765386632623838336432363062353835666430393133323631346239303266646463393061313532376638313639623138356563666438653130303435393864623134643464666263393232336432653833386665313333626138393761643532383139666264396336326333306637343964333564313236386464663365383137643339333036616365343964636130316531376163306432616161646362346263613439333763386564656261396139313962363433363833616163333234316330393734343962613033383962616238303432653339656666666332316264643163636436343464643362646565346335666564316164616132653537666537656166353964306331303865396664626135323735396366353265306430336536 + + + 100 + 1 + 2018-07-22 22:23:11 + + 1532297701 + 1 + 0 + 0 + 0 + + 0 + 0 + 0 + 4F3A33323A2253505C446174614D6F64656C5C55736572507265666572656E63657344617461223A31303A7B733A373A22757365725F6964223B693A313B733A363A22757365324661223B623A303B733A343A226C616E67223B733A353A2265735F4553223B733A353A227468656D65223B733A31333A226D6174657269616C2D626C7565223B733A31343A22726573756C747350657250616765223B693A363B733A31313A226163636F756E744C696E6B223B623A303B733A393A22736F72745669657773223B623A303B733A393A22746F704E6176626172223B623A303B733A31353A226F7074696F6E616C416374696F6E73223B623A303B733A31343A22726573756C747341734361726473223B623A303B7D + + + 2 + sysPass demo + 2 + demo + demo + 2432792431302454726E69756C5763754361433635346F76566F35392E766B4C5433414E31624A6D726A79553462696335325069436A6B5572396669 + 64656635303230303231616533353730373263373165626239393534353966366236636164373235336534316336633534353036336339326136653730616366333930393165373934613865376662386662326664333931383932363562396466303133333631623063323732323339653465373165343839313030646534326265633737623966343238396635633936613837646531343864313963653663643338613131343932623163313765653630326430623532343564346566 + 6465663130303030646566353032303035643534316262633462653032333563313338626561366561333536626436663037353365313035653030333563653166316235336534663364343565366262353335626163396639646538653131316262356334383865336535633637323333666632626365313837626335386135353839373535373034386564353634366361646638623736396132323164363032353435653034306264613135663138323638383665373536313236353361313037306530333261323365636364336339616438323162306363383962643130333035303931653965626332653935313465656631373462663339343664656132393661346262366264343463646333363361643335623032373561356633323430313936346531633131663937313764313139633130633561373161666332356365346534366661623234646663626362326237303964336335316532623834326464303933653230353965373265356638376363366236626239306231346265376264373637663163303937366231313362393630613265636565336633313131663538656131346139353736623332653163303962636435313366383733656664653062373333366238643464646637616237323333373038613264393965633738356139393036306135643262316366306262663739346262663765 + demo@syspass.org + aaaa + 12 + 2 + 2018-04-01 21:29:47 + 2018-04-14 08:47:43 + 1522582852 + 0 + 0 + 0 + 0 + + 0 + 0 + 0 + + + + 3 + User A + 2 + user_a + user_a + 2432792431302469444B442E2F4F624D79742E6F43594F5249514D5065624454783966744D636A703034365A435976662E765479597A594F6A4C472E + + + user_a@syspass.org + + 0 + 1 + + 2018-04-14 08:48:08 + 0 + 0 + 0 + 0 + 0 + + 0 + 0 + 0 + + + + 4 + User B + 2 + user_b + + 243279243130244C37643658736A663955794F6E583662472E6F384E4F713961674B6F64536B4B5674485350462F6861414E657971517065372E6532 + + + user_b@syspass.org + + 0 + 1 + + 2018-03-30 18:38:32 + 0 + 0 + 0 + 0 + 0 + + 0 + 0 + 0 + + + + +