diff --git a/lib/private/Files/External/Auth/Password/SessionCredentials.php b/lib/private/Files/External/Auth/Password/SessionCredentials.php index ff4e0b875f3c..fa3e982f8ed3 100644 --- a/lib/private/Files/External/Auth/Password/SessionCredentials.php +++ b/lib/private/Files/External/Auth/Password/SessionCredentials.php @@ -75,7 +75,11 @@ public function manipulateStorageConfig(IStorageConfig &$storage, IUser $user = } $credentials = \json_decode($this->crypto->decrypt($encrypted), true); - $storage->setBackendOption('user', $this->session->get('loginname')); + if ($user !== null) { + $storage->setBackendOption('user', $user->getUserName()); + } else { + $storage->setBackendOption('user', $this->session->get('loginname')); + } $storage->setBackendOption('password', $credentials['password']); } diff --git a/lib/private/Files/External/ConfigAdapter.php b/lib/private/Files/External/ConfigAdapter.php index 9942361911cf..c50ed726b85b 100644 --- a/lib/private/Files/External/ConfigAdapter.php +++ b/lib/private/Files/External/ConfigAdapter.php @@ -36,6 +36,8 @@ use OC\Files\Storage\FailedStorage; use OCP\Files\StorageNotAvailableException; use OCP\IConfig; +use OCP\Files\ObjectStore\IObjectStore; +use OCP\ISession; /** * Make the old files_external config work with the new public mount config api @@ -51,6 +53,9 @@ class ConfigAdapter implements IMountProvider { /** @var IUserGlobalStoragesService */ private $userGlobalStoragesService; + /** @var ISession */ + private $session; + /** * @param IUserStoragesService $userStoragesService * @param IUserGlobalStoragesService $userGlobalStoragesService @@ -58,11 +63,13 @@ class ConfigAdapter implements IMountProvider { public function __construct( IConfig $config, IUserStoragesService $userStoragesService, - IUserGlobalStoragesService $userGlobalStoragesService + IUserGlobalStoragesService $userGlobalStoragesService, + ISession $session ) { $this->config = $config; $this->userStoragesService = $userStoragesService; $this->userGlobalStoragesService = $userGlobalStoragesService; + $this->session = $session; } /** @@ -74,7 +81,7 @@ public function __construct( private function prepareStorageConfig(IStorageConfig &$storage, IUser $user) { foreach ($storage->getBackendOptions() as $option => $value) { $storage->setBackendOption($option, $this->setUserVars( - $user->getUID(), $value + $user->getUserName(), $value )); } diff --git a/lib/private/Files/External/LegacyUtil.php b/lib/private/Files/External/LegacyUtil.php index 553679e6dc3b..49b384177266 100644 --- a/lib/private/Files/External/LegacyUtil.php +++ b/lib/private/Files/External/LegacyUtil.php @@ -66,7 +66,7 @@ public static function getAbsoluteMountPoints($uid) { $mountPoint = '/'.$uid.'/files'.$storage->getMountPoint(); $mountEntry = self::prepareMountPointEntry($storage, false); foreach ($mountEntry['options'] as &$option) { - $option = self::setUserVars($uid, $option); + $option = self::setUserVars($user->getUserName(), $option); } $mountPoints[$mountPoint] = $mountEntry; } @@ -75,7 +75,7 @@ public static function getAbsoluteMountPoints($uid) { $mountPoint = '/'.$uid.'/files'.$storage->getMountPoint(); $mountEntry = self::prepareMountPointEntry($storage, true); foreach ($mountEntry['options'] as &$option) { - $option = self::setUserVars($uid, $option); + $option = self::setUserVars($user->getUserName(), $option); } $mountPoints[$mountPoint] = $mountEntry; } @@ -190,8 +190,11 @@ public static function getBackendStatus($class, $options, $isPersonal, $testOnly if (self::$skipTest) { return StorageNotAvailableException::STATUS_SUCCESS; } - foreach ($options as $key => $option) { - $options[$key] = self::setUserVars(\OCP\User::getUser(), $option); + $user = \OC::$server->getUserSession()->getUser(); + if ($user) { + foreach ($options as $key => $option) { + $options[$key] = self::setUserVars($user->getUserName(), $option); + } } if (\class_exists($class)) { try { diff --git a/lib/private/Server.php b/lib/private/Server.php index 087972d9f3c4..cb4fccfdaaf9 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -632,7 +632,8 @@ public function __construct($webRoot, \OC\Config $config) { $manager->registerProvider(new \OC\Files\External\ConfigAdapter( $c->query('AllConfig'), $c->query('UserStoragesService'), - $c->query('UserGlobalStoragesService') + $c->query('UserGlobalStoragesService'), + $c->getSession() )); } diff --git a/lib/private/User/RemoteUser.php b/lib/private/User/RemoteUser.php index f7c138d00ce0..ca9e36992794 100644 --- a/lib/private/User/RemoteUser.php +++ b/lib/private/User/RemoteUser.php @@ -49,6 +49,19 @@ public function getUID() { return $this->userId; } + /** + * @inheritdoc + */ + public function getUserName() { + return $this->getUID(); + } + + /** + * @inheritdoc + */ + public function setUserName($userName) { + } + /** * @inheritdoc */ diff --git a/lib/private/User/SyncService.php b/lib/private/User/SyncService.php index 8575bd014348..628208d625be 100644 --- a/lib/private/User/SyncService.php +++ b/lib/private/User/SyncService.php @@ -24,11 +24,13 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\IConfig; use OCP\ILogger; +use OCP\PreConditionNotMetException; use OCP\User\IProvidesDisplayNameBackend; use OCP\User\IProvidesEMailBackend; use OCP\User\IProvidesExtendedSearchBackend; use OCP\User\IProvidesHomeBackend; use OCP\User\IProvidesQuotaBackend; +use OCP\User\IProvidesUserNameBackend; use OCP\UserInterface; use OCP\AppFramework\Db\MultipleObjectsReturnedException; @@ -292,6 +294,30 @@ private function syncDisplayName(Account $a, UserInterface $backend) { } } + /** + * TODO store username in account table instead of user preferences + * + * @param Account $a + * @param UserInterface $backend + */ + private function syncUserName(Account $a, UserInterface $backend) { + $uid = $a->getUserId(); + if ($backend instanceof IProvidesUserNameBackend) { + $userName = $backend->getUserName($uid); + $currentUserName = $this->config->getUserValue($uid, 'core', 'username', null); + if ($userName !== $currentUserName) { + try { + $this->config->setUserValue($uid, 'core', 'username', $userName); + } catch (PreConditionNotMetException $e) { + // ignore, because precondition is empty + } + $this->logger->debug( + "Setting userName for <$uid> from <$currentUserName> to <$userName>", ['app' => self::class] + ); + } + } + } + /** * @param Account $a * @param UserInterface $backend @@ -322,6 +348,7 @@ public function syncAccount(Account $a, UserInterface $backend) { $this->syncQuota($a, $backend); $this->syncHome($a, $backend); $this->syncDisplayName($a, $backend); + $this->syncUserName($a, $backend); $this->syncSearchTerms($a, $backend); return $a; } diff --git a/lib/private/User/User.php b/lib/private/User/User.php index f8307ab2a7a5..e09620d599c9 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -42,6 +42,7 @@ use OCP\IConfig; use OCP\IUserBackend; use OCP\IUserSession; +use OCP\PreConditionNotMetException; use OCP\User\IChangePasswordBackend; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\GenericEvent; @@ -123,6 +124,35 @@ public function getUID() { return $this->account->getUserId(); } + /** + * get the user name + * TODO move username to account table + * + * @return string + */ + public function getUserName() { + $uid = $this->getUID(); + return $this->config->getUserValue($uid, 'core', 'username', $uid); + } + + /** + * set the user name + * TODO move username to account table + * + * @param string $userName + */ + public function setUserName($userName) { + $currentUserName = $this->getUserName(); + if ($userName !== $currentUserName) { + $uid = $this->getUID(); + try { + $this->config->setUserValue($uid, 'core', 'username', $userName); + } catch (PreConditionNotMetException $e) { + // ignore, because precondition is empty + } + } + } + /** * get the display name for the user, if no specific display name is set it will fallback to the user id * diff --git a/lib/public/IUser.php b/lib/public/IUser.php index 89c536ba45ee..25d011df7c7b 100644 --- a/lib/public/IUser.php +++ b/lib/public/IUser.php @@ -41,6 +41,22 @@ interface IUser { */ public function getUID(); + /** + * get the user name + * + * @return string + * @since 10.0.10 + */ + public function getUserName(); + + /** + * set the user name + * + * @param string $userName + * @since 10.0.10 + */ + public function setUserName($userName); + /** * get the display name for the user, if no specific display name is set it will fallback to the user id * diff --git a/lib/public/User/IProvidesUserNameBackend.php b/lib/public/User/IProvidesUserNameBackend.php new file mode 100644 index 000000000000..afa6a8c2eb6b --- /dev/null +++ b/lib/public/User/IProvidesUserNameBackend.php @@ -0,0 +1,41 @@ + + * + * @copyright Copyright (c) 2018, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * 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, version 3, + * along with this program. If not, see + * + */ + +// use OCP namespace for all classes that are considered public. +// This means that they should be used by apps instead of the internal ownCloud classes +namespace OCP\User; + +/** + * Interface IProvidesUserNameBackend + * + * @package OCP\User + * @since 10.0 + */ +interface IProvidesUserNameBackend { + + /** + * get display name of the user + * @param string $uid user ID of the user + * @return string user name + * @since 10.0.10 + */ + public function getUserName($uid); +} diff --git a/tests/lib/Files/External/Auth/Password/SessionCredentialsTest.php b/tests/lib/Files/External/Auth/Password/SessionCredentialsTest.php new file mode 100644 index 000000000000..47cb8aebd75f --- /dev/null +++ b/tests/lib/Files/External/Auth/Password/SessionCredentialsTest.php @@ -0,0 +1,124 @@ + + * + * @copyright Copyright (c) 2018, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * 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, version 3, + * along with this program. If not, see + * + */ + +namespace Test\Files\External\Auth\Password; + +use OC\Files\External\Auth\Password\SessionCredentials; +use OCP\ISession; +use OCP\Security\ICrypto; +use OCP\Files\External\IStorageConfig; +use OCP\IUser; + +class SessionCredentialsTest extends \Test\TestCase { + + /** @var SessionCredentials | \PHPUnit_Framework_MockObject_MockObject */ + private $authMech; + + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject */ + private $session; + + /** @var ICrypto | \PHPUnit_Framework_MockObject_MockObject */ + private $crypto; + + public function setUp() { + parent::setUp(); + $this->session = $this->createMock(ISession::class); + $this->crypto = $this->createMock(ICrypto::class); + $this->authMech = new SessionCredentials( + $this->session, + $this->crypto + ); + } + + public function tearDown() { + \OC_Hook::clear('OC_User', 'post_login'); + parent::tearDown(); + } + + public function sessionDataProvider() { + return [ + [ + [ + ['password::sessioncredentials/credentials', 'encrypted_stuff'], + ['loginname', 'login1'], + ], + null, + 'login1' + ], + [ + [ + ['password::sessioncredentials/credentials', 'encrypted_stuff'], + ], + 'altlogin2', + 'altlogin2' + ], + ]; + } + + /** + * @dataProvider sessionDataProvider + */ + public function testManipulateStorageConfigSetsBackendOptions($sessionData, $userName, $expectedLogin) { + $storageConfig = $this->createMock(IStorageConfig::class); + if ($userName !== null) { + $user = $this->createMock(IUser::class); + $user->expects($this->once()) + ->method('getUserName') + ->willReturn($userName); + } else { + $user = null; + } + + $this->session->method('get')->will($this->returnValueMap($sessionData)); + $this->session->method('exists')->will($this->returnCallback(function ($key) { + return $this->session->get($key) !== null; + })); + + $this->crypto->expects($this->once()) + ->method('decrypt') + ->with('encrypted_stuff') + ->willReturn('{"user":"user1","password":"pw"}'); + + $storageConfig->expects($this->at(0)) + ->method('setBackendOption') + ->with('user', $expectedLogin); + $storageConfig->expects($this->at(1)) + ->method('setBackendOption') + ->with('password', 'pw'); + + $this->authMech->manipulateStorageConfig($storageConfig, $user); + } + + /** + * @expectedException \OCP\Files\External\InsufficientDataForMeaningfulAnswerException + */ + public function testManipulateStorageConfigFailsWhenEmptyCredentials() { + $storageConfig = $this->createMock(IStorageConfig::class); + $user = $this->createMock(IUser::class); + + $this->session->expects($this->once()) + ->method('get') + ->with('password::sessioncredentials/credentials') + ->willReturn(null); + + $this->authMech->manipulateStorageConfig($storageConfig, $user); + } +} diff --git a/tests/lib/Files/External/ConfigAdapterTest.php b/tests/lib/Files/External/ConfigAdapterTest.php index eb2f78d05429..050aebe91a14 100644 --- a/tests/lib/Files/External/ConfigAdapterTest.php +++ b/tests/lib/Files/External/ConfigAdapterTest.php @@ -32,10 +32,12 @@ use OCP\Files\External\Service\IUserStoragesService; use OCP\IConfig; use OCP\IUser; +use OCP\ISession; +use OC\Files\External\StorageConfig; class ConfigAdapterTest extends \Test\TestCase { - /** @var \OCP\IConfig */ + /** @var \OCP\IConfig | \PHPUnit_Framework_MockObject_MockObject */ private $config; /** @var IUserStoragesService */ @@ -44,42 +46,38 @@ class ConfigAdapterTest extends \Test\TestCase { /** @var IUserGlobalStoragesService */ private $userGlobalStoragesService; - /** @var IUser **/ + /** @var IUser | \PHPUnit_Framework_MockObject_MockObject **/ private $user; + /** @var ISession | \PHPUnit_Framework_MockObject_MockObject **/ + private $session; + + /** @var int */ + private $configId; + protected function setUp() { $this->config = $this->createMock(IConfig::class); $this->userStoragesService = $this->createMock(UserStoragesService::class); $this->userGlobalStoragesService = $this->createMock(UserGlobalStoragesService::class); + $this->session = $this->createMock(ISession::class); $this->user = $this->createMock(IUser::class); $this->user->expects($this->any()) ->method('getUID') ->willReturn('user1'); + $this->configId = 0; } - private function createStorageConfig($mountPoint, $mountOptions) { + private function createStorageConfig($mountPoint, $mountOptions, $storageOptions) { $auth = $this->createMock(AuthMechanism::class); $backend = $this->createMock(Backend::class); - $config = $this->createMock(IStorageConfig::class); - - $config->expects($this->any()) - ->method('getBackendOptions') - ->willReturn([]); - $config->expects($this->any()) - ->method('getBackendOption') - ->willReturn(null); - $config->expects($this->any()) - ->method('getAuthMechanism') - ->willReturn($auth); - $config->expects($this->any()) - ->method('getBackend') - ->willReturn($backend); - $config->expects($this->any()) - ->method('getMountPoint') - ->willReturn($mountPoint); - $config->expects($this->any()) - ->method('getMountOptions') - ->willReturn($mountOptions); + $this->configId++; + $config = new StorageConfig($this->configId); + + $config->setMountPoint($mountPoint); + $config->setMountOptions($mountOptions); + $config->setBackendOptions($storageOptions); + $config->setAuthMechanism($auth); + $config->setBackend($backend); $auth->expects($this->once()) ->method('manipulateStorageConfig') @@ -130,15 +128,16 @@ private function getMountsForUser($globalStorages, $personalStorages) { $configAdapter = new ConfigAdapter( $this->config, $this->userStoragesService, - $this->userGlobalStoragesService + $this->userGlobalStoragesService, + $this->session ); return $configAdapter->getMountsForUser($this->user, $storageFactory); } public function testGetPersonalMounts() { - $storage1 = $this->createStorageConfig('/mount1', ['test_value' => true]); - $storage2 = $this->createStorageConfig('/globalmount1', ['test_value2' => 'abc']); + $storage1 = $this->createStorageConfig('/mount1', ['test_value' => true], ['backend_option' => 'abc']); + $storage2 = $this->createStorageConfig('/globalmount1', ['test_value2' => 'abc'], ['backend_option' => 'def']); $result = $this->getMountsForUser([$storage2], [$storage1]); @@ -173,8 +172,8 @@ public function testGetPersonalMountSharingOption($isSharingAllowed, $expectedVa ['core', 'allow_user_mount_sharing', 'yes', $isSharingAllowed ? 'yes' : 'no'] ])); - $storage1 = $this->createStorageConfig('/mount1', ['enable_sharing' => true]); - $storage2 = $this->createStorageConfig('/globalmount1', ['enable_sharing' => true]); + $storage1 = $this->createStorageConfig('/mount1', ['enable_sharing' => true], ['backend_option' => 'abc']); + $storage2 = $this->createStorageConfig('/globalmount1', ['enable_sharing' => true], ['backend_option' => 'abc']); $result = $this->getMountsForUser([$storage2], [$storage1]); @@ -188,4 +187,36 @@ public function testGetPersonalMountSharingOption($isSharingAllowed, $expectedVa $options = $mount->getOptions(); $this->assertTrue($options['enable_sharing']); } + + public function testGetPersonalMountsUserPlaceholder() { + $this->user->expects($this->any()) + ->method('getUserName') + ->willReturn('user1'); + + $storage1 = $this->createStorageConfig('/mount1', [], ['backend_option' => '$user']); + $storage2 = $this->createStorageConfig('/globalmount1', [], ['backend_option' => '$user']); + + $result = $this->getMountsForUser([$storage2], [$storage1]); + + $this->assertCount(2, $result); + + $this->assertEquals('user1', $storage1->getBackendOption('backend_option')); + $this->assertEquals('user1', $storage2->getBackendOption('backend_option')); + } + + public function testGetPersonalMountsUserPlaceholderAltLogin() { + $this->user->expects($this->any()) + ->method('getUserName') + ->willReturn('altlogin1'); + + $storage1 = $this->createStorageConfig('/mount1', [], ['backend_option' => '$user']); + $storage2 = $this->createStorageConfig('/globalmount1', [], ['backend_option' => '$user']); + + $result = $this->getMountsForUser([$storage2], [$storage1]); + + $this->assertCount(2, $result); + + $this->assertEquals('altlogin1', $storage1->getBackendOption('backend_option')); + $this->assertEquals('altlogin1', $storage2->getBackendOption('backend_option')); + } } diff --git a/tests/lib/User/SyncServiceTest.php b/tests/lib/User/SyncServiceTest.php index d9aed977686e..6c7ba5a249bb 100644 --- a/tests/lib/User/SyncServiceTest.php +++ b/tests/lib/User/SyncServiceTest.php @@ -34,6 +34,7 @@ use OCP\ILogger; use OCP\User\IProvidesHomeBackend; use OCP\User\IProvidesQuotaBackend; +use OCP\User\IProvidesUserNameBackend; use OCP\UserInterface; use Test\TestCase; @@ -276,4 +277,27 @@ public function testSyncQuota($backendProvidesQuota, $backendQuota, $preferences $s = new SyncService($this->config, $this->logger, $this->mapper); static::invokePrivate($s, 'syncQuota', [$a, $backend]); } + + public function testSyncUserName() { + $a = $this->createMock(Account::class); + $a->method('__call')->with('getUserId')->willReturn('user1'); + + /** @var UserInterface | IProvidesUserNameBackend | \PHPUnit_Framework_MockObject_MockObject $backend */ + $backend = $this->createMock([UserInterface::class, IProvidesUserNameBackend::class]); + $backend->expects($this->once()) + ->method('getUserName') + ->willReturn('userName1'); + + $this->config->expects($this->once()) + ->method('getUserValue') + ->with('user1', 'core', 'username', null) + ->willReturn(null); + + $this->config->expects($this->once()) + ->method('setUserValue') + ->with('user1', 'core', 'username', 'userName1'); + + $s = new SyncService($this->config, $this->logger, $this->mapper); + static::invokePrivate($s, 'syncUserName', [$a, $backend]); + } } diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 95a4afc62e12..5f3cd6d7e333 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -86,6 +86,43 @@ public function testDisplayNameEmpty() { $this->assertEquals('foo', $this->user->getDisplayName()); } + public function testGetUserName() { + $this->config->expects($this->once()) + ->method('getUserValue') + ->with('foo', 'core', 'username') + ->willReturn('fooName'); + $this->assertEquals('fooName', $this->user->getUserName()); + } + + public function testGetUserNameFallback() { + $this->config->expects($this->once()) + ->method('getUserValue') + ->with('foo', 'core', 'username') + ->willReturn('foo'); + $this->assertEquals('foo', $this->user->getUserName()); + } + + public function testSetUserName() { + $this->config->expects($this->at(0)) + ->method('getUserValue') + ->with('foo', 'core', 'username', 'foo') + ->willReturn('foo'); + $this->config->expects($this->at(1)) + ->method('setUserValue') + ->with('foo', 'core', 'username', 'fooName'); + $this->user->setUserName('fooName'); + } + + public function testSetUserNameSame() { + $this->config->expects($this->once()) + ->method('getUserValue') + ->with('foo', 'core', 'username', 'foo') + ->willReturn('foo'); + $this->config->expects($this->never()) + ->method('setUserValue'); + $this->user->setUserName('foo'); + } + public function testSetPassword() { $this->config->expects($this->once()) ->method('deleteUserValue')