Skip to content

Commit

Permalink
Dont allow reusing usernames
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
LukasReschke committed May 26, 2021
1 parent 903b99b commit 2d2274d
Show file tree
Hide file tree
Showing 10 changed files with 287 additions and 5 deletions.
34 changes: 34 additions & 0 deletions core/Migrations/Version22000Date20210525173326.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
3 changes: 3 additions & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,7 @@
'OC\\Core\\Migrations\\Version21000Date20210309185126' => $baseDir . '/core/Migrations/Version21000Date20210309185126.php',
'OC\\Core\\Migrations\\Version21000Date20210309185127' => $baseDir . '/core/Migrations/Version21000Date20210309185127.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\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php',
'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php',
Expand Down Expand Up @@ -1433,6 +1434,8 @@
'OC\\User\\NoUserException' => $baseDir . '/lib/private/User/NoUserException.php',
'OC\\User\\Session' => $baseDir . '/lib/private/User/Session.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_App' => $baseDir . '/lib/private/legacy/OC_App.php',
'OC_DB' => $baseDir . '/lib/private/legacy/OC_DB.php',
Expand Down
11 changes: 7 additions & 4 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
{
public static $prefixLengthsPsr4 = array (
'O' =>
'O' =>
array (
'OC\\Core\\' => 8,
'OC\\' => 3,
Expand All @@ -16,15 +16,15 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
);

public static $prefixDirsPsr4 = array (
'OC\\Core\\' =>
'OC\\Core\\' =>
array (
0 => __DIR__ . '/../../..' . '/core',
),
'OC\\' =>
'OC\\' =>
array (
0 => __DIR__ . '/../../..' . '/lib/private',
),
'OCP\\' =>
'OCP\\' =>
array (
0 => __DIR__ . '/../../..' . '/lib/public',
),
Expand Down Expand Up @@ -989,6 +989,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\Core\\Migrations\\Version21000Date20210309185126' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185126.php',
'OC\\Core\\Migrations\\Version21000Date20210309185127' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185127.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\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php',
'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php',
Expand Down Expand Up @@ -1462,6 +1463,8 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\User\\NoUserException' => __DIR__ . '/../../..' . '/lib/private/User/NoUserException.php',
'OC\\User\\Session' => __DIR__ . '/../../..' . '/lib/private/User/Session.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_App' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_App.php',
'OC_DB' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_DB.php',
Expand Down
2 changes: 2 additions & 0 deletions lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@
use OC\SystemTag\ManagerFactory as SystemTagManagerFactory;
use OC\Tagging\TagMapper;
use OC\Template\JSCombiner;
use OC\User\UserDeletedListener;
use OCA\Theming\ImageManager;
use OCA\Theming\ThemingDefaults;
use OCA\Theming\Util;
Expand Down Expand Up @@ -1445,6 +1446,7 @@ private function connectDispatcher() {
$eventDispatched = $this->get(IEventDispatcher::class);
$eventDispatched->addServiceListener(LoginFailed::class, LoginFailedListener::class);
$eventDispatched->addServiceListener(PostLoginEvent::class, UserLoggedInListener::class);
$eventDispatched->addServiceListener(UserDeletedEvent::class, UserDeletedListener::class);
}

/**
Expand Down
4 changes: 4 additions & 0 deletions lib/private/User/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,10 @@ public function createUser($uid, $password) {
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 = [];
foreach ($this->backends as $backend) {
if ($backend instanceof Database) {
Expand Down
47 changes: 47 additions & 0 deletions lib/private/User/UserDeletedListener.php
Original file line number Diff line number Diff line change
@@ -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());
}
}
75 changes: 75 additions & 0 deletions lib/private/User/UsernameDuplicationPreventionManager.php
Original file line number Diff line number Diff line change
@@ -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();
}
}
2 changes: 1 addition & 1 deletion lib/public/DB/QueryBuilder/IQueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public function getState();
* @return IResult|int
* @throws Exception since 21.0.0
* @since 8.2.0
* @deprecated 22.0.0 Use executeQuery or executeStatement
* @deprecated 22.0.0 Use executeQuery or executeUpdate
*/
public function execute();

Expand Down
53 changes: 53 additions & 0 deletions tests/lib/User/UserDeletedListenerTest.php
Original file line number Diff line number Diff line change
@@ -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));
}
}
61 changes: 61 additions & 0 deletions tests/lib/User/UsernameDuplicationPreventionManagerTest.php
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit 2d2274d

Please sign in to comment.