Dont allow reusing usernames

We have cases in which app developers forget to delete user data when a user is being deleted. This could be problematic as future users with the same username could then access this data.

This mitigates this risk a bit by not allowing reusing of usernames. That said, we still need to fix occurrences of these issues by deleting missed data.

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
This commit is contained in:
Lukas Reschke 2021-05-25 20:25:19 +02:00
parent d9a9714675
commit 39974acc5e
9 changed files with 282 additions and 0 deletions

View File

@ -0,0 +1,34 @@
<?php
declare(strict_types=1);
namespace OC\Core\Migrations;
use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
class Version22000Date20210525173326 extends SimpleMigrationStep {
/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
* @param array $options
* @return null|ISchemaWrapper
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
$table = $schema->createTable('previously_used_userids');
$table->addColumn('user_id_hash', \OCP\DB\Types::STRING, [
'notnull' => true,
'length' => 128,
]);
$table->setPrimaryKey(['user_id_hash'], 'uid_hash_idx');
return $schema;
}
}

View File

@ -960,6 +960,7 @@ return array(
'OC\\Core\\Migrations\\Version21000Date20210309185126' => $baseDir . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version21000Date20210309185126' => $baseDir . '/core/Migrations/Version21000Date20210309185126.php',
'OC\\Core\\Migrations\\Version21000Date20210309185127' => $baseDir . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Migrations\\Version21000Date20210309185127' => $baseDir . '/core/Migrations/Version21000Date20210309185127.php',
'OC\\Core\\Migrations\\Version22000Date20210216080825' => $baseDir . '/core/Migrations/Version22000Date20210216080825.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => $baseDir . '/core/Migrations/Version22000Date20210216080825.php',
'OC\\Core\\Migrations\\Version22000Date20210525173326' => $baseDir . '/core/Migrations/Version22000Date20210525173326.php',
'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',
'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php',
'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php',
@ -1433,6 +1434,8 @@ return array(
'OC\\User\\NoUserException' => $baseDir . '/lib/private/User/NoUserException.php', 'OC\\User\\NoUserException' => $baseDir . '/lib/private/User/NoUserException.php',
'OC\\User\\Session' => $baseDir . '/lib/private/User/Session.php', 'OC\\User\\Session' => $baseDir . '/lib/private/User/Session.php',
'OC\\User\\User' => $baseDir . '/lib/private/User/User.php', 'OC\\User\\User' => $baseDir . '/lib/private/User/User.php',
'OC\\User\\UserDeletedListener' => $baseDir . '/lib/private/User/UserDeletedListener.php',
'OC\\User\\UsernameDuplicationPreventionManager' => $baseDir . '/lib/private/User/UsernameDuplicationPreventionManager.php',
'OC_API' => $baseDir . '/lib/private/legacy/OC_API.php', 'OC_API' => $baseDir . '/lib/private/legacy/OC_API.php',
'OC_App' => $baseDir . '/lib/private/legacy/OC_App.php', 'OC_App' => $baseDir . '/lib/private/legacy/OC_App.php',
'OC_DB' => $baseDir . '/lib/private/legacy/OC_DB.php', 'OC_DB' => $baseDir . '/lib/private/legacy/OC_DB.php',

View File

@ -989,6 +989,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\Core\\Migrations\\Version21000Date20210309185126' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version21000Date20210309185126' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185126.php',
'OC\\Core\\Migrations\\Version21000Date20210309185127' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Migrations\\Version21000Date20210309185127' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185127.php',
'OC\\Core\\Migrations\\Version22000Date20210216080825' => __DIR__ . '/../../..' . '/core/Migrations/Version22000Date20210216080825.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => __DIR__ . '/../../..' . '/core/Migrations/Version22000Date20210216080825.php',
'OC\\Core\\Migrations\\Version22000Date20210525173326' => __DIR__ . '/../../..' . '/core/Migrations/Version22000Date20210525173326.php',
'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',
'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php',
'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php',
@ -1462,6 +1463,8 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\User\\NoUserException' => __DIR__ . '/../../..' . '/lib/private/User/NoUserException.php', 'OC\\User\\NoUserException' => __DIR__ . '/../../..' . '/lib/private/User/NoUserException.php',
'OC\\User\\Session' => __DIR__ . '/../../..' . '/lib/private/User/Session.php', 'OC\\User\\Session' => __DIR__ . '/../../..' . '/lib/private/User/Session.php',
'OC\\User\\User' => __DIR__ . '/../../..' . '/lib/private/User/User.php', 'OC\\User\\User' => __DIR__ . '/../../..' . '/lib/private/User/User.php',
'OC\\User\\UserDeletedListener' => __DIR__ . '/../../..' . '/lib/private/User/UserDeletedListener.php',
'OC\\User\\UsernameDuplicationPreventionManager' => __DIR__ . '/../../..' . '/lib/private/User/UsernameDuplicationPreventionManager.php',
'OC_API' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_API.php', 'OC_API' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_API.php',
'OC_App' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_App.php', 'OC_App' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_App.php',
'OC_DB' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_DB.php', 'OC_DB' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_DB.php',

View File

@ -142,6 +142,7 @@ use OC\Share20\ShareHelper;
use OC\SystemTag\ManagerFactory as SystemTagManagerFactory; use OC\SystemTag\ManagerFactory as SystemTagManagerFactory;
use OC\Tagging\TagMapper; use OC\Tagging\TagMapper;
use OC\Template\JSCombiner; use OC\Template\JSCombiner;
use OC\User\UserDeletedListener;
use OCA\Theming\ImageManager; use OCA\Theming\ImageManager;
use OCA\Theming\ThemingDefaults; use OCA\Theming\ThemingDefaults;
use OCA\Theming\Util; use OCA\Theming\Util;
@ -1445,6 +1446,7 @@ class Server extends ServerContainer implements IServerContainer {
$eventDispatched = $this->get(IEventDispatcher::class); $eventDispatched = $this->get(IEventDispatcher::class);
$eventDispatched->addServiceListener(LoginFailed::class, LoginFailedListener::class); $eventDispatched->addServiceListener(LoginFailed::class, LoginFailedListener::class);
$eventDispatched->addServiceListener(PostLoginEvent::class, UserLoggedInListener::class); $eventDispatched->addServiceListener(PostLoginEvent::class, UserLoggedInListener::class);
$eventDispatched->addServiceListener(UserDeletedEvent::class, UserDeletedListener::class);
} }
/** /**

View File

@ -378,6 +378,10 @@ class Manager extends PublicEmitter implements IUserManager {
throw new HintException($l->t('The user limit has been reached and the user was not created.')); throw new HintException($l->t('The user limit has been reached and the user was not created.'));
} }
if (\OC::$server->get(UsernameDuplicationPreventionManager::class)->wasUsed($uid)) {
$l = \OC::$server->getL10N('lib');
throw new HintException($l->t("The user name has already been used previously."));
}
$localBackends = []; $localBackends = [];
foreach ($this->backends as $backend) { foreach ($this->backends as $backend) {
if ($backend instanceof Database) { if ($backend instanceof Database) {

View File

@ -0,0 +1,47 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2021 Lukas Reschke <lukas@statuscode.ch>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OC\User;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
use OCP\User\Events\UserDeletedEvent;
class UserDeletedListener implements IEventListener {
/** @var UsernameDuplicationPreventionManager */
private $usernameDuplicationPreventionManager;
public function __construct(UsernameDuplicationPreventionManager $usernameDuplicationPreventionManager) {
$this->usernameDuplicationPreventionManager = $usernameDuplicationPreventionManager;
}
public function handle(Event $event): void {
if (!($event instanceof UserDeletedEvent)) {
return;
}
$user = $event->getUser();
$this->usernameDuplicationPreventionManager->markUsed($user->getUID());
}
}

View File

@ -0,0 +1,75 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2021, Lukas Reschke <lukas@statuscode.ch>
*
* @author Lukas Reschke <lukas@statuscode.ch>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OC\User;
use OCP\IDBConnection;
class UsernameDuplicationPreventionManager {
/** @var IDBConnection */
private $dbConnection;
private const TABLE_NAME = 'previously_used_userids';
private const HASHED_USER_ID_NAME = 'user_id_hash';
public function __construct(IDBConnection $connection) {
$this->dbConnection = $connection;
}
private function calculateUserNameHash(string $username) : string {
return hash('sha512', $username);
}
public function markUsed(string $userName) : void {
$queryBuilder = $this->dbConnection->getQueryBuilder();
$queryBuilder->insert(self::TABLE_NAME)
->values([
self::HASHED_USER_ID_NAME => $queryBuilder->createNamedParameter($this->calculateUserNameHash($userName)),
])
->executeStatement();
}
public function wasUsed(string $userName) : bool {
$queryBuilder = $this->dbConnection->getQueryBuilder();
$result = $queryBuilder->select($queryBuilder->func()->count())
->from(self::TABLE_NAME)
->where(
$queryBuilder->expr()->eq(
self::HASHED_USER_ID_NAME,
$queryBuilder->expr()->literal($this->calculateUserNameHash($userName))
)
)
->executeQuery();
return (int)$result->fetchOne() !== 0;
}
public function cleanUp(): void {
$qb = $this->dbConnection->getQueryBuilder();
$qb
->delete(self::TABLE_NAME)
->executeStatement();
}
}

View File

@ -0,0 +1,53 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2021, Lukas Reschke <lukas@statuscode.ch>
*
* @author Lukas Reschke <lukas@statuscode.ch>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace Test\User;
use OC\User\UserDeletedListener;
use OC\User\UsernameDuplicationPreventionManager;
use OCP\IUser;
use OCP\User\Events\UserDeletedEvent;
use Test\TestCase;
class UserDeletedListenerTest extends TestCase {
public function testHandle() {
/** @var UsernameDuplicationPreventionManager|PHPUnit_Framework_MockObject_MockObject $usernameDuplicationPreventionManager */
$usernameDuplicationPreventionManager = $this->createMock(UsernameDuplicationPreventionManager::class);
$usernameDuplicationPreventionManager
->expects($this->once())
->method('markUsed')
->with($this->equalTo('ThisIsTheUsername'));
/** @var IUser|PHPUnit_Framework_MockObject_MockObject $mockUser */
$mockUser = $this->createMock(IUser::class);
$mockUser
->expects($this->once())
->method('getUID')
->willReturn('ThisIsTheUsername');
$listener = new UserDeletedListener($usernameDuplicationPreventionManager);
$listener->handle(new UserDeletedEvent($mockUser));
}
}

View File

@ -0,0 +1,61 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2021, Lukas Reschke <lukas@statuscode.ch>
*
* @author Lukas Reschke <lukas@statuscode.ch>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace Test\User;
use OC\User\UsernameDuplicationPreventionManager;
use Test\TestCase;
/**
* @group DB
*/
class UsernameDuplicationPreventionManagerTest extends TestCase {
/** @var UsernameDuplicationPreventionManager */
private $usernameDuplicationPreventionManager;
protected function setUp(): void {
parent::setUp();
$this->usernameDuplicationPreventionManager = \OC::$server->get(UsernameDuplicationPreventionManager::class);
}
protected function tearDown(): void {
parent::tearDown();
$this->usernameDuplicationPreventionManager->cleanUp();
}
public function testNotMarkedAsDeleted() {
$return = $this->usernameDuplicationPreventionManager->wasUsed('not_deleted_user');
$this->assertFalse($return);
}
public function testMarkedAsDeleted() {
$this->usernameDuplicationPreventionManager->markUsed('deleted_user');
$return = $this->usernameDuplicationPreventionManager->wasUsed('deleted_user');
$this->assertTrue($return);
}
}