Skip to content

Commit

Permalink
feat(security): use random_bytes to strongen hash for password recove…
Browse files Browse the repository at this point in the history 
…ry links
  • Loading branch information
@mrflos
mrflos committed Oct 31, 2024
1 parent 8929f68 commit 0e58ac9
Showing 1 changed file with 21 additions and 19 deletions.
40 changes: 21 additions & 19 deletions includes/services/UserManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface;
use Throwable;
use YesWiki\Core\Controller\AuthController;
use YesWiki\Core\Entity\User;
use YesWiki\Core\Exception\DeleteUserException;
Expand Down Expand Up @@ -63,6 +62,7 @@ private function arrayToUser(?array $userAsArray = null, bool $fillEmpty = false
}
}
}

// be carefull the User::__construct is really strict about list of properties that should set
return new User($userAsArray);
}
Expand Down Expand Up @@ -110,13 +110,13 @@ function ($userAsArray) {
* @param string email (optionnal if parameters by array)
* @param string plainPassword (optionnal if parameters by array)
*
* @throws UserNameAlreadyUsedException|UserEmailAlreadyUsedException|Exception
* @throws UserNameAlreadyUsedException|UserEmailAlreadyUsedException|\Exception
*/
public function create($wikiNameOrUser, string $email = '', string $plainPassword = '')
{
$this->userlink = '';
if ($this->securityController->isWikiHibernated()) {
throw new Exception(_t('WIKI_IN_HIBERNATION'));
throw new \Exception(_t('WIKI_IN_HIBERNATION'));
}

if (is_array($wikiNameOrUser)) {
Expand Down Expand Up @@ -147,23 +147,23 @@ public function create($wikiNameOrUser, string $email = '', string $plainPasswor
'signuptime' => '',
];
} else {
throw new Exception('First parameter of UserManager->create should be string or array!');
throw new \Exception('First parameter of UserManager->create should be string or array!');
}

if (empty($wikiName)) {
throw new Exception("'Name' parameter of UserManager->create should not be empty!");
throw new \Exception("'Name' parameter of UserManager->create should not be empty!");
}
if (!empty($this->getOneByName($wikiName))) {
throw new UserNameAlreadyUsedException();
}
if (empty($email)) {
throw new Exception("'email' parameter of UserManager->create should not be empty!");
throw new \Exception("'email' parameter of UserManager->create should not be empty!");
}
if (!empty($this->getOneByEmail($email))) {
throw new UserEmailAlreadyUsedException();
}
if (empty($plainPassword)) {
throw new Exception("'password' parameter of UserManager->create should not be empty!");
throw new \Exception("'password' parameter of UserManager->create should not be empty!");
}

unset($this->getOneByNameCacheResults[$wikiName]);
Expand Down Expand Up @@ -198,7 +198,7 @@ protected function generateUserLink($user)
{
// Generate the password recovery key
$passwordHasher = $this->passwordHasherFactory->getPasswordHasher($user);
$plainKey = $user['name'] . '_' . $user['email'] . random_int(0, 10000) . date('Y-m-d H:i:s');
$plainKey = $user['name'] . '_' . $user['email'] . random_bytes(16) . date('Y-m-d H:i:s');
$hashedKey = $passwordHasher->hash($plainKey);
$tripleStore = $this->wiki->services->get(TripleStore::class);
// Erase the previous triples in the trible table
Expand Down Expand Up @@ -238,6 +238,7 @@ public function sendPasswordRecoveryEmail(User $user, string $title): bool
$message .= _t('LOGIN_THE_TEAM') . ' ' . $domain . "\n";

$subject = $title . ' ' . $domain;

// Send the email
return send_mail($this->params->get('BAZ_ADRESSE_MAIL_ADMIN'), $this->params->get('BAZ_ADRESSE_MAIL_ADMIN'), $user['email'], $subject, $message);
}
Expand All @@ -256,7 +257,7 @@ public function getUserLink(): string
public function getLastUserLink(User $user): string
{
$passwordHasher = $this->passwordHasherFactory->getPasswordHasher($user);
$plainKey = $user['name'] . '_' . $user['email'] . random_int(0, 10000) . date('Y-m-d H:i:s');
$plainKey = $user['name'] . '_' . $user['email'] . random_bytes(16) . date('Y-m-d H:i:s');
$hashedKey = $passwordHasher->hash($plainKey);
$tripleStore = $this->wiki->services->get(TripleStore::class);
$key = $tripleStore->getOne($user['name'], self::KEY_VOCABULARY, '', '');
Expand All @@ -279,13 +280,13 @@ public function getLastUserLink(User $user): string
*
* @param array $newValues (associative array)
*
* @throws Exception
* @throws \Exception
* @throws UserEmailAlreadyUsedException
*/
public function update(User $user, array $newValues): bool
{
if ($this->securityController->isWikiHibernated()) {
throw new Exception(_t('WIKI_IN_HIBERNATION'));
throw new \Exception(_t('WIKI_IN_HIBERNATION'));
}
$newKeys = array_keys($newValues);
$authorizedKeys = array_filter($newKeys, function ($key) {
Expand All @@ -294,15 +295,15 @@ public function update(User $user, array $newValues): bool
'doubleclickedit',
'email',
'motto',
//'name', // name not currently updateable
// 'name', // name not currently updateable
// 'password', // password not updateable by this method
'revisioncount',
'show_comments',
]);
});
if (isset($newValues['email'])) {
if (empty($newValues['email'])) {
throw new Exception("\$newValues['email'] parameter of UserManager->update should not be empty!");
throw new \Exception("\$newValues['email'] parameter of UserManager->update should not be empty!");
} elseif ($user['email'] == $newValues['email']) {
$authorizedKeys = array_filter($authorizedKeys, function ($item) {
return $item != 'email';
Expand Down Expand Up @@ -343,7 +344,7 @@ function ($key) use ($newValues) {
public function delete(User $user)
{
if ($this->securityController->isWikiHibernated()) {
throw new Exception(_t('WIKI_IN_HIBERNATION'));
throw new \Exception(_t('WIKI_IN_HIBERNATION'));
}
unset($this->getOneByNameCacheResults[$user['name']]);
$query = "DELETE FROM {$this->dbService->prefixTable('users')} " .
Expand All @@ -352,7 +353,7 @@ public function delete(User $user)
if (!$this->dbService->query($query)) {
throw new DeleteUserException(_t('USER_DELETE_QUERY_FAILED') . '.');
}
} catch (Exception $ex) {
} catch (\Exception $ex) {
throw new DeleteUserException(_t('USER_DELETE_QUERY_FAILED') . '.');
}
}
Expand Down Expand Up @@ -396,12 +397,12 @@ public function isInGroup(string $groupName, ?string $username = null, bool $adm
* @param PasswordAuthenticatedUserInterface|UserInterface $user
*
* @throws UnsupportedUserException if the user is not supported
* @throws Exception if wiki is in hibernation
* @throws \Exception if wiki is in hibernation
*/
public function upgradePassword($user, string $newHashedPassword)
{
if ($this->securityController->isWikiHibernated()) {
throw new Exception(_t('WIKI_IN_HIBERNATION'));
throw new \Exception(_t('WIKI_IN_HIBERNATION'));
}
if (!$this->supportsClass(get_class($user))) {
throw new UnsupportedUserException();
Expand All @@ -416,7 +417,7 @@ public function upgradePassword($user, string $newHashedPassword)
'AND email= "' . $this->dbService->escape($user['email']) . '" ' .
'AND password= "' . $this->dbService->escape($previousPassword) . '";';
$this->dbService->query($query);
} catch (Throwable $th) {
} catch (\Throwable $th) {
// only throw error in debug mode
if ($this->wiki->GetConfigValue('debug') == 'yes') {
throw $th;
Expand Down Expand Up @@ -444,6 +445,7 @@ public function refreshUser(UserInterface $user)
if (!$this->supportsClass(get_class($user))) {
throw new UnsupportedUserException();
}

// currently force refresh
return $this->getOneByName($user->getName());
}
Expand Down Expand Up @@ -519,4 +521,4 @@ public function logout()
{
$this->wiki->services->get(AuthController::class)->logout();
}
}
}

0 comments on commit 0e58ac9

Please sign in to comment.