From 2fde21e318e021d48a4880ff9cc10f0e45f5e1e9 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 22 Dec 2017 13:38:10 +0100 Subject: [PATCH 1/3] extend tests for status quo Signed-off-by: Arthur Schiwon --- tests/lib/User/ManagerTest.php | 12 ++++++++++++ tests/lib/Util/User/Dummy.php | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index 0d98a9825d..ac54d27d32 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -185,6 +185,9 @@ class ManagerTest extends TestCase { ->method('userExists') ->with($this->equalTo('foo')) ->will($this->returnValue(true)); + $backend->expects($this->once()) + ->method('loginName2UserName') + ->willReturn(false); $manager = new \OC\User\Manager($this->config); $manager->registerBackend($backend); @@ -236,6 +239,9 @@ class ManagerTest extends TestCase { ->method('getUsers') ->with($this->equalTo('fo'), $this->equalTo(3), $this->equalTo(1)) ->will($this->returnValue(array('foo1', 'foo2'))); + $backend1->expects($this->exactly(2)) + ->method('loginName2UserName') + ->willReturn(false); /** * @var \Test\Util\User\Dummy | \PHPUnit_Framework_MockObject_MockObject $backend2 @@ -245,6 +251,9 @@ class ManagerTest extends TestCase { ->method('getUsers') ->with($this->equalTo('fo'), $this->equalTo(3), $this->equalTo(1)) ->will($this->returnValue(array('foo3'))); + $backend2->expects($this->once()) + ->method('loginName2UserName') + ->willReturn(false); $manager = new \OC\User\Manager($this->config); $manager->registerBackend($backend1); @@ -324,6 +333,9 @@ class ManagerTest extends TestCase { ->method('userExists') ->with($this->equalTo('foo')) ->will($this->returnValue(false)); + $backend->expects($this->once()) + ->method('loginName2UserName') + ->willReturn(false); $manager = new \OC\User\Manager($this->config); $manager->registerBackend($backend); diff --git a/tests/lib/Util/User/Dummy.php b/tests/lib/Util/User/Dummy.php index 806a97bacb..e77902d6f4 100644 --- a/tests/lib/Util/User/Dummy.php +++ b/tests/lib/Util/User/Dummy.php @@ -108,6 +108,13 @@ class Dummy extends Backend implements \OCP\IUserBackend { return false; } + public function loginName2UserName($loginName) { + if(isset($this->users[strtolower($loginName)])) { + return strtolower($loginName); + } + return false; + } + /** * Get a list of all users * From 4f3d52a364b81c2af7bac274780af9b7650d6326 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 22 Dec 2017 15:22:49 +0100 Subject: [PATCH 2/3] never translate login names when requiring with a user id where appropriate, the preLoginNameUsedAsUserName hook should be thrown. Signed-off-by: Arthur Schiwon --- core/Controller/LostController.php | 6 +++++ lib/private/User/Manager.php | 10 -------- tests/lib/User/ManagerTest.php | 40 +++++++++++++++++++++--------- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php index befa1da694..e746218038 100644 --- a/core/Controller/LostController.php +++ b/core/Controller/LostController.php @@ -224,6 +224,12 @@ class LostController extends Controller { return new JSONResponse($this->error($this->l10n->t('Password reset is disabled'))); } + \OCP\Util::emitHook( + '\OCA\Files_Sharing\API\Server2Server', + 'preLoginNameUsedAsUserName', + ['uid' => &$user] + ); + // FIXME: use HTTP error codes try { $this->sendEmail($user); diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index c77e0ac89e..5f2a010561 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -152,16 +152,6 @@ class Manager extends PublicEmitter implements IUserManager { return $this->cachedUsers[$uid]; } - if (method_exists($backend, 'loginName2UserName')) { - $loginName = $backend->loginName2UserName($uid); - if ($loginName !== false) { - $uid = $loginName; - } - if (isset($this->cachedUsers[$uid])) { - return $this->cachedUsers[$uid]; - } - } - $user = new User($uid, $backend, $this, $this->config); if ($cacheUser) { $this->cachedUsers[$uid] = $user; diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index ac54d27d32..f6a362a503 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -185,9 +185,8 @@ class ManagerTest extends TestCase { ->method('userExists') ->with($this->equalTo('foo')) ->will($this->returnValue(true)); - $backend->expects($this->once()) - ->method('loginName2UserName') - ->willReturn(false); + $backend->expects($this->never()) + ->method('loginName2UserName'); $manager = new \OC\User\Manager($this->config); $manager->registerBackend($backend); @@ -211,6 +210,24 @@ class ManagerTest extends TestCase { $this->assertEquals(null, $manager->get('foo')); } + public function testGetOneBackendDoNotTranslateLoginNames() { + /** + * @var \Test\Util\User\Dummy | \PHPUnit_Framework_MockObject_MockObject $backend + */ + $backend = $this->createMock(\Test\Util\User\Dummy::class); + $backend->expects($this->once()) + ->method('userExists') + ->with($this->equalTo('bLeNdEr')) + ->will($this->returnValue(true)); + $backend->expects($this->never()) + ->method('loginName2UserName'); + + $manager = new \OC\User\Manager($this->config); + $manager->registerBackend($backend); + + $this->assertEquals('bLeNdEr', $manager->get('bLeNdEr')->getUID()); + } + public function testSearchOneBackend() { /** * @var \Test\Util\User\Dummy | \PHPUnit_Framework_MockObject_MockObject $backend @@ -220,6 +237,8 @@ class ManagerTest extends TestCase { ->method('getUsers') ->with($this->equalTo('fo')) ->will($this->returnValue(array('foo', 'afoo'))); + $backend->expects($this->never()) + ->method('loginName2UserName'); $manager = new \OC\User\Manager($this->config); $manager->registerBackend($backend); @@ -239,9 +258,8 @@ class ManagerTest extends TestCase { ->method('getUsers') ->with($this->equalTo('fo'), $this->equalTo(3), $this->equalTo(1)) ->will($this->returnValue(array('foo1', 'foo2'))); - $backend1->expects($this->exactly(2)) - ->method('loginName2UserName') - ->willReturn(false); + $backend1->expects($this->never()) + ->method('loginName2UserName'); /** * @var \Test\Util\User\Dummy | \PHPUnit_Framework_MockObject_MockObject $backend2 @@ -251,9 +269,8 @@ class ManagerTest extends TestCase { ->method('getUsers') ->with($this->equalTo('fo'), $this->equalTo(3), $this->equalTo(1)) ->will($this->returnValue(array('foo3'))); - $backend2->expects($this->once()) - ->method('loginName2UserName') - ->willReturn(false); + $backend2->expects($this->never()) + ->method('loginName2UserName'); $manager = new \OC\User\Manager($this->config); $manager->registerBackend($backend1); @@ -333,9 +350,8 @@ class ManagerTest extends TestCase { ->method('userExists') ->with($this->equalTo('foo')) ->will($this->returnValue(false)); - $backend->expects($this->once()) - ->method('loginName2UserName') - ->willReturn(false); + $backend->expects($this->never()) + ->method('loginName2UserName'); $manager = new \OC\User\Manager($this->config); $manager->registerBackend($backend); From e9eccf34f9258a3720afb3cde218275f4b35f884 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 3 Jan 2018 21:44:30 +0100 Subject: [PATCH 3/3] removes invalid tests, adapts another one Signed-off-by: Arthur Schiwon --- apps/user_ldap/tests/User_LDAPTest.php | 7 +++--- tests/lib/Files/FilesystemTest.php | 33 -------------------------- 2 files changed, 3 insertions(+), 37 deletions(-) diff --git a/apps/user_ldap/tests/User_LDAPTest.php b/apps/user_ldap/tests/User_LDAPTest.php index 5b53cc3da2..3262a2360a 100644 --- a/apps/user_ldap/tests/User_LDAPTest.php +++ b/apps/user_ldap/tests/User_LDAPTest.php @@ -1343,10 +1343,6 @@ class User_LDAPTest extends TestCase { } return true; })); - - $access->userManager->expects($this->atLeastOnce()) - ->method('get') - ->willReturn($this->createMock(User::class)); } /** @@ -1357,6 +1353,9 @@ class User_LDAPTest extends TestCase { $access = $this->getAccessMock(); $this->prepareAccessForSetPassword($access); + $access->userManager->expects($this->atLeastOnce()) + ->method('get') + ->willReturn($this->createMock(User::class)); $backend = new UserLDAP($access, $this->createMock(IConfig::class), $this->createMock(INotificationManager::class), $this->createMock(Session::class), $this->getDefaultPluginManagerMock()); \OC_User::useBackend($backend); diff --git a/tests/lib/Files/FilesystemTest.php b/tests/lib/Files/FilesystemTest.php index 1eb6c4fc54..a98af220ba 100644 --- a/tests/lib/Files/FilesystemTest.php +++ b/tests/lib/Files/FilesystemTest.php @@ -368,39 +368,6 @@ class FilesystemTest extends \Test\TestCase { $this->assertEquals(2, $thrown); } - public function testUserNameCasing() { - $this->logout(); - $userId = $this->getUniqueID('user_'); - - \OC_User::clearBackends(); - // needed for loginName2UserName mapping - $userBackend = $this->createMock(\OC\User\Database::class); - \OC::$server->getUserManager()->registerBackend($userBackend); - - $userBackend->expects($this->once()) - ->method('userExists') - ->with(strtoupper($userId)) - ->will($this->returnValue(true)); - $userBackend->expects($this->once()) - ->method('loginName2UserName') - ->with(strtoupper($userId)) - ->will($this->returnValue($userId)); - - $view = new \OC\Files\View(); - $this->assertFalse($view->file_exists('/' . $userId)); - - \OC\Files\Filesystem::initMountPoints(strtoupper($userId)); - - list($storage1, $path1) = $view->resolvePath('/' . $userId); - list($storage2, $path2) = $view->resolvePath('/' . strtoupper($userId)); - - $this->assertTrue($storage1->instanceOfStorage('\OCP\Files\IHomeStorage')); - $this->assertEquals('', $path1); - - // not mounted, still on the local root storage - $this->assertEquals(strtoupper($userId), $path2); - } - /** * Tests that the home storage is used for the user's mount point */