diff options
author | Lukas Reschke <lukas@statuscode.ch> | 2017-08-17 12:08:40 +0200 |
---|---|---|
committer | Lukas Reschke <lukas@statuscode.ch> | 2017-08-17 12:08:40 +0200 |
commit | ed8a98eaa1e44d172b838c5c9caa74261ac27eb1 (patch) | |
tree | 3b9a54794fdc298c8583b23c2e45cf7b9a90aed9 | |
parent | a53aa40b4ddf4c9f868ad03df7131ceee417f2c9 (diff) | |
download | nextcloud-server-ed8a98eaa1e44d172b838c5c9caa74261ac27eb1.tar.gz nextcloud-server-ed8a98eaa1e44d172b838c5c9caa74261ac27eb1.zip |
Prevent SQL error message in case of error
`\OC\User\Database::createUser` can throw a PHP exception in case the UID is longer than
permitted in the database. This is against it's PHPDocs and we should cast this to `false`,
so that the regular error handling triggers in.
The easiest way to reproduce is on MySQL:
1. Create user `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa` in admin panel
2. Create user `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa` in admin panel again
3. See SQL exception as error message
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
-rw-r--r-- | lib/private/User/Database.php | 6 | ||||
-rw-r--r-- | lib/private/User/Manager.php | 5 | ||||
-rw-r--r-- | tests/lib/User/ManagerTest.php | 21 |
3 files changed, 29 insertions, 3 deletions
diff --git a/lib/private/User/Database.php b/lib/private/User/Database.php index 73506c7d7c5..79032cfd405 100644 --- a/lib/private/User/Database.php +++ b/lib/private/User/Database.php @@ -92,7 +92,11 @@ class Database extends Backend implements IUserBackend { $event = new GenericEvent($password); $this->eventDispatcher->dispatch('OCP\PasswordPolicy::validate', $event); $query = \OC_DB::prepare('INSERT INTO `*PREFIX*users` ( `uid`, `password` ) VALUES( ?, ? )'); - $result = $query->execute(array($uid, \OC::$server->getHasher()->hash($password))); + try { + $result = $query->execute(array($uid, \OC::$server->getHasher()->hash($password))); + } catch (\Exception $e) { + $result = false; + } // Clear cache unset($this->cache[$uid]); diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 8f3c98d4b5e..6b6c10ab295 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -349,7 +349,10 @@ class Manager extends PublicEmitter implements IUserManager { } $this->emit('\OC\User', 'preCreateUser', [$uid, $password]); - $backend->createUser($uid, $password); + $state = $backend->createUser($uid, $password); + if($state === false) { + throw new \InvalidArgumentException($l->t('Could not create user')); + } $user = $this->getUserObject($uid, $backend); if ($user instanceof IUser) { $this->emit('\OC\User', 'postCreateUser', [$user, $password]); diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index cf725aae671..9520cd640fd 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -9,6 +9,7 @@ namespace Test\User; use OC\User\Database; +use OC\User\Manager; use OCP\IConfig; use OCP\IUser; use Test\TestCase; @@ -304,7 +305,6 @@ class ManagerTest extends TestCase { $this->setExpectedException(\InvalidArgumentException::class, $exception); $manager->createUser($uid, $password); - } public function testCreateUserSingleBackendNotExists() { @@ -386,6 +386,25 @@ class ManagerTest extends TestCase { } /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Could not create user + */ + public function testCreateUserFromBackendWithBackendError() { + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject $config */ + $config = $this->createMock(IConfig::class); + /** @var \Test\Util\User\Dummy|\PHPUnit_Framework_MockObject_MockObject $backend */ + $backend = $this->createMock(\Test\Util\User\Dummy::class); + $backend + ->expects($this->once()) + ->method('createUser') + ->with('MyUid', 'MyPassword') + ->willReturn(false); + + $manager = new Manager($config); + $manager->createUserFromBackend('MyUid', 'MyPassword', $backend); + } + + /** * @expectedException \Exception */ public function testCreateUserTwoBackendExists() { |