Merge pull request #27035 from nextcloud/techdebt/26866/userscontroller-accountmanager-api

do not use private AccountManager in UsersController
pull/26466/head
blizzz 4 years ago committed by GitHub
commit 1b47886abb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 166
      apps/settings/lib/Controller/UsersController.php
  2. 402
      apps/settings/tests/Controller/UsersControllerTest.php
  3. 4
      lib/private/Accounts/Account.php
  4. 17
      lib/private/Accounts/AccountProperty.php
  5. 5
      lib/public/Accounts/IAccount.php
  6. 14
      lib/public/Accounts/IAccountProperty.php
  7. 7
      tests/lib/Accounts/AccountManagerTest.php
  8. 35
      tests/lib/Accounts/AccountPropertyTest.php
  9. 16
      tests/lib/Accounts/AccountTest.php

@ -37,7 +37,7 @@ declare(strict_types=1);
namespace OCA\Settings\Controller;
use OC\Accounts\AccountManager;
use InvalidArgumentException;
use OC\AppFramework\Http;
use OC\Encryption\Exceptions\ModuleDoesNotExistsException;
use OC\ForbiddenException;
@ -49,7 +49,9 @@ use OC\User\Manager as UserManager;
use OCA\Settings\BackgroundJobs\VerifyUserData;
use OCA\Settings\Events\BeforeTemplateRenderedEvent;
use OCA\User_LDAP\User_Proxy;
use OCP\Accounts\IAccount;
use OCP\Accounts\IAccountManager;
use OCP\Accounts\PropertyDoesNotExistException;
use OCP\App\IAppManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\DataResponse;
@ -88,7 +90,7 @@ class UsersController extends Controller {
private $l10nFactory;
/** @var IAppManager */
private $appManager;
/** @var AccountManager */
/** @var IAccountManager */
private $accountManager;
/** @var Manager */
private $keyManager;
@ -114,7 +116,7 @@ class UsersController extends Controller {
IMailer $mailer,
IFactory $l10nFactory,
IAppManager $appManager,
AccountManager $accountManager,
IAccountManager $accountManager,
Manager $keyManager,
IJobList $jobList,
IManager $encryptionManager,
@ -393,53 +395,36 @@ class UsersController extends Controller {
);
}
$data = $this->accountManager->getUser($user);
$beforeData = $data;
if (!is_null($avatarScope)) {
$data[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope;
}
if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) {
if (!is_null($displayname)) {
$data[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $displayname;
}
if (!is_null($displaynameScope)) {
$data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displaynameScope;
$userAccount = $this->accountManager->getAccount($user);
$oldPhoneValue = $userAccount->getProperty(IAccountManager::PROPERTY_PHONE)->getValue();
$updatable = [
IAccountManager::PROPERTY_AVATAR => ['value' => null, 'scope' => $avatarScope],
IAccountManager::PROPERTY_DISPLAYNAME => ['value' => $displayname, 'scope' => $displaynameScope],
IAccountManager::PROPERTY_EMAIL => ['value' => $email, 'scope' => $emailScope],
IAccountManager::PROPERTY_WEBSITE => ['value' => $website, 'scope' => $websiteScope],
IAccountManager::PROPERTY_ADDRESS => ['value' => $address, 'scope' => $addressScope],
IAccountManager::PROPERTY_PHONE => ['value' => $phone, 'scope' => $phoneScope],
IAccountManager::PROPERTY_TWITTER => ['value' => $twitter, 'scope' => $twitterScope],
];
$allowUserToChangeDisplayName = $this->config->getSystemValueBool('allow_user_to_change_display_name', true);
foreach ($updatable as $property => $data) {
if ($allowUserToChangeDisplayName === false
&& in_array($property, [IAccountManager::PROPERTY_DISPLAYNAME, IAccountManager::PROPERTY_EMAIL], true)) {
continue;
}
if (!is_null($email)) {
$data[IAccountManager::PROPERTY_EMAIL]['value'] = $email;
$property = $userAccount->getProperty($property);
if (null !== $data['value']) {
$property->setValue($data['value']);
}
if (!is_null($emailScope)) {
$data[IAccountManager::PROPERTY_EMAIL]['scope'] = $emailScope;
if (null !== $data['scope']) {
$property->setScope($data['scope']);
}
}
if (!is_null($website)) {
$data[IAccountManager::PROPERTY_WEBSITE]['value'] = $website;
}
if (!is_null($websiteScope)) {
$data[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope;
}
if (!is_null($address)) {
$data[IAccountManager::PROPERTY_ADDRESS]['value'] = $address;
}
if (!is_null($addressScope)) {
$data[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope;
}
if (!is_null($phone)) {
$data[IAccountManager::PROPERTY_PHONE]['value'] = $phone;
}
if (!is_null($phoneScope)) {
$data[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope;
}
if (!is_null($twitter)) {
$data[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter;
}
if (!is_null($twitterScope)) {
$data[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope;
}
try {
$data = $this->saveUserSettings($user, $data);
if ($beforeData[IAccountManager::PROPERTY_PHONE]['value'] !== $data[IAccountManager::PROPERTY_PHONE]['value']) {
$this->saveUserSettings($userAccount);
if ($oldPhoneValue !== $userAccount->getProperty(IAccountManager::PROPERTY_PHONE)->getValue()) {
$this->knownUserService->deleteByContactUserId($user->getUID());
}
return new DataResponse(
@ -447,32 +432,25 @@ class UsersController extends Controller {
'status' => 'success',
'data' => [
'userId' => $user->getUID(),
'avatarScope' => $data[IAccountManager::PROPERTY_AVATAR]['scope'],
'displayname' => $data[IAccountManager::PROPERTY_DISPLAYNAME]['value'],
'displaynameScope' => $data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'],
'phone' => $data[IAccountManager::PROPERTY_PHONE]['value'],
'phoneScope' => $data[IAccountManager::PROPERTY_PHONE]['scope'],
'email' => $data[IAccountManager::PROPERTY_EMAIL]['value'],
'emailScope' => $data[IAccountManager::PROPERTY_EMAIL]['scope'],
'website' => $data[IAccountManager::PROPERTY_WEBSITE]['value'],
'websiteScope' => $data[IAccountManager::PROPERTY_WEBSITE]['scope'],
'address' => $data[IAccountManager::PROPERTY_ADDRESS]['value'],
'addressScope' => $data[IAccountManager::PROPERTY_ADDRESS]['scope'],
'twitter' => $data[IAccountManager::PROPERTY_TWITTER]['value'],
'twitterScope' => $data[IAccountManager::PROPERTY_TWITTER]['scope'],
'message' => $this->l10n->t('Settings saved')
]
'avatarScope' => $userAccount->getProperty(IAccountManager::PROPERTY_AVATAR)->getScope(),
'displayname' => $userAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME)->getValue(),
'displaynameScope' => $userAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME)->getScope(),
'phone' => $userAccount->getProperty(IAccountManager::PROPERTY_PHONE)->getValue(),
'phoneScope' => $userAccount->getProperty(IAccountManager::PROPERTY_PHONE)->getScope(),
'email' => $userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getValue(),
'emailScope' => $userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getScope(),
'website' => $userAccount->getProperty(IAccountManager::PROPERTY_WEBSITE)->getValue(),
'websiteScope' => $userAccount->getProperty(IAccountManager::PROPERTY_WEBSITE)->getScope(),
'address' => $userAccount->getProperty(IAccountManager::PROPERTY_ADDRESS)->getValue(),
'addressScope' => $userAccount->getProperty(IAccountManager::PROPERTY_ADDRESS)->getScope(),
'twitter' => $userAccount->getProperty(IAccountManager::PROPERTY_TWITTER)->getValue(),
'twitterScope' => $userAccount->getProperty(IAccountManager::PROPERTY_TWITTER)->getScope(),
'message' => $this->l10n->t('Settings saved'),
],
],
Http::STATUS_OK
);
} catch (ForbiddenException $e) {
return new DataResponse([
'status' => 'error',
'data' => [
'message' => $e->getMessage()
],
]);
} catch (\InvalidArgumentException $e) {
} catch (ForbiddenException | InvalidArgumentException | PropertyDoesNotExistException $e) {
return new DataResponse([
'status' => 'error',
'data' => [
@ -484,49 +462,41 @@ class UsersController extends Controller {
/**
* update account manager with new user data
*
* @param IUser $user
* @param array $data
* @return array
* @throws ForbiddenException
* @throws \InvalidArgumentException
* @throws InvalidArgumentException
*/
protected function saveUserSettings(IUser $user, array $data): array {
protected function saveUserSettings(IAccount $userAccount): void {
// keep the user back-end up-to-date with the latest display name and email
// address
$oldDisplayName = $user->getDisplayName();
$oldDisplayName = is_null($oldDisplayName) ? '' : $oldDisplayName;
if (isset($data[IAccountManager::PROPERTY_DISPLAYNAME]['value'])
&& $oldDisplayName !== $data[IAccountManager::PROPERTY_DISPLAYNAME]['value']
) {
$result = $user->setDisplayName($data[IAccountManager::PROPERTY_DISPLAYNAME]['value']);
$oldDisplayName = $userAccount->getUser()->getDisplayName();
if ($oldDisplayName !== $userAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME)->getValue()) {
$result = $userAccount->getUser()->setDisplayName($userAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME)->getValue());
if ($result === false) {
throw new ForbiddenException($this->l10n->t('Unable to change full name'));
}
}
$oldEmailAddress = $user->getEMailAddress();
$oldEmailAddress = is_null($oldEmailAddress) ? '' : strtolower($oldEmailAddress);
if (isset($data[IAccountManager::PROPERTY_EMAIL]['value'])
&& $oldEmailAddress !== $data[IAccountManager::PROPERTY_EMAIL]['value']
) {
$oldEmailAddress = $userAccount->getUser()->getEMailAddress();
$oldEmailAddress = strtolower((string)$oldEmailAddress);
if ($oldEmailAddress !== $userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getValue()) {
// this is the only permission a backend provides and is also used
// for the permission of setting a email address
if (!$user->canChangeDisplayName()) {
if (!$userAccount->getUser()->canChangeDisplayName()) {
throw new ForbiddenException($this->l10n->t('Unable to change email address'));
}
$user->setEMailAddress($data[IAccountManager::PROPERTY_EMAIL]['value']);
$userAccount->getUser()->setEMailAddress($userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getValue());
}
try {
return $this->accountManager->updateUser($user, $data, true);
} catch (\InvalidArgumentException $e) {
$this->accountManager->updateAccount($userAccount);
} catch (InvalidArgumentException $e) {
if ($e->getMessage() === IAccountManager::PROPERTY_PHONE) {
throw new \InvalidArgumentException($this->l10n->t('Unable to set invalid phone number'));
throw new InvalidArgumentException($this->l10n->t('Unable to set invalid phone number'));
}
if ($e->getMessage() === IAccountManager::PROPERTY_WEBSITE) {
throw new \InvalidArgumentException($this->l10n->t('Unable to set invalid website'));
throw new InvalidArgumentException($this->l10n->t('Unable to set invalid website'));
}
throw new \InvalidArgumentException($this->l10n->t('Some account data was invalid'));
throw new InvalidArgumentException($this->l10n->t('Some account data was invalid'));
}
}
@ -548,7 +518,7 @@ class UsersController extends Controller {
return new DataResponse([], Http::STATUS_BAD_REQUEST);
}
$accountData = $this->accountManager->getUser($user);
$userAccount = $this->accountManager->getAccount($user);
$cloudId = $user->getCloudId();
$message = 'Use my Federated Cloud ID to share with me: ' . $cloudId;
$signature = $this->signMessage($user, $message);
@ -558,30 +528,30 @@ class UsersController extends Controller {
switch ($account) {
case 'verify-twitter':
$accountData[IAccountManager::PROPERTY_TWITTER]['verified'] = IAccountManager::VERIFICATION_IN_PROGRESS;
$msg = $this->l10n->t('In order to verify your Twitter account, post the following tweet on Twitter (please make sure to post it without any line breaks):');
$code = $codeMd5;
$type = IAccountManager::PROPERTY_TWITTER;
$accountData[IAccountManager::PROPERTY_TWITTER]['signature'] = $signature;
break;
case 'verify-website':
$accountData[IAccountManager::PROPERTY_WEBSITE]['verified'] = IAccountManager::VERIFICATION_IN_PROGRESS;
$msg = $this->l10n->t('In order to verify your Website, store the following content in your web-root at \'.well-known/CloudIdVerificationCode.txt\' (please make sure that the complete text is in one line):');
$type = IAccountManager::PROPERTY_WEBSITE;
$accountData[IAccountManager::PROPERTY_WEBSITE]['signature'] = $signature;
break;
default:
return new DataResponse([], Http::STATUS_BAD_REQUEST);
}
$userProperty = $userAccount->getProperty($type);
$userProperty
->setVerified(IAccountManager::VERIFICATION_IN_PROGRESS)
->setVerificationData($signature);
if ($onlyVerificationCode === false) {
$accountData = $this->accountManager->updateUser($user, $accountData);
$data = $accountData[$type]['value'];
$this->accountManager->updateAccount($userAccount);
$this->jobList->add(VerifyUserData::class,
[
'verificationCode' => $code,
'data' => $data,
'data' => $userProperty->getValue(),
'type' => $type,
'uid' => $user->getUID(),
'try' => 0,

@ -31,10 +31,14 @@ namespace OCA\Settings\Tests\Controller;
use OC\Accounts\AccountManager;
use OC\Encryption\Exceptions\ModuleDoesNotExistsException;
use OC\ForbiddenException;
use OC\Group\Manager;
use OC\KnownUser\KnownUserService;
use OCA\Settings\Controller\UsersController;
use OCP\Accounts\IAccount;
use OCP\Accounts\IAccountManager;
use OCP\Accounts\IAccountProperty;
use OCP\Accounts\PropertyDoesNotExistException;
use OCP\App\IAppManager;
use OCP\AppFramework\Http;
use OCP\BackgroundJob\IJobList;
@ -53,6 +57,7 @@ use OCP\IUserSession;
use OCP\L10N\IFactory;
use OCP\Mail\IMailer;
use OCP\Security\ISecureRandom;
use PHPUnit\Framework\MockObject\MockObject;
/**
* @group DB
@ -180,49 +185,77 @@ class UsersControllerTest extends \Test\TestCase {
}
}
protected function getDefaultAccountManagerUserData() {
return [
IAccountManager::PROPERTY_DISPLAYNAME =>
[
'value' => 'Default display name',
'scope' => IAccountManager::SCOPE_FEDERATED,
'verified' => IAccountManager::VERIFIED,
],
IAccountManager::PROPERTY_ADDRESS =>
[
'value' => 'Default address',
'scope' => IAccountManager::SCOPE_LOCAL,
'verified' => IAccountManager::VERIFIED,
],
IAccountManager::PROPERTY_WEBSITE =>
[
'value' => 'Default website',
'scope' => IAccountManager::SCOPE_LOCAL,
'verified' => IAccountManager::VERIFIED,
],
IAccountManager::PROPERTY_EMAIL =>
[
'value' => 'Default email',
'scope' => IAccountManager::SCOPE_FEDERATED,
'verified' => IAccountManager::VERIFIED,
],
IAccountManager::PROPERTY_AVATAR =>
[
'scope' => IAccountManager::SCOPE_FEDERATED
],
IAccountManager::PROPERTY_PHONE =>
[
'value' => 'Default phone',
'scope' => IAccountManager::SCOPE_LOCAL,
'verified' => IAccountManager::VERIFIED,
],
IAccountManager::PROPERTY_TWITTER =>
[
'value' => 'Default twitter',
'scope' => IAccountManager::SCOPE_LOCAL,
'verified' => IAccountManager::VERIFIED,
],
protected function buildPropertyMock(string $name, string $value, string $scope, string $verified = IAccountManager::VERIFIED): MockObject {
$property = $this->createMock(IAccountProperty::class);
$property->expects($this->any())
->method('getName')
->willReturn($name);
$property->expects($this->any())
->method('getValue')
->willReturn($value);
$property->expects($this->any())
->method('getScope')
->willReturn($scope);
$property->expects($this->any())
->method('getVerified')
->willReturn($verified);
return $property;
}
protected function getDefaultAccountMock(bool $useDefaultValues = true): MockObject {
$propertyMocks = [
IAccountManager::PROPERTY_DISPLAYNAME => $this->buildPropertyMock(
IAccountManager::PROPERTY_DISPLAYNAME,
'Default display name',
IAccountManager::SCOPE_FEDERATED,
),
IAccountManager::PROPERTY_ADDRESS => $this->buildPropertyMock(
IAccountManager::PROPERTY_ADDRESS,
'Default address',
IAccountManager::SCOPE_LOCAL,
),
IAccountManager::PROPERTY_WEBSITE => $this->buildPropertyMock(
IAccountManager::PROPERTY_WEBSITE,
'Default website',
IAccountManager::SCOPE_LOCAL,
),
IAccountManager::PROPERTY_EMAIL => $this->buildPropertyMock(
IAccountManager::PROPERTY_EMAIL,
'Default email',
IAccountManager::SCOPE_FEDERATED,
),
IAccountManager::PROPERTY_AVATAR => $this->buildPropertyMock(
IAccountManager::PROPERTY_AVATAR,
'',
IAccountManager::SCOPE_FEDERATED,
),
IAccountManager::PROPERTY_PHONE => $this->buildPropertyMock(
IAccountManager::PROPERTY_PHONE,
'Default phone',
IAccountManager::SCOPE_LOCAL,
),
IAccountManager::PROPERTY_TWITTER => $this->buildPropertyMock(
IAccountManager::PROPERTY_TWITTER,
'Default twitter',
IAccountManager::SCOPE_LOCAL,
),
];
$account = $this->createMock(IAccount::class);
$account->expects($this->any())
->method('getProperty')
->willReturnCallback(function (string $propertyName) use ($propertyMocks) {
if (isset($propertyMocks[$propertyName])) {
return $propertyMocks[$propertyName];
}
throw new PropertyDoesNotExistException($propertyName);
});
$account->expects($this->any())
->method('getProperties')
->willReturn($propertyMocks);
return $account;
}
/**
@ -248,13 +281,12 @@ class UsersControllerTest extends \Test\TestCase {
if ($saveData) {
$this->accountManager->expects($this->once())
->method('getUser')
->method('getAccount')
->with($user)
->willReturn($this->getDefaultAccountManagerUserData());
->willReturn($this->getDefaultAccountMock());
$controller->expects($this->once())
->method('saveUserSettings')
->willReturnArgument(1);
->method('saveUserSettings');
} else {
$controller->expects($this->never())->method('saveUserSettings');
}
@ -289,20 +321,67 @@ class UsersControllerTest extends \Test\TestCase {
public function testSetUserSettingsWhenUserDisplayNameChangeNotAllowed() {
$controller = $this->getController(false, ['saveUserSettings']);
$avatarScope = IAccountManager::SCOPE_PUBLISHED;
$displayName = 'Display name';
$displayNameScope = IAccountManager::SCOPE_PUBLISHED;
$phone = '47658468';
$phoneScope = IAccountManager::SCOPE_PUBLISHED;
$email = 'john@example.com';
$emailScope = IAccountManager::SCOPE_PUBLISHED;
$website = 'nextcloud.com';
$websiteScope = IAccountManager::SCOPE_PUBLISHED;
$address = 'street and city';
$addressScope = IAccountManager::SCOPE_PUBLISHED;
$twitter = '@nextclouders';
$twitterScope = IAccountManager::SCOPE_PUBLISHED;
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('johndoe');
$this->userSession->method('getUser')->willReturn($user);
$defaultProperties = $this->getDefaultAccountManagerUserData();
/** @var MockObject|IAccount $userAccount */
$userAccount = $this->getDefaultAccountMock();
$this->accountManager->expects($this->once())
->method('getUser')
->method('getAccount')
->with($user)
->willReturn($defaultProperties);
->willReturn($userAccount);
/** @var MockObject|IAccountProperty $avatarProperty */
$avatarProperty = $userAccount->getProperty(IAccountManager::PROPERTY_AVATAR);
$avatarProperty->expects($this->atLeastOnce())
->method('setScope')
->with($avatarScope)
->willReturnSelf();
/** @var MockObject|IAccountProperty $avatarProperty */
$avatarProperty = $userAccount->getProperty(IAccountManager::PROPERTY_ADDRESS);
$avatarProperty->expects($this->atLeastOnce())
->method('setScope')
->with($addressScope)
->willReturnSelf();
$avatarProperty->expects($this->atLeastOnce())
->method('setValue')
->with($address)
->willReturnSelf();
/** @var MockObject|IAccountProperty $emailProperty */
$emailProperty = $userAccount->getProperty(IAccountManager::PROPERTY_EMAIL);
$emailProperty->expects($this->never())
->method('setValue');
$emailProperty->expects($this->never())
->method('setScope');
/** @var MockObject|IAccountProperty $emailProperty */
$emailProperty = $userAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME);
$emailProperty->expects($this->never())
->method('setValue');
$emailProperty->expects($this->never())
->method('setScope');
$this->config->expects($this->once())
->method('getSystemValue')
->method('getSystemValueBool')
->with('allow_user_to_change_display_name')
->willReturn(false);
@ -311,41 +390,13 @@ class UsersControllerTest extends \Test\TestCase {
->with('federatedfilesharing')
->willReturn(true);
$avatarScope = IAccountManager::SCOPE_PUBLISHED;
$displayName = 'Display name';
$displayNameScope = IAccountManager::SCOPE_PUBLISHED;
$phone = '47658468';
$phoneScope = IAccountManager::SCOPE_PUBLISHED;
$email = 'john@example.com';
$emailScope = IAccountManager::SCOPE_PUBLISHED;
$website = 'nextcloud.com';
$websiteScope = IAccountManager::SCOPE_PUBLISHED;
$address = 'street and city';
$addressScope = IAccountManager::SCOPE_PUBLISHED;
$twitter = '@nextclouders';
$twitterScope = IAccountManager::SCOPE_PUBLISHED;
// Display name and email are not changed.
$expectedProperties = $defaultProperties;
$expectedProperties[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope;
$expectedProperties[IAccountManager::PROPERTY_PHONE]['value'] = $phone;
$expectedProperties[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope;
$expectedProperties[IAccountManager::PROPERTY_WEBSITE]['value'] = $website;
$expectedProperties[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope;
$expectedProperties[IAccountManager::PROPERTY_ADDRESS]['value'] = $address;
$expectedProperties[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope;
$expectedProperties[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter;
$expectedProperties[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope;
$this->mailer->expects($this->once())->method('validateMailAddress')
->willReturn(true);
$controller->expects($this->once())
->method('saveUserSettings')
->with($user, $expectedProperties)
->willReturnArgument(1);
->method('saveUserSettings');
$result = $controller->setUserSettings(
$controller->setUserSettings(
$avatarScope,
$displayName,
$displayNameScope,
@ -369,12 +420,13 @@ class UsersControllerTest extends \Test\TestCase {
$this->userSession->method('getUser')->willReturn($user);
$defaultProperties = $this->getDefaultAccountManagerUserData();
$defaultProperties = []; //$this->getDefaultAccountMock();
$userAccount = $this->getDefaultAccountMock();
$this->accountManager->expects($this->once())
->method('getUser')
->method('getAccount')
->with($user)
->willReturn($defaultProperties);
->willReturn($userAccount);
$this->appManager->expects($this->any())
->method('isEnabledForUser')
@ -417,10 +469,9 @@ class UsersControllerTest extends \Test\TestCase {
$controller->expects($this->once())
->method('saveUserSettings')
->with($user, $expectedProperties)
->willReturnArgument(1);
->with($userAccount);
$result = $controller->setUserSettings(
$controller->setUserSettings(
$avatarScope,
$displayName,
$displayNameScope,
@ -450,12 +501,13 @@ class UsersControllerTest extends \Test\TestCase {
$this->userSession->method('getUser')->willReturn($user);
$defaultProperties = $this->getDefaultAccountManagerUserData();
/** @var IAccount|MockObject $userAccount */
$userAccount = $this->getDefaultAccountMock();
$this->accountManager->expects($this->once())
->method('getUser')
->method('getAccount')
->with($user)
->willReturn($defaultProperties);
->willReturn($userAccount);
$avatarScope = ($property === 'avatarScope') ? $propertyValue : null;
$displayName = ($property === 'displayName') ? $propertyValue : null;
@ -471,46 +523,43 @@ class UsersControllerTest extends \Test\TestCase {
$twitter = ($property === 'twitter') ? $propertyValue : null;
$twitterScope = ($property === 'twitterScope') ? $propertyValue : null;
$expectedProperties = $defaultProperties;
if ($property === 'avatarScope') {
$expectedProperties[IAccountManager::PROPERTY_AVATAR]['scope'] = $propertyValue;
}
if ($property === 'displayName') {
$expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $propertyValue;
}
if ($property === 'displayNameScope') {
$expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $propertyValue;
}
if ($property === 'phone') {
$expectedProperties[IAccountManager::PROPERTY_PHONE]['value'] = $propertyValue;
}
if ($property === 'phoneScope') {
$expectedProperties[IAccountManager::PROPERTY_PHONE]['scope'] = $propertyValue;
}
if ($property === 'email') {
$expectedProperties[IAccountManager::PROPERTY_EMAIL]['value'] = $propertyValue;
}
if ($property === 'emailScope') {
$expectedProperties[IAccountManager::PROPERTY_EMAIL]['scope'] = $propertyValue;
}
if ($property === 'website') {
$expectedProperties[IAccountManager::PROPERTY_WEBSITE]['value'] = $propertyValue;
}
if ($property === 'websiteScope') {
$expectedProperties[IAccountManager::PROPERTY_WEBSITE]['scope'] = $propertyValue;
}
if ($property === 'address') {
$expectedProperties[IAccountManager::PROPERTY_ADDRESS]['value'] = $propertyValue;
}
if ($property === 'addressScope') {
$expectedProperties[IAccountManager::PROPERTY_ADDRESS]['scope'] = $propertyValue;
}
if ($property === 'twitter') {
$expectedProperties[IAccountManager::PROPERTY_TWITTER]['value'] = $propertyValue;
}
if ($property === 'twitterScope') {
$expectedProperties[IAccountManager::PROPERTY_TWITTER]['scope'] = $propertyValue;
/** @var IAccountProperty[]|MockObject[] $expectedProperties */
$expectedProperties = $userAccount->getProperties();
$isScope = strrpos($property, 'Scope') === strlen($property) - strlen(5);
switch ($property) {
case 'avatarScope':
$propertyId = IAccountManager::PROPERTY_AVATAR;
break;
case 'displayName':
case 'displayNameScope':
$propertyId = IAccountManager::PROPERTY_DISPLAYNAME;
break;
case 'phone':
case 'phoneScope':
$propertyId = IAccountManager::PROPERTY_PHONE;
break;
case 'email':
case 'emailScope':
$propertyId = IAccountManager::PROPERTY_EMAIL;
break;
case 'website':
case 'websiteScope':
$propertyId = IAccountManager::PROPERTY_WEBSITE;
break;
case 'address':
case 'addressScope':
$propertyId = IAccountManager::PROPERTY_ADDRESS;
break;
case 'twitter':
case 'twitterScope':
$propertyId = IAccountManager::PROPERTY_TWITTER;
break;
default:
$propertyId = '404';
}
$expectedProperties[$propertyId]->expects($this->any())
->method($isScope ? 'getScope' : 'getValue')
->willReturn($propertyValue);
if (!empty($email)) {
$this->mailer->expects($this->once())->method('validateMailAddress')
@ -519,10 +568,9 @@ class UsersControllerTest extends \Test\TestCase {
$controller->expects($this->once())
->method('saveUserSettings')
->with($user, $expectedProperties)
->willReturnArgument(1);
->with($userAccount);
$result = $controller->setUserSettings(
$controller->setUserSettings(
$avatarScope,
$displayName,
$displayNameScope,
@ -593,10 +641,33 @@ class UsersControllerTest extends \Test\TestCase {
->willReturn(true);
}
$this->accountManager->expects($this->once())->method('updateUser')
->with($user, $data);
$properties = [];
foreach ($data as $propertyName => $propertyData) {
$properties[$propertyName] = $this->createMock(IAccountProperty::class);
$properties[$propertyName]->expects($this->any())
->method('getValue')
->willReturn($propertyData['value']);
}
$account = $this->createMock(IAccount::class);
$account->expects($this->any())
->method('getUser')
->willReturn($user);
$account->expects($this->any())
->method('getProperty')
->willReturnCallback(function (string $propertyName) use ($properties) {
return $properties[$propertyName];
});
$this->accountManager->expects($this->any())
->method('getAccount')
->willReturn($account);
$this->accountManager->expects($this->once())
->method('updateAccount')
->with($account);
$this->invokePrivate($controller, 'saveUserSettings', [$user, $data]);
$this->invokePrivate($controller, 'saveUserSettings', [$account]);
}
public function dataTestSaveUserSettings() {
@ -655,21 +726,15 @@ class UsersControllerTest extends \Test\TestCase {
/**
* @dataProvider dataTestSaveUserSettingsException
*
* @param array $data
* @param string $oldEmailAddress
* @param string $oldDisplayName
* @param bool $setDisplayNameResult
* @param bool $canChangeEmail
*
*/
public function testSaveUserSettingsException($data,
$oldEmailAddress,
$oldDisplayName,
$setDisplayNameResult,
$canChangeEmail
public function testSaveUserSettingsException(
array $data,
string $oldEmailAddress,
string $oldDisplayName,
bool $setDisplayNameResult,
bool $canChangeEmail
) {
$this->expectException(\OC\ForbiddenException::class);
$this->expectException(ForbiddenException::class);
$controller = $this->getController();
$user = $this->createMock(IUser::class);
@ -677,6 +742,22 @@ class UsersControllerTest extends \Test\TestCase {
$user->method('getDisplayName')->willReturn($oldDisplayName);
$user->method('getEMailAddress')->willReturn($oldEmailAddress);
/** @var MockObject|IAccount $userAccount */
$userAccount = $this->createMock(IAccount::class);
$userAccount->expects($this->any())
->method('getUser')
->willReturn($user);
$propertyMocks = [];
foreach ($data as $propertyName => $propertyData) {
/** @var MockObject|IAccountProperty $property */
$propertyMocks[$propertyName] = $this->buildPropertyMock($propertyName, $propertyData['value'], '');
}
$userAccount->expects($this->any())
->method('getProperty')
->willReturnCallback(function (string $propertyName) use ($propertyMocks) {
return $propertyMocks[$propertyName];
});
if ($data[IAccountManager::PROPERTY_EMAIL]['value'] !== $oldEmailAddress) {
$user->method('canChangeDisplayName')
->willReturn($canChangeEmail);
@ -688,7 +769,7 @@ class UsersControllerTest extends \Test\TestCase {
->willReturn($setDisplayNameResult);
}
$this->invokePrivate($controller, 'saveUserSettings', [$user, $data]);
$this->invokePrivate($controller, 'saveUserSettings', [$userAccount]);
}
@ -748,15 +829,34 @@ class UsersControllerTest extends \Test\TestCase {
$controller = $this->getController(false, ['signMessage', 'getCurrentTime']);
$user = $this->createMock(IUser::class);
$property = $this->buildPropertyMock($type, $dataBefore[$type]['value'], '', IAccountManager::NOT_VERIFIED);
$property->expects($this->atLeastOnce())
->method('setVerified')
->with(IAccountManager::VERIFICATION_IN_PROGRESS)
->willReturnSelf();
$property->expects($this->atLeastOnce())
->method('setVerificationData')
->with($signature)
->willReturnSelf();
$userAccount = $this->createMock(IAccount::class);
$userAccount->expects($this->any())
->method('getUser')
->willReturn($user);
$userAccount->expects($this->any())
->method('getProperty')
->willReturn($property);
$this->userSession->expects($this->once())->method('getUser')->willReturn($user);
$this->accountManager->expects($this->once())->method('getUser')->with($user)->willReturn($dataBefore);
$this->accountManager->expects($this->once())->method('getAccount')->with($user)->willReturn($userAccount);
$user->expects($this->any())->method('getCloudId')->willReturn('user@nextcloud.com');
$user->expects($this->any())->method('getUID')->willReturn('uid');
$controller->expects($this->once())->method('signMessage')->with($user, $message)->willReturn($signature);
$controller->expects($this->any())->method('getCurrentTime')->willReturn(1234567);
if ($onlyVerificationCode === false) {
$this->accountManager->expects($this->once())->method('updateUser')->with($user, $expectedData)->willReturnArgument(1);
$this->accountManager->expects($this->once())->method('updateAccount')->with($userAccount)->willReturnArgument(1);
$this->jobList->expects($this->once())->method('add')
->with('OCA\Settings\BackgroundJobs\VerifyUserData',
[

@ -44,8 +44,8 @@ class Account implements IAccount {
$this->user = $user;
}
public function setProperty(string $property, string $value, string $scope, string $verified): IAccount {
$this->properties[$property] = new AccountProperty($property, $value, $scope, $verified);
public function setProperty(string $property, string $value, string $scope, string $verified, string $verificationData = ''): IAccount {
$this->properties[$property] = new AccountProperty($property, $value, $scope, $verified, $verificationData);
return $this;
}

@ -39,12 +39,15 @@ class AccountProperty implements IAccountProperty {
private $scope;
/** @var string */
private $verified;
/** @var string */
private $verificationData;
public function __construct(string $name, string $value, string $scope, string $verified) {
public function __construct(string $name, string $value, string $scope, string $verified, string $verificationData) {
$this->name = $name;
$this->value = $value;
$this->setScope($scope);
$this->verified = $verified;
$this->verificationData = $verificationData;
}
public function jsonSerialize() {
@ -52,7 +55,8 @@ class AccountProperty implements IAccountProperty {
'name' => $this->getName(),
'value' => $this->getValue(),
'scope' => $this->getScope(),
'verified' => $this->getVerified()
'verified' => $this->getVerified(),
'verificationData' => $this->getVerificationData(),
];
}
@ -164,4 +168,13 @@ class AccountProperty implements IAccountProperty {
public function getVerified(): string {
return $this->verified;
}
public function setVerificationData(string $verificationData): IAccountProperty {
$this->verificationData = $verificationData;
return $this;
}
public function getVerificationData(): string {
return $this->verificationData;
}
}

@ -44,9 +44,10 @@ interface IAccount extends \JsonSerializable {
* @param string $value
* @param string $scope Must be one of the VISIBILITY_ prefixed constants of \OCP\Accounts\IAccountManager
* @param string $verified \OCP\Accounts\IAccountManager::NOT_VERIFIED | \OCP\Accounts\IAccountManager::VERIFICATION_IN_PROGRESS | \OCP\Accounts\IAccountManager::VERIFIED
* @param string $verificationData Optional, defaults to empty string. Since @22.0.0.
* @return IAccount
*/
public function setProperty(string $property, string $value, string $scope, string $verified): IAccount;
public function setProperty(string $property, string $value, string $scope, string $verified, string $verificationData = ''): IAccount;
/**
* Get a property by its key
@ -60,7 +61,7 @@ interface IAccount extends \JsonSerializable {
public function getProperty(string $property): IAccountProperty;
/**
* Get all properties of an account
* Get all properties of an account. Array indices are property names.
*
* @since 15.0.0
*

@ -101,4 +101,18 @@ interface IAccountProperty extends \JsonSerializable {
* @return string
*/
public function getVerified(): string;
/**
* Sets data for verification purposes.
*
* @since 22.0.0
*/
public function setVerificationData(string $verificationData): IAccountProperty;
/**
* Retrieves data for verification purposes.
*
* @since 22.0.0
*/
public function getVerificationData(): string;
}

@ -106,6 +106,7 @@ class AccountManagerTest extends TestCase {
/** @var IUser $user */
$user = $this->createMock(IUser::class);
// FIXME: should be an integration test instead of this abomination
$accountManager->expects($this->once())->method('getUser')->with($user)->willReturn($oldData);
if ($updateExisting) {
@ -147,9 +148,9 @@ class AccountManagerTest extends TestCase {
public function dataTrueFalse() {
return [
[['newData'], ['oldData'], false, true],
[['newData'], [], true, false],
[['oldData'], ['oldData'], false, false]
[['myProperty' => ['value' => 'newData']], ['myProperty' => ['value' => 'oldData']], false, true],
[['myProperty' => ['value' => 'newData']], [], true, false],
[['myProperty' => ['value' => 'oldData']], ['myProperty' => ['value' => 'oldData']], false, false]
];
}

@ -38,7 +38,8 @@ class AccountPropertyTest extends TestCase {
IAccountManager::PROPERTY_WEBSITE,
'https://example.com',
IAccountManager::SCOPE_PUBLISHED,
IAccountManager::VERIFIED
IAccountManager::VERIFIED,
''
);
$this->assertEquals(IAccountManager::PROPERTY_WEBSITE, $accountProperty->getName());
$this->assertEquals('https://example.com', $accountProperty->getValue());
@ -51,7 +52,8 @@ class AccountPropertyTest extends TestCase {
IAccountManager::PROPERTY_WEBSITE,
'https://example.com',
IAccountManager::SCOPE_PUBLISHED,
IAccountManager::VERIFIED
IAccountManager::VERIFIED,
''
);
$actualReturn = $accountProperty->setValue('https://example.org');
$this->assertEquals('https://example.org', $accountProperty->getValue());
@ -63,7 +65,8 @@ class AccountPropertyTest extends TestCase {
IAccountManager::PROPERTY_WEBSITE,
'https://example.com',
IAccountManager::SCOPE_PUBLISHED,
IAccountManager::VERIFIED
IAccountManager::VERIFIED,
''
);
$actualReturn = $accountProperty->setScope(IAccountManager::SCOPE_LOCAL);
$this->assertEquals(IAccountManager::SCOPE_LOCAL, $accountProperty->getScope());
@ -98,7 +101,8 @@ class AccountPropertyTest extends TestCase {
IAccountManager::PROPERTY_WEBSITE,
'https://example.com',
$storedScope,
IAccountManager::VERIFIED
IAccountManager::VERIFIED,
''
);
$this->assertEquals($returnedScope, $accountProperty->getScope());
}
@ -108,25 +112,42 @@ class AccountPropertyTest extends TestCase {
IAccountManager::PROPERTY_WEBSITE,
'https://example.com',
IAccountManager::SCOPE_PUBLISHED,
IAccountManager::VERIFIED
IAccountManager::VERIFIED,
''
);
$actualReturn = $accountProperty->setVerified(IAccountManager::NOT_VERIFIED);
$this->assertEquals(IAccountManager::NOT_VERIFIED, $accountProperty->getVerified());
$this->assertEquals(IAccountManager::NOT_VERIFIED, $actualReturn->getVerified());
}
public function testSetVerificationData() {
$accountProperty = new AccountProperty(
IAccountManager::PROPERTY_WEBSITE,
'https://example.com',
IAccountManager::SCOPE_PUBLISHED,
IAccountManager::VERIFIED,
''
);
$token = uniqid();
$actualReturn = $accountProperty->setVerificationData($token);
$this->assertEquals($token, $accountProperty->getVerificationData());
$this->assertEquals($token, $actualReturn->getVerificationData());
}
public function testJsonSerialize() {
$accountProperty = new AccountProperty(
IAccountManager::PROPERTY_WEBSITE,
'https://example.com',
IAccountManager::SCOPE_PUBLISHED,
IAccountManager::VERIFIED
IAccountManager::VERIFIED,
'60a7a633b74af',
);
$this->assertEquals([
'name' => IAccountManager::PROPERTY_WEBSITE,
'value' => 'https://example.com',
'scope' => IAccountManager::SCOPE_PUBLISHED,
'verified' => IAccountManager::VERIFIED
'verified' => IAccountManager::VERIFIED,
'verificationData' => '60a7a633b74af'
], $accountProperty->jsonSerialize());
}
}

@ -43,7 +43,7 @@ class AccountTest extends TestCase {
public function testSetProperty() {
$user = $this->createMock(IUser::class);
$property = new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED);
$property = new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED, '');
$account = new Account($user);
$account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED);
$this->assertEquals($property, $account->getProperty(IAccountManager::PROPERTY_WEBSITE));
@ -52,8 +52,8 @@ class AccountTest extends TestCase {
public function testGetProperties() {
$user = $this->createMock(IUser::class);
$properties = [
IAccountManager::PROPERTY_WEBSITE => new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED),
IAccountManager::PROPERTY_EMAIL => new AccountProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::VERIFIED)
IAccountManager::PROPERTY_WEBSITE => new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED, ''),
IAccountManager::PROPERTY_EMAIL => new AccountProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::VERIFIED, '')
];
$account = new Account($user);
$account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED);
@ -65,9 +65,9 @@ class AccountTest extends TestCase {
public function testGetFilteredProperties() {
$user = $this->createMock(IUser::class);
$properties = [
IAccountManager::PROPERTY_WEBSITE => new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED),
IAccountManager::PROPERTY_EMAIL => new AccountProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::VERIFIED),
IAccountManager::PROPERTY_PHONE => new AccountProperty(IAccountManager::PROPERTY_PHONE, '123456', IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED),
IAccountManager::PROPERTY_WEBSITE => new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED, ''),
IAccountManager::PROPERTY_EMAIL => new AccountProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::VERIFIED, ''),
IAccountManager::PROPERTY_PHONE => new AccountProperty(IAccountManager::PROPERTY_PHONE, '123456', IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED, ''),
];
$account = new Account($user);
$account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED);
@ -98,8 +98,8 @@ class AccountTest extends TestCase {
public function testJsonSerialize() {
$user = $this->createMock(IUser::class);
$properties = [
IAccountManager::PROPERTY_WEBSITE => new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED),
IAccountManager::PROPERTY_EMAIL => new AccountProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::VERIFIED)
IAccountManager::PROPERTY_WEBSITE => new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED, ''),
IAccountManager::PROPERTY_EMAIL => new AccountProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::VERIFIED, '')
];
$account = new Account($user);
$account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED);

Loading…
Cancel
Save