diff --git a/apps/provisioning_api/lib/Controller/AUserData.php b/apps/provisioning_api/lib/Controller/AUserData.php index c26c4f9e2d..6f682b41f2 100644 --- a/apps/provisioning_api/lib/Controller/AUserData.php +++ b/apps/provisioning_api/lib/Controller/AUserData.php @@ -31,12 +31,13 @@ declare(strict_types=1); namespace OCA\Provisioning_API\Controller; -use OC\Accounts\AccountManager; use OC\Group\Manager; use OC\User\Backend; use OC\User\NoUserException; use OC_Helper; use OCP\Accounts\IAccountManager; +use OCP\Accounts\PropertyDoesNotExistException; +use OCP\AppFramework\Http; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; @@ -61,7 +62,7 @@ abstract class AUserData extends OCSController { protected $groupManager; /** @var IUserSession */ protected $userSession; - /** @var AccountManager */ + /** @var IAccountManager */ protected $accountManager; /** @var IFactory */ protected $l10nFactory; @@ -72,7 +73,7 @@ abstract class AUserData extends OCSController { IConfig $config, IGroupManager $groupManager, IUserSession $userSession, - AccountManager $accountManager, + IAccountManager $accountManager, IFactory $l10nFactory) { parent::__construct($appName, $request); @@ -140,30 +141,35 @@ abstract class AUserData extends OCSController { $data['subadmin'] = $this->getUserSubAdminGroupsData($targetUserObject->getUID()); $data['quota'] = $this->fillStorageInfo($targetUserObject->getUID()); - if ($includeScopes) { - $data[IAccountManager::PROPERTY_AVATAR . self::SCOPE_SUFFIX] = $userAccount->getProperty(IAccountManager::PROPERTY_AVATAR)->getScope(); - } - - $data[IAccountManager::PROPERTY_EMAIL] = $targetUserObject->getEMailAddress(); - if ($includeScopes) { - $data[IAccountManager::PROPERTY_EMAIL . self::SCOPE_SUFFIX] = $userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getScope(); - } - $data[IAccountManager::PROPERTY_DISPLAYNAME] = $targetUserObject->getDisplayName(); - if ($includeScopes) { - $data[IAccountManager::PROPERTY_DISPLAYNAME . self::SCOPE_SUFFIX] = $userAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME)->getScope(); - } - - foreach ([ - IAccountManager::PROPERTY_PHONE, - IAccountManager::PROPERTY_ADDRESS, - IAccountManager::PROPERTY_WEBSITE, - IAccountManager::PROPERTY_TWITTER, - ] as $propertyName) { - $property = $userAccount->getProperty($propertyName); - $data[$propertyName] = $property->getValue(); + try { if ($includeScopes) { - $data[$propertyName . self::SCOPE_SUFFIX] = $property->getScope(); + $data[IAccountManager::PROPERTY_AVATAR . self::SCOPE_SUFFIX] = $userAccount->getProperty(IAccountManager::PROPERTY_AVATAR)->getScope(); } + + $data[IAccountManager::PROPERTY_EMAIL] = $targetUserObject->getEMailAddress(); + if ($includeScopes) { + $data[IAccountManager::PROPERTY_EMAIL . self::SCOPE_SUFFIX] = $userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getScope(); + } + $data[IAccountManager::PROPERTY_DISPLAYNAME] = $targetUserObject->getDisplayName(); + if ($includeScopes) { + $data[IAccountManager::PROPERTY_DISPLAYNAME . self::SCOPE_SUFFIX] = $userAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME)->getScope(); + } + + foreach ([ + IAccountManager::PROPERTY_PHONE, + IAccountManager::PROPERTY_ADDRESS, + IAccountManager::PROPERTY_WEBSITE, + IAccountManager::PROPERTY_TWITTER, + ] as $propertyName) { + $property = $userAccount->getProperty($propertyName); + $data[$propertyName] = $property->getValue(); + if ($includeScopes) { + $data[$propertyName . self::SCOPE_SUFFIX] = $property->getScope(); + } + } + } catch (PropertyDoesNotExistException $e) { + // hard coded properties should exist + throw new OCSException($e->getMessage(), Http::STATUS_INTERNAL_SERVER_ERROR, $e); } $data['groups'] = $gids; diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php index b031c40504..583af8454f 100644 --- a/apps/provisioning_api/lib/Controller/GroupsController.php +++ b/apps/provisioning_api/lib/Controller/GroupsController.php @@ -34,7 +34,7 @@ declare(strict_types=1); namespace OCA\Provisioning_API\Controller; -use OC\Accounts\AccountManager; +use OCP\Accounts\IAccountManager; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSForbiddenException; @@ -61,7 +61,7 @@ class GroupsController extends AUserData { IConfig $config, IGroupManager $groupManager, IUserSession $userSession, - AccountManager $accountManager, + IAccountManager $accountManager, IFactory $l10nFactory, LoggerInterface $logger) { parent::__construct($appName, diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 115b955354..84fdaf8bc8 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -46,7 +46,6 @@ use libphonenumber\NumberParseException; use libphonenumber\PhoneNumber; use libphonenumber\PhoneNumberFormat; use libphonenumber\PhoneNumberUtil; -use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\HintException; use OC\KnownUser\KnownUserService; @@ -102,7 +101,7 @@ class UsersController extends AUserData { IAppManager $appManager, IGroupManager $groupManager, IUserSession $userSession, - AccountManager $accountManager, + IAccountManager $accountManager, IURLGenerator $urlGenerator, LoggerInterface $logger, IFactory $l10nFactory, @@ -734,13 +733,14 @@ class UsersController extends AUserData { case IAccountManager::PROPERTY_ADDRESS: case IAccountManager::PROPERTY_WEBSITE: case IAccountManager::PROPERTY_TWITTER: - $userAccount = $this->accountManager->getUser($targetUser); - if ($userAccount[$key]['value'] !== $value) { - $userAccount[$key]['value'] = $value; + $userAccount = $this->accountManager->getAccount($targetUser); + $userProperty = $userAccount->getProperty($key); + if ($userProperty->getValue() !== $value) { try { - $this->accountManager->updateUser($targetUser, $userAccount, true); + $userProperty->setValue($value); + $this->accountManager->updateAccount($userAccount); - if ($key === IAccountManager::PROPERTY_PHONE) { + if ($userProperty->getName() === IAccountManager::PROPERTY_PHONE) { $this->knownUserService->deleteByContactUserId($targetUser->getUID()); } } catch (\InvalidArgumentException $e) { @@ -756,11 +756,12 @@ class UsersController extends AUserData { case IAccountManager::PROPERTY_TWITTER . self::SCOPE_SUFFIX: case IAccountManager::PROPERTY_AVATAR . self::SCOPE_SUFFIX: $propertyName = substr($key, 0, strlen($key) - strlen(self::SCOPE_SUFFIX)); - $userAccount = $this->accountManager->getUser($targetUser); - if ($userAccount[$propertyName]['scope'] !== $value) { - $userAccount[$propertyName]['scope'] = $value; + $userAccount = $this->accountManager->getAccount($targetUser); + $userProperty = $userAccount->getProperty($propertyName); + if ($userProperty->getScope() !== $value) { + $userProperty->setScope($value); try { - $this->accountManager->updateUser($targetUser, $userAccount, true); + $this->accountManager->updateAccount($userAccount); } catch (\InvalidArgumentException $e) { throw new OCSException('Invalid ' . $e->getMessage(), 102); } diff --git a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php index 0624c9e0b7..b32b6e4d89 100644 --- a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php +++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php @@ -30,7 +30,6 @@ namespace OCA\Provisioning_API\Tests\Controller; -use OC\Accounts\AccountManager; use OC\Group\Manager; use OC\SubAdmin; use OC\User\NoUserException; @@ -57,7 +56,7 @@ class GroupsControllerTest extends \Test\TestCase { protected $groupManager; /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ protected $userSession; - /** @var AccountManager|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IAccountManager|\PHPUnit\Framework\MockObject\MockObject */ protected $accountManager; /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ protected $logger; @@ -76,7 +75,7 @@ class GroupsControllerTest extends \Test\TestCase { $this->config = $this->createMock(IConfig::class); $this->groupManager = $this->createMock(Manager::class); $this->userSession = $this->createMock(IUserSession::class); - $this->accountManager = $this->createMock(AccountManager::class); + $this->accountManager = $this->createMock(IAccountManager::class); $this->l10nFactory = $this->createMock(IFactory::class); $this->logger = $this->createMock(LoggerInterface::class); @@ -181,19 +180,6 @@ class GroupsControllerTest extends \Test\TestCase { }); } - private function useAccountManager() { - $this->accountManager->expects($this->any()) - ->method('getUser') - ->willReturnCallback(function (IUser $user) { - return [ - IAccountManager::PROPERTY_PHONE => ['value' => '0800-call-' . $user->getUID()], - IAccountManager::PROPERTY_ADDRESS => ['value' => 'Holzweg 99, 0601 Herrera, Panama'], - IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://' . $user->getUid() . '.pa'], - IAccountManager::PROPERTY_TWITTER => ['value' => '@' . $user->getUID()], - ]; - }); - } - public function dataGetGroups() { return [ [null, 0, 0], @@ -505,7 +491,6 @@ class GroupsControllerTest extends \Test\TestCase { $gid = 'ncg1'; $this->asAdmin(); - $this->useAccountManager(); $users = [ 'ncu1' => $this->createUser('ncu1'), # regular @@ -551,7 +536,6 @@ class GroupsControllerTest extends \Test\TestCase { $gid = 'Department A/B C/D'; $this->asAdmin(); - $this->useAccountManager(); $users = [ 'ncu1' => $this->createUser('ncu1'), # regular diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 1afe9be431..ed644f58cd 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -41,7 +41,6 @@ namespace OCA\Provisioning_API\Tests\Controller; use Exception; -use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\Group\Manager; use OC\KnownUser\KnownUserService; @@ -88,7 +87,7 @@ class UsersControllerTest extends TestCase { protected $logger; /** @var UsersController|MockObject */ protected $api; - /** @var AccountManager|MockObject */ + /** @var IAccountManager|MockObject */ protected $accountManager; /** @var IURLGenerator|MockObject */ protected $urlGenerator; @@ -117,7 +116,7 @@ class UsersControllerTest extends TestCase { $this->userSession = $this->createMock(IUserSession::class); $this->logger = $this->createMock(LoggerInterface::class); $this->request = $this->createMock(IRequest::class); - $this->accountManager = $this->createMock(AccountManager::class); + $this->accountManager = $this->createMock(IAccountManager::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->l10nFactory = $this->createMock(IFactory::class); $this->newUserMailHelper = $this->createMock(NewUserMailHelper::class); @@ -1574,13 +1573,34 @@ class UsersControllerTest extends TestCase { ->method('getBackend') ->willReturn($backend); - $this->accountManager->expects($this->once()) - ->method('getUser') + $propertyMock = $this->createMock(IAccountProperty::class); + $propertyMock->expects($this->any()) + ->method('getName') + ->willReturn($propertyName); + $propertyMock->expects($this->any()) + ->method('getValue') + ->willReturn($oldValue); + $propertyMock->expects($this->once()) + ->method('setValue') + ->with($newValue) + ->willReturnSelf(); + $propertyMock->expects($this->any()) + ->method('getScope') + ->willReturn(IAccountManager::SCOPE_LOCAL); + + $accountMock = $this->createMock(IAccount::class); + $accountMock->expects($this->any()) + ->method('getProperty') + ->with($propertyName) + ->willReturn($propertyMock); + + $this->accountManager->expects($this->atLeastOnce()) + ->method('getAccount') ->with($loggedInUser) - ->willReturn([$propertyName => ['value' => $oldValue, 'scope' => IAccountManager::SCOPE_LOCAL]]); + ->willReturn($accountMock); $this->accountManager->expects($this->once()) - ->method('updateUser') - ->with($loggedInUser, [$propertyName => ['value' => $newValue, 'scope' => IAccountManager::SCOPE_LOCAL]], true); + ->method('updateAccount') + ->with($accountMock); $this->assertEquals([], $this->api->editUser('UserToEdit', $propertyName, $newValue)->getData()); } @@ -1624,13 +1644,34 @@ class UsersControllerTest extends TestCase { ->method('getBackend') ->willReturn($backend); - $this->accountManager->expects($this->once()) - ->method('getUser') + $propertyMock = $this->createMock(IAccountProperty::class); + $propertyMock->expects($this->any()) + ->method('getName') + ->willReturn($propertyName); + $propertyMock->expects($this->any()) + ->method('getValue') + ->willReturn('somevalue'); + $propertyMock->expects($this->any()) + ->method('getScope') + ->willReturn($oldScope); + $propertyMock->expects($this->atLeastOnce()) + ->method('setScope') + ->with($newScope) + ->willReturnSelf(); + + $accountMock = $this->createMock(IAccount::class); + $accountMock->expects($this->any()) + ->method('getProperty') + ->with($propertyName) + ->willReturn($propertyMock); + + $this->accountManager->expects($this->atLeastOnce()) + ->method('getAccount') ->with($loggedInUser) - ->willReturn([$propertyName => ['value' => 'somevalue', 'scope' => $oldScope]]); + ->willReturn($accountMock); $this->accountManager->expects($this->once()) - ->method('updateUser') - ->with($loggedInUser, [$propertyName => ['value' => 'somevalue', 'scope' => $newScope]], true); + ->method('updateAccount') + ->with($accountMock); $this->assertEquals([], $this->api->editUser('UserToEdit', $propertyName . 'Scope', $newScope)->getData()); }