diff --git a/apps/dav/lib/CardDAV/Converter.php b/apps/dav/lib/CardDAV/Converter.php index 59e5401d05..95ac43aba3 100644 --- a/apps/dav/lib/CardDAV/Converter.php +++ b/apps/dav/lib/CardDAV/Converter.php @@ -71,8 +71,8 @@ class Converter { foreach ($userData as $property => $value) { $shareWithTrustedServers = - $value['scope'] === AccountManager::VISIBILITY_CONTACTS_ONLY || - $value['scope'] === AccountManager::VISIBILITY_PUBLIC; + $value['scope'] === AccountManager::SCOPE_FEDERATED || + $value['scope'] === AccountManager::SCOPE_PUBLISHED; $emptyValue = !isset($value['value']) || $value['value'] === ''; diff --git a/apps/dav/tests/unit/CardDAV/ConverterTest.php b/apps/dav/tests/unit/CardDAV/ConverterTest.php index aef5cf8ef1..4334445143 100644 --- a/apps/dav/tests/unit/CardDAV/ConverterTest.php +++ b/apps/dav/tests/unit/CardDAV/ConverterTest.php @@ -53,36 +53,36 @@ class ConverterTest extends TestCase { IAccountManager::PROPERTY_DISPLAYNAME => [ 'value' => $user->getDisplayName(), - 'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY, + 'scope' => AccountManager::SCOPE_FEDERATED, ], IAccountManager::PROPERTY_ADDRESS => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, ], IAccountManager::PROPERTY_WEBSITE => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, ], IAccountManager::PROPERTY_EMAIL => [ 'value' => $user->getEMailAddress(), - 'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY, + 'scope' => AccountManager::SCOPE_FEDERATED, ], IAccountManager::PROPERTY_AVATAR => [ - 'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY + 'scope' => AccountManager::SCOPE_FEDERATED ], IAccountManager::PROPERTY_PHONE => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, ], IAccountManager::PROPERTY_TWITTER => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, ], ] ); diff --git a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php index eb8186807c..724670bc98 100644 --- a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php +++ b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php @@ -136,36 +136,36 @@ class SyncServiceTest extends TestCase { IAccountManager::PROPERTY_DISPLAYNAME => [ 'value' => $user->getDisplayName(), - 'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY, + 'scope' => AccountManager::SCOPE_FEDERATED, ], IAccountManager::PROPERTY_ADDRESS => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, ], IAccountManager::PROPERTY_WEBSITE => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, ], IAccountManager::PROPERTY_EMAIL => [ 'value' => $user->getEMailAddress(), - 'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY, + 'scope' => AccountManager::SCOPE_FEDERATED, ], IAccountManager::PROPERTY_AVATAR => [ - 'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY + 'scope' => AccountManager::SCOPE_FEDERATED ], IAccountManager::PROPERTY_PHONE => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, ], IAccountManager::PROPERTY_TWITTER => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, ], ] ); diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 31f13ee275..7e83ffaa7d 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -343,7 +343,7 @@ class ShareController extends AuthPublicShareController { $ownerAccount = $this->accountManager->getAccount($owner); $ownerName = $ownerAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME); - if ($ownerName->getScope() === IAccountManager::VISIBILITY_PUBLIC) { + if ($ownerName->getScope() === IAccountManager::SCOPE_PUBLISHED) { $shareTmpl['owner'] = $owner->getUID(); $shareTmpl['shareOwner'] = $owner->getDisplayName(); } diff --git a/apps/files_sharing/tests/Controller/ShareControllerTest.php b/apps/files_sharing/tests/Controller/ShareControllerTest.php index 270f38a114..e00d6bc8c6 100644 --- a/apps/files_sharing/tests/Controller/ShareControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareControllerTest.php @@ -234,7 +234,7 @@ class ShareControllerTest extends \Test\TestCase { $accountName = $this->createMock(IAccountProperty::class); $accountName->method('getScope') - ->willReturn(IAccountManager::VISIBILITY_PUBLIC); + ->willReturn(IAccountManager::SCOPE_PUBLISHED); $account = $this->createMock(IAccount::class); $account->method('getProperty') ->with(IAccountManager::PROPERTY_DISPLAYNAME) @@ -381,7 +381,7 @@ class ShareControllerTest extends \Test\TestCase { $accountName = $this->createMock(IAccountProperty::class); $accountName->method('getScope') - ->willReturn(IAccountManager::VISIBILITY_PRIVATE); + ->willReturn(IAccountManager::SCOPE_LOCAL); $account = $this->createMock(IAccount::class); $account->method('getProperty') ->with(IAccountManager::PROPERTY_DISPLAYNAME) @@ -528,7 +528,7 @@ class ShareControllerTest extends \Test\TestCase { $accountName = $this->createMock(IAccountProperty::class); $accountName->method('getScope') - ->willReturn(IAccountManager::VISIBILITY_PUBLIC); + ->willReturn(IAccountManager::SCOPE_PUBLISHED); $account = $this->createMock(IAccount::class); $account->method('getProperty') ->with(IAccountManager::PROPERTY_DISPLAYNAME) @@ -688,7 +688,7 @@ class ShareControllerTest extends \Test\TestCase { $accountName = $this->createMock(IAccountProperty::class); $accountName->method('getScope') - ->willReturn(IAccountManager::VISIBILITY_PUBLIC); + ->willReturn(IAccountManager::SCOPE_PUBLISHED); $account = $this->createMock(IAccount::class); $account->method('getProperty') ->with(IAccountManager::PROPERTY_DISPLAYNAME) diff --git a/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php b/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php index 889fcfd627..c462eeedb4 100644 --- a/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php +++ b/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php @@ -193,7 +193,7 @@ class RetryJob extends Job { $publicData = []; foreach ($account->getProperties() as $property) { - if ($property->getScope() === IAccountManager::VISIBILITY_PUBLIC) { + if ($property->getScope() === IAccountManager::SCOPE_PUBLISHED) { $publicData[$property->getName()] = $property->getValue(); } } diff --git a/apps/provisioning_api/composer/composer/autoload_classmap.php b/apps/provisioning_api/composer/composer/autoload_classmap.php index e94a97c194..22927806e6 100644 --- a/apps/provisioning_api/composer/composer/autoload_classmap.php +++ b/apps/provisioning_api/composer/composer/autoload_classmap.php @@ -8,6 +8,7 @@ $baseDir = $vendorDir; return array( 'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php', 'OCA\\Provisioning_API\\AppInfo\\Application' => $baseDir . '/../lib/AppInfo/Application.php', + 'OCA\\Provisioning_API\\Capabilities' => $baseDir . '/../lib/Capabilities.php', 'OCA\\Provisioning_API\\Controller\\AUserData' => $baseDir . '/../lib/Controller/AUserData.php', 'OCA\\Provisioning_API\\Controller\\AppConfigController' => $baseDir . '/../lib/Controller/AppConfigController.php', 'OCA\\Provisioning_API\\Controller\\AppsController' => $baseDir . '/../lib/Controller/AppsController.php', diff --git a/apps/provisioning_api/composer/composer/autoload_static.php b/apps/provisioning_api/composer/composer/autoload_static.php index b982f20321..f5a4b73f4f 100644 --- a/apps/provisioning_api/composer/composer/autoload_static.php +++ b/apps/provisioning_api/composer/composer/autoload_static.php @@ -23,6 +23,7 @@ class ComposerStaticInitProvisioning_API public static $classMap = array ( 'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php', 'OCA\\Provisioning_API\\AppInfo\\Application' => __DIR__ . '/..' . '/../lib/AppInfo/Application.php', + 'OCA\\Provisioning_API\\Capabilities' => __DIR__ . '/..' . '/../lib/Capabilities.php', 'OCA\\Provisioning_API\\Controller\\AUserData' => __DIR__ . '/..' . '/../lib/Controller/AUserData.php', 'OCA\\Provisioning_API\\Controller\\AppConfigController' => __DIR__ . '/..' . '/../lib/Controller/AppConfigController.php', 'OCA\\Provisioning_API\\Controller\\AppsController' => __DIR__ . '/..' . '/../lib/Controller/AppsController.php', diff --git a/apps/provisioning_api/lib/AppInfo/Application.php b/apps/provisioning_api/lib/AppInfo/Application.php index 7ec21c3329..af6b2b3371 100644 --- a/apps/provisioning_api/lib/AppInfo/Application.php +++ b/apps/provisioning_api/lib/AppInfo/Application.php @@ -29,6 +29,7 @@ namespace OCA\Provisioning_API\AppInfo; use OC\Group\Manager as GroupManager; +use OCA\Provisioning_API\Capabilities; use OCA\Provisioning_API\Listener\UserDeletedListener; use OCA\Provisioning_API\Middleware\ProvisioningApiMiddleware; use OCA\Settings\Mailer\NewUserMailHelper; @@ -92,6 +93,7 @@ class Application extends App implements IBootstrap { ); }); $context->registerMiddleware(ProvisioningApiMiddleware::class); + $context->registerCapability(Capabilities::class); } public function boot(IBootContext $context): void { diff --git a/apps/provisioning_api/lib/Capabilities.php b/apps/provisioning_api/lib/Capabilities.php new file mode 100644 index 0000000000..d355e4db4c --- /dev/null +++ b/apps/provisioning_api/lib/Capabilities.php @@ -0,0 +1,62 @@ + + * + * @author Vincent Petry + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace OCA\Provisioning_API; + +use OCA\FederatedFileSharing\FederatedShareProvider; +use OCP\App\IAppManager; +use OCP\Capabilities\ICapability; + +class Capabilities implements ICapability { + + /** @var IAppManager */ + private $appManager; + + public function __construct(IAppManager $appManager) { + $this->appManager = $appManager; + } + + /** + * Function an app uses to return the capabilities + * + * @return array Array containing the apps capabilities + */ + public function getCapabilities() { + $federationScopesEnabled = false; + + $federatedFileSharingEnabled = $this->appManager->isEnabledForUser('federatedfilesharing'); + if ($federatedFileSharingEnabled) { + /** @var FederatedShareProvider $shareProvider */ + $shareProvider = \OC::$server->query(FederatedShareProvider::class); + $federationScopesEnabled = $shareProvider->isLookupServerUploadEnabled(); + } + + return [ + 'provisioning_api' => [ + 'version' => $this->appManager->getAppVersion('provisioning_api'), + 'AccountPropertyScopesVersion' => 2, + 'AccountPropertyScopesFederationEnabled' => $federationScopesEnabled, + ] + ]; + } +} diff --git a/apps/provisioning_api/lib/Controller/AUserData.php b/apps/provisioning_api/lib/Controller/AUserData.php index b7b31b18b5..1280ef2136 100644 --- a/apps/provisioning_api/lib/Controller/AUserData.php +++ b/apps/provisioning_api/lib/Controller/AUserData.php @@ -50,6 +50,7 @@ use OCP\User\Backend\ISetDisplayNameBackend; use OCP\User\Backend\ISetPasswordBackend; abstract class AUserData extends OCSController { + public const SCOPE_SUFFIX = 'Scope'; /** @var IUserManager */ protected $userManager; @@ -86,12 +87,13 @@ abstract class AUserData extends OCSController { * creates a array with all user data * * @param string $userId + * @param bool $includeScopes * @return array * @throws NotFoundException * @throws OCSException * @throws OCSNotFoundException */ - protected function getUserData(string $userId): array { + protected function getUserData(string $userId, bool $includeScopes = false): array { $currentLoggedInUser = $this->userSession->getUser(); $data = []; @@ -114,7 +116,7 @@ abstract class AUserData extends OCSController { } // Get groups data - $userAccount = $this->accountManager->getUser($targetUserObject); + $userAccount = $this->accountManager->getAccount($targetUserObject); $groups = $this->groupManager->getUserGroups($targetUserObject); $gids = []; foreach ($groups as $group) { @@ -136,12 +138,33 @@ abstract class AUserData extends OCSController { $data['backend'] = $targetUserObject->getBackendClassName(); $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(); - $data[IAccountManager::PROPERTY_PHONE] = $userAccount[IAccountManager::PROPERTY_PHONE]['value']; - $data[IAccountManager::PROPERTY_ADDRESS] = $userAccount[IAccountManager::PROPERTY_ADDRESS]['value']; - $data[IAccountManager::PROPERTY_WEBSITE] = $userAccount[IAccountManager::PROPERTY_WEBSITE]['value']; - $data[IAccountManager::PROPERTY_TWITTER] = $userAccount[IAccountManager::PROPERTY_TWITTER]['value']; + 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(); + } + } + $data['groups'] = $gids; $data['language'] = $this->l10nFactory->getUserLanguage($targetUserObject); $data['locale'] = $this->config->getUserValue($targetUserObject->getUID(), 'core', 'locale'); diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index db4138054e..f933cd4cd7 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -50,7 +50,6 @@ use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\HintException; use OC\KnownUser\KnownUserService; -use OCA\Provisioning_API\FederatedShareProviderFactory; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\Accounts\IAccountManager; use OCP\App\IAppManager; @@ -84,8 +83,6 @@ class UsersController extends AUserData { protected $l10nFactory; /** @var NewUserMailHelper */ private $newUserMailHelper; - /** @var FederatedShareProviderFactory */ - private $federatedShareProviderFactory; /** @var ISecureRandom */ private $secureRandom; /** @var RemoteWipe */ @@ -107,7 +104,6 @@ class UsersController extends AUserData { ILogger $logger, IFactory $l10nFactory, NewUserMailHelper $newUserMailHelper, - FederatedShareProviderFactory $federatedShareProviderFactory, ISecureRandom $secureRandom, RemoteWipe $remoteWipe, KnownUserService $knownUserService, @@ -126,7 +122,6 @@ class UsersController extends AUserData { $this->logger = $logger; $this->l10nFactory = $l10nFactory; $this->newUserMailHelper = $newUserMailHelper; - $this->federatedShareProviderFactory = $federatedShareProviderFactory; $this->secureRandom = $secureRandom; $this->remoteWipe = $remoteWipe; $this->knownUserService = $knownUserService; @@ -475,7 +470,13 @@ class UsersController extends AUserData { * @throws OCSException */ public function getUser(string $userId): DataResponse { - $data = $this->getUserData($userId); + $includeScopes = false; + $currentUser = $this->userSession->getUser(); + if ($currentUser && $currentUser->getUID() === $userId) { + $includeScopes = true; + } + + $data = $this->getUserData($userId, $includeScopes); // getUserData returns empty array if not enough permissions if (empty($data)) { throw new OCSException('', \OCP\API::RESPOND_UNAUTHORISED); @@ -495,7 +496,7 @@ class UsersController extends AUserData { public function getCurrentUser(): DataResponse { $user = $this->userSession->getUser(); if ($user) { - $data = $this->getUserData($user->getUID()); + $data = $this->getUserData($user->getUID(), true); // rename "displayname" to "display-name" only for this call to keep // the API stable. $data['display-name'] = $data['displayname']; @@ -519,15 +520,10 @@ class UsersController extends AUserData { $permittedFields[] = IAccountManager::PROPERTY_EMAIL; } - if ($this->appManager->isEnabledForUser('federatedfilesharing')) { - $shareProvider = $this->federatedShareProviderFactory->get(); - if ($shareProvider->isLookupServerUploadEnabled()) { - $permittedFields[] = IAccountManager::PROPERTY_PHONE; - $permittedFields[] = IAccountManager::PROPERTY_ADDRESS; - $permittedFields[] = IAccountManager::PROPERTY_WEBSITE; - $permittedFields[] = IAccountManager::PROPERTY_TWITTER; - } - } + $permittedFields[] = IAccountManager::PROPERTY_PHONE; + $permittedFields[] = IAccountManager::PROPERTY_ADDRESS; + $permittedFields[] = IAccountManager::PROPERTY_WEBSITE; + $permittedFields[] = IAccountManager::PROPERTY_TWITTER; return new DataResponse($permittedFields); } @@ -562,6 +558,9 @@ class UsersController extends AUserData { $permittedFields[] = IAccountManager::PROPERTY_EMAIL; } + $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME . self::SCOPE_SUFFIX; + $permittedFields[] = IAccountManager::PROPERTY_EMAIL . self::SCOPE_SUFFIX; + $permittedFields[] = 'password'; if ($this->config->getSystemValue('force_language', false) === false || $this->groupManager->isAdmin($currentLoggedInUser->getUID())) { @@ -573,15 +572,16 @@ class UsersController extends AUserData { $permittedFields[] = 'locale'; } - if ($this->appManager->isEnabledForUser('federatedfilesharing')) { - $shareProvider = $this->federatedShareProviderFactory->get(); - if ($shareProvider->isLookupServerUploadEnabled()) { - $permittedFields[] = IAccountManager::PROPERTY_PHONE; - $permittedFields[] = IAccountManager::PROPERTY_ADDRESS; - $permittedFields[] = IAccountManager::PROPERTY_WEBSITE; - $permittedFields[] = IAccountManager::PROPERTY_TWITTER; - } - } + $permittedFields[] = IAccountManager::PROPERTY_PHONE; + $permittedFields[] = IAccountManager::PROPERTY_ADDRESS; + $permittedFields[] = IAccountManager::PROPERTY_WEBSITE; + $permittedFields[] = IAccountManager::PROPERTY_TWITTER; + $permittedFields[] = IAccountManager::PROPERTY_PHONE . self::SCOPE_SUFFIX; + $permittedFields[] = IAccountManager::PROPERTY_ADDRESS . self::SCOPE_SUFFIX; + $permittedFields[] = IAccountManager::PROPERTY_WEBSITE . self::SCOPE_SUFFIX; + $permittedFields[] = IAccountManager::PROPERTY_TWITTER . self::SCOPE_SUFFIX; + + $permittedFields[] = IAccountManager::PROPERTY_AVATAR . self::SCOPE_SUFFIX; // If admin they can edit their own quota if ($this->groupManager->isAdmin($currentLoggedInUser->getUID())) { @@ -686,6 +686,24 @@ class UsersController extends AUserData { } } break; + case IAccountManager::PROPERTY_DISPLAYNAME . self::SCOPE_SUFFIX: + case IAccountManager::PROPERTY_EMAIL . self::SCOPE_SUFFIX: + case IAccountManager::PROPERTY_PHONE . self::SCOPE_SUFFIX: + case IAccountManager::PROPERTY_ADDRESS . self::SCOPE_SUFFIX: + case IAccountManager::PROPERTY_WEBSITE . self::SCOPE_SUFFIX: + 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; + try { + $this->accountManager->updateUser($targetUser, $userAccount, true); + } catch (\InvalidArgumentException $e) { + throw new OCSException('Invalid ' . $e->getMessage(), 102); + } + } + break; default: throw new OCSException('', 103); } diff --git a/apps/provisioning_api/tests/CapabilitiesTest.php b/apps/provisioning_api/tests/CapabilitiesTest.php new file mode 100644 index 0000000000..89a4215d27 --- /dev/null +++ b/apps/provisioning_api/tests/CapabilitiesTest.php @@ -0,0 +1,92 @@ + + * + * @author Vincent Petry + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace OCA\Provisioning_API\Tests\unit; + +use OCA\FederatedFileSharing\FederatedShareProvider; +use OCA\Provisioning_API\Capabilities; +use OCP\App\IAppManager; +use PHPUnit\Framework\MockObject\MockObject; +use Test\TestCase; + +/** + * Capabilities test for provisioning API. + * + * Note: group DB needed because of usage of overwriteService() + * + * @package OCA\Provisioning_API\Tests + * @group DB + */ +class CapabilitiesTest extends TestCase { + + /** @var Capabilities */ + protected $capabilities; + + /** @var IAppManager|MockObject */ + protected $appManager; + + public function setUp(): void { + parent::setUp(); + $this->appManager = $this->createMock(IAppManager::class); + $this->capabilities = new Capabilities($this->appManager); + + $this->appManager->expects($this->once()) + ->method('getAppVersion') + ->with('provisioning_api') + ->willReturn('1.12'); + } + + public function getCapabilitiesProvider() { + return [ + [false, false, false], + [true, false, false], + [true, true, true], + ]; + } + + /** + * @dataProvider getCapabilitiesProvider + */ + public function testGetCapabilities($federationAppEnabled, $lookupServerEnabled, $expectedFederationScopesEnabled) { + $this->appManager->expects($this->once()) + ->method('isEnabledForUser') + ->with('federatedfilesharing') + ->willReturn($federationAppEnabled); + + $federatedShareProvider = $this->createMock(FederatedShareProvider::class); + $this->overwriteService(FederatedShareProvider::class, $federatedShareProvider); + + $federatedShareProvider->expects($this->any()) + ->method('isLookupServerUploadEnabled') + ->willReturn($lookupServerEnabled); + + $expected = [ + 'provisioning_api' => [ + 'version' => '1.12', + 'AccountPropertyScopesVersion' => 2, + 'AccountPropertyScopesFederationEnabled' => $expectedFederationScopesEnabled, + ], + ]; + $this->assertSame($expected, $this->capabilities->getCapabilities()); + } +} diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 39743579b7..2b4ca78062 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -46,11 +46,11 @@ use OC\Authentication\Token\RemoteWipe; use OC\Group\Manager; use OC\KnownUser\KnownUserService; use OC\SubAdmin; -use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\Provisioning_API\Controller\UsersController; -use OCA\Provisioning_API\FederatedShareProviderFactory; use OCA\Settings\Mailer\NewUserMailHelper; +use OCP\Accounts\IAccount; use OCP\Accounts\IAccountManager; +use OCP\Accounts\IAccountProperty; use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; use OCP\EventDispatcher\IEventDispatcher; @@ -97,8 +97,6 @@ class UsersControllerTest extends TestCase { private $l10nFactory; /** @var NewUserMailHelper|MockObject */ private $newUserMailHelper; - /** @var FederatedShareProviderFactory|MockObject */ - private $federatedShareProviderFactory; /** @var ISecureRandom|MockObject */ private $secureRandom; /** @var RemoteWipe|MockObject */ @@ -122,7 +120,6 @@ class UsersControllerTest extends TestCase { $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->l10nFactory = $this->createMock(IFactory::class); $this->newUserMailHelper = $this->createMock(NewUserMailHelper::class); - $this->federatedShareProviderFactory = $this->createMock(FederatedShareProviderFactory::class); $this->secureRandom = $this->createMock(ISecureRandom::class); $this->remoteWipe = $this->createMock(RemoteWipe::class); $this->knownUserService = $this->createMock(KnownUserService::class); @@ -142,7 +139,6 @@ class UsersControllerTest extends TestCase { $this->logger, $this->l10nFactory, $this->newUserMailHelper, - $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, $this->knownUserService, @@ -407,7 +403,6 @@ class UsersControllerTest extends TestCase { $this->logger, $this->l10nFactory, $this->newUserMailHelper, - $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, $this->knownUserService, @@ -933,7 +928,6 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $this->userSession - ->expects($this->once()) ->method('getUser') ->willReturn($loggedInUser); $this->userManager @@ -1003,16 +997,13 @@ class UsersControllerTest extends TestCase { $group->expects($this->at(3)) ->method('getGID') ->willReturn('group3'); - $this->accountManager->expects($this->any())->method('getUser') - ->with($targetUser) - ->willReturn( - [ - IAccountManager::PROPERTY_ADDRESS => ['value' => 'address'], - IAccountManager::PROPERTY_PHONE => ['value' => 'phone'], - IAccountManager::PROPERTY_TWITTER => ['value' => 'twitter'], - IAccountManager::PROPERTY_WEBSITE => ['value' => 'website'], - ] - ); + + $this->mockAccount($targetUser, [ + IAccountManager::PROPERTY_ADDRESS => ['value' => 'address'], + IAccountManager::PROPERTY_PHONE => ['value' => 'phone'], + IAccountManager::PROPERTY_TWITTER => ['value' => 'twitter'], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'website'], + ]); $this->config ->expects($this->at(0)) ->method('getUserValue') @@ -1172,16 +1163,13 @@ class UsersControllerTest extends TestCase { $targetUser ->method('getUID') ->willReturn('UID'); - $this->accountManager->expects($this->any())->method('getUser') - ->with($targetUser) - ->willReturn( - [ - IAccountManager::PROPERTY_ADDRESS => ['value' => 'address'], - IAccountManager::PROPERTY_PHONE => ['value' => 'phone'], - IAccountManager::PROPERTY_TWITTER => ['value' => 'twitter'], - IAccountManager::PROPERTY_WEBSITE => ['value' => 'website'], - ] - ); + + $this->mockAccount($targetUser, [ + IAccountManager::PROPERTY_ADDRESS => ['value' => 'address'], + IAccountManager::PROPERTY_PHONE => ['value' => 'phone'], + IAccountManager::PROPERTY_TWITTER => ['value' => 'twitter'], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'website'], + ]); $this->l10nFactory ->expects($this->once()) @@ -1224,14 +1212,13 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->exactly(2)) + ->expects($this->exactly(3)) ->method('getUID') ->willReturn('subadmin'); $targetUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); $this->userSession - ->expects($this->once()) ->method('getUser') ->willReturn($loggedInUser); $this->userManager @@ -1343,16 +1330,12 @@ class UsersControllerTest extends TestCase { ->expects($this->once()) ->method('getBackend') ->willReturn($backend); - $this->accountManager->expects($this->any())->method('getUser') - ->with($targetUser) - ->willReturn( - [ - IAccountManager::PROPERTY_ADDRESS => ['value' => 'address'], - IAccountManager::PROPERTY_PHONE => ['value' => 'phone'], - IAccountManager::PROPERTY_TWITTER => ['value' => 'twitter'], - IAccountManager::PROPERTY_WEBSITE => ['value' => 'website'], - ] - ); + $this->mockAccount($targetUser, [ + IAccountManager::PROPERTY_ADDRESS => ['value' => 'address'], + IAccountManager::PROPERTY_PHONE => ['value' => 'phone'], + IAccountManager::PROPERTY_TWITTER => ['value' => 'twitter'], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'website'], + ]); $this->l10nFactory ->expects($this->once()) @@ -1537,6 +1520,91 @@ class UsersControllerTest extends TestCase { $this->api->editUser('UserToEdit', 'email', 'demo.org'); } + public function selfEditChangePropertyProvider() { + return [ + [IAccountManager::PROPERTY_TWITTER, '@oldtwitter', '@newtwitter'], + [IAccountManager::PROPERTY_PHONE, '1234', '12345'], + [IAccountManager::PROPERTY_ADDRESS, 'Something street 2', 'Another street 3'], + [IAccountManager::PROPERTY_WEBSITE, 'https://examplesite1', 'https://examplesite2'], + ]; + } + + /** + * @dataProvider selfEditChangePropertyProvider + */ + public function testEditUserRegularUserSelfEditChangeProperty($propertyName, $oldValue, $newValue) { + $loggedInUser = $this->getMockBuilder(IUser::class) + ->disableOriginalConstructor() + ->getMock(); + $loggedInUser + ->expects($this->any()) + ->method('getUID') + ->willReturn('UID'); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($loggedInUser); + $this->userManager + ->expects($this->once()) + ->method('get') + ->with('UserToEdit') + ->willReturn($loggedInUser); + + $this->accountManager->expects($this->once()) + ->method('getUser') + ->with($loggedInUser) + ->willReturn([$propertyName => ['value' => $oldValue, 'scope' => IAccountManager::SCOPE_LOCAL]]); + $this->accountManager->expects($this->once()) + ->method('updateUser') + ->with($loggedInUser, [$propertyName => ['value' => $newValue, 'scope' => IAccountManager::SCOPE_LOCAL]], true); + + $this->assertEquals([], $this->api->editUser('UserToEdit', $propertyName, $newValue)->getData()); + } + + public function selfEditChangePropertyScopeProvider() { + return [ + [IAccountManager::PROPERTY_AVATAR, IAccountManager::SCOPE_LOCAL, IAccountManager::SCOPE_FEDERATED], + [IAccountManager::PROPERTY_DISPLAYNAME, IAccountManager::SCOPE_LOCAL, IAccountManager::SCOPE_FEDERATED], + [IAccountManager::PROPERTY_EMAIL, IAccountManager::SCOPE_LOCAL, IAccountManager::SCOPE_FEDERATED], + [IAccountManager::PROPERTY_TWITTER, IAccountManager::SCOPE_LOCAL, IAccountManager::SCOPE_FEDERATED], + [IAccountManager::PROPERTY_PHONE, IAccountManager::SCOPE_LOCAL, IAccountManager::SCOPE_FEDERATED], + [IAccountManager::PROPERTY_ADDRESS, IAccountManager::SCOPE_LOCAL, IAccountManager::SCOPE_FEDERATED], + [IAccountManager::PROPERTY_WEBSITE, IAccountManager::SCOPE_LOCAL, IAccountManager::SCOPE_FEDERATED], + ]; + } + + /** + * @dataProvider selfEditChangePropertyProvider + */ + public function testEditUserRegularUserSelfEditChangePropertyScope($propertyName, $oldScope, $newScope) { + $loggedInUser = $this->getMockBuilder(IUser::class) + ->disableOriginalConstructor() + ->getMock(); + $loggedInUser + ->expects($this->any()) + ->method('getUID') + ->willReturn('UID'); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($loggedInUser); + $this->userManager + ->expects($this->once()) + ->method('get') + ->with('UserToEdit') + ->willReturn($loggedInUser); + + $this->accountManager->expects($this->once()) + ->method('getUser') + ->with($loggedInUser) + ->willReturn([$propertyName => ['value' => 'somevalue', 'scope' => $oldScope]]); + $this->accountManager->expects($this->once()) + ->method('updateUser') + ->with($loggedInUser, [$propertyName => ['value' => 'somevalue', 'scope' => $newScope]], true); + + $this->assertEquals([], $this->api->editUser('UserToEdit', $propertyName . 'Scope', $newScope)->getData()); + } + public function testEditUserRegularUserSelfEditChangePassword() { $loggedInUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() @@ -3246,7 +3314,6 @@ class UsersControllerTest extends TestCase { $this->logger, $this->l10nFactory, $this->newUserMailHelper, - $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, $this->knownUserService, @@ -3255,7 +3322,7 @@ class UsersControllerTest extends TestCase { ->setMethods(['getUserData']) ->getMock(); - $api->expects($this->once())->method('getUserData')->with('UID') + $api->expects($this->once())->method('getUserData')->with('UID', true) ->willReturn( [ 'id' => 'UID', @@ -3296,8 +3363,15 @@ class UsersControllerTest extends TestCase { $this->api->getCurrentUser(); } - public function testGetUser() { + $loggedInUser = $this->createMock(IUser::class); + $loggedInUser + ->method('getUID') + ->willReturn('currentuser'); + $this->userSession + ->method('getUser') + ->willReturn($loggedInUser); + /** @var UsersController | MockObject $api */ $api = $this->getMockBuilder(UsersController::class) ->setConstructorArgs([ @@ -3313,7 +3387,6 @@ class UsersControllerTest extends TestCase { $this->logger, $this->l10nFactory, $this->newUserMailHelper, - $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, $this->knownUserService, @@ -3334,11 +3407,16 @@ class UsersControllerTest extends TestCase { 'displayname' => 'Demo User' ]; - $api->expects($this->once())->method('getUserData') - ->with('uid') + $api->expects($this->at(0))->method('getUserData') + ->with('uid', false) + ->willReturn($expected); + $api->expects($this->at(1))->method('getUserData') + ->with('currentuser', true) ->willReturn($expected); $this->assertSame($expected, $api->getUser('uid')->getData()); + + $this->assertSame($expected, $api->getUser('currentuser')->getData()); } @@ -3638,18 +3716,13 @@ class UsersControllerTest extends TestCase { public function dataGetEditableFields() { return [ - [false, false, []], - [false, true, [ + [false, [ IAccountManager::PROPERTY_PHONE, IAccountManager::PROPERTY_ADDRESS, IAccountManager::PROPERTY_WEBSITE, IAccountManager::PROPERTY_TWITTER, ]], - [ true, false, [ - IAccountManager::PROPERTY_DISPLAYNAME, - IAccountManager::PROPERTY_EMAIL, - ]], - [ true, true ,[ + [ true, [ IAccountManager::PROPERTY_DISPLAYNAME, IAccountManager::PROPERTY_EMAIL, IAccountManager::PROPERTY_PHONE, @@ -3664,29 +3737,36 @@ class UsersControllerTest extends TestCase { * @dataProvider dataGetEditableFields * * @param bool $allowedToChangeDisplayName - * @param bool $federatedSharingEnabled * @param array $expected */ - public function testGetEditableFields(bool $allowedToChangeDisplayName, bool $federatedSharingEnabled, array $expected) { + public function testGetEditableFields(bool $allowedToChangeDisplayName, array $expected) { $this->config ->method('getSystemValue') ->with( $this->equalTo('allow_user_to_change_display_name'), $this->anything() )->willReturn($allowedToChangeDisplayName); - $this->appManager - ->method('isEnabledForUser') - ->with($this->equalTo('federatedfilesharing')) - ->willReturn($federatedSharingEnabled); - - $shareprovider = $this->createMock(FederatedShareProvider::class); - $shareprovider->method('isLookupServerUploadEnabled')->willReturn(true); - - $this->federatedShareProviderFactory - ->method('get') - ->willReturn($shareprovider); $expectedResp = new DataResponse($expected); $this->assertEquals($expectedResp, $this->api->getEditableFields()); } + + private function mockAccount($targetUser, $accountProperties) { + $mockedProperties = []; + + foreach ($accountProperties as $propertyName => $data) { + $mockedProperty = $this->createMock(IAccountProperty::class); + $mockedProperty->method('getValue')->willReturn($data['value'] ?? ''); + $mockedProperty->method('getScope')->willReturn($data['scope'] ?? ''); + $mockedProperties[] = [$propertyName, $mockedProperty]; + } + + $account = $this->createMock(IAccount::class); + $account->method('getProperty') + ->will($this->returnValueMap($mockedProperties)); + + $this->accountManager->expects($this->any())->method('getAccount') + ->with($targetUser) + ->willReturn($account); + } } diff --git a/apps/settings/js/federationscopemenu.js b/apps/settings/js/federationscopemenu.js index 170aec15a8..617a8e3d41 100644 --- a/apps/settings/js/federationscopemenu.js +++ b/apps/settings/js/federationscopemenu.js @@ -15,6 +15,8 @@ * Construct a new FederationScopeMenu instance * @constructs FederationScopeMenu * @memberof OC.Settings + * @param {object} options + * @param {array.} [options.excludedScopes] array of excluded scopes */ var FederationScopeMenu = OC.Backbone.View.extend({ tagName: 'div', @@ -26,27 +28,40 @@ this.field = options.field; this._scopes = [ { - name: 'private', + name: 'v2-private', displayName: t('settings', 'Private'), + tooltip: t('settings', "Don't show via public link"), + iconClass: 'icon-password', + active: false + }, + { + name: 'v2-local', + displayName: t('settings', 'Local'), tooltip: t('settings', "Don't synchronize to servers"), iconClass: 'icon-password', active: false }, { - name: 'contacts', - displayName: t('settings', 'Trusted'), + name: 'v2-federated', + displayName: t('settings', 'Federated'), tooltip: t('settings', 'Only synchronize to trusted servers'), iconClass: 'icon-contacts-dark', active: false }, { - name: 'public', - displayName: t('settings', 'Public'), + name: 'v2-published', + displayName: t('settings', 'Published'), tooltip: t('settings', 'Synchronize to trusted servers and the global and public address book'), iconClass: 'icon-link', active: false } ]; + + if (options.excludedScopes && options.excludedScopes.length) { + this._scopes = this._scopes.filter(function(scopeEntry) { + return options.excludedScopes.indexOf(scopeEntry.name) === -1; + }) + } }, /** @@ -102,19 +117,11 @@ var currentlyActiveValue = $('#'+context.target.closest('form').id).find('input[type="hidden"]')[0].value; for(var i in this._scopes) { - this._scopes[i].active = false; - } - - switch (currentlyActiveValue) { - case 'private': - this._scopes[0].active = true; - break; - case 'contacts': - this._scopes[1].active = true; - break; - case 'public': - this._scopes[2].active = true; - break; + if (this._scopes[i].name === currentlyActiveValue) { + this._scopes[i].active = true; + } else { + this._scopes[i].active = false; + } } this.render(); diff --git a/apps/settings/js/federationsettingsview.js b/apps/settings/js/federationsettingsview.js index 9cefaf132f..cf7f464890 100644 --- a/apps/settings/js/federationsettingsview.js +++ b/apps/settings/js/federationsettingsview.js @@ -10,6 +10,13 @@ (function(_, $, OC) { 'use strict'; + /** + * Construct a new FederationScopeMenu instance + * @constructs FederationScopeMenu + * @memberof OC.Settings + * @param {object} options + * @param {bool} [options.lookupServerUploadEnabled=false] whether uploading to the lookup server is enabled + */ var FederationSettingsView = OC.Backbone.View.extend({ _inputFields: undefined, @@ -24,6 +31,7 @@ } else { this._config = new OC.Settings.UserSettings(); } + this.showFederationScopes = !!options.showFederationScopes; this._inputFields = [ 'displayname', @@ -61,9 +69,31 @@ render: function() { var self = this; + var fieldsWithV2Private = [ + 'avatar', + 'phone', + 'twitter', + 'website', + 'address', + ]; + _.each(this._inputFields, function(field) { var $icon = self.$('#' + field + 'form h3 > .federation-menu'); - var scopeMenu = new OC.Settings.FederationScopeMenu({field: field}); + var excludedScopes = [] + + if (fieldsWithV2Private.indexOf(field) === -1) { + excludedScopes.push('v2-private'); + } + + if (!self.showFederationScopes) { + excludedScopes.push('v2-federated'); + excludedScopes.push('v2-published'); + } + + var scopeMenu = new OC.Settings.FederationScopeMenu({ + field: field, + excludedScopes: excludedScopes, + }); self.listenTo(scopeMenu, 'select:scope', function(scope) { self._onScopeChanged(field, scope); @@ -207,15 +237,16 @@ $icon.addClass('hidden'); switch (scope) { - case 'private': + case 'v2-private': + case 'v2-local': $icon.addClass('icon-password'); $icon.removeClass('hidden'); break; - case 'contacts': + case 'v2-federated': $icon.addClass('icon-contacts-dark'); $icon.removeClass('hidden'); break; - case 'public': + case 'v2-published': $icon.addClass('icon-link'); $icon.removeClass('hidden'); break; diff --git a/apps/settings/js/settings/personalInfo.js b/apps/settings/js/settings/personalInfo.js index a6055fd7a9..e71f484012 100644 --- a/apps/settings/js/settings/personalInfo.js +++ b/apps/settings/js/settings/personalInfo.js @@ -199,10 +199,12 @@ window.addEventListener('DOMContentLoaded', function () { }); + var settingsEl = $('#personal-settings') var userSettings = new OC.Settings.UserSettings(); var federationSettingsView = new OC.Settings.FederationSettingsView({ - el: '#personal-settings', - config: userSettings + el: settingsEl, + config: userSettings, + showFederationScopes: !!settingsEl.data('lookup-server-upload-enabled'), }); userSettings.on("sync", function() { diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 02a0cda139..c59c16666f 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -46,7 +46,6 @@ use OC\KnownUser\KnownUserService; use OC\L10N\Factory; use OC\Security\IdentityProof\Manager; use OC\User\Manager as UserManager; -use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\Settings\BackgroundJobs\VerifyUserData; use OCA\Settings\Events\BeforeTemplateRenderedEvent; use OCA\User_LDAP\User_Proxy; @@ -401,15 +400,11 @@ class UsersController extends Controller { $data[IAccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope]; $data[IAccountManager::PROPERTY_EMAIL] = ['value' => $email, 'scope' => $emailScope]; } - if ($this->appManager->isEnabledForUser('federatedfilesharing')) { - $shareProvider = \OC::$server->query(FederatedShareProvider::class); - if ($shareProvider->isLookupServerUploadEnabled()) { - $data[IAccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope]; - $data[IAccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope]; - $data[IAccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope]; - $data[IAccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope]; - } - } + $data[IAccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope]; + $data[IAccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope]; + $data[IAccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope]; + $data[IAccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope]; + try { $data = $this->saveUserSettings($user, $data); if ($beforeData[IAccountManager::PROPERTY_PHONE]['value'] !== $data[IAccountManager::PROPERTY_PHONE]['value']) { diff --git a/apps/settings/lib/Settings/Personal/PersonalInfo.php b/apps/settings/lib/Settings/Personal/PersonalInfo.php index a853846fad..7a0253d2be 100644 --- a/apps/settings/lib/Settings/Personal/PersonalInfo.php +++ b/apps/settings/lib/Settings/Personal/PersonalInfo.php @@ -37,6 +37,7 @@ namespace OCA\Settings\Settings\Personal; use OC\Accounts\AccountManager; use OCA\FederatedFileSharing\FederatedShareProvider; +use OCP\Accounts\IAccount; use OCP\Accounts\IAccountManager; use OCP\App\IAppManager; use OCP\AppFramework\Http\TemplateResponse; @@ -96,7 +97,7 @@ class PersonalInfo implements ISettings { $uid = \OC_User::getUser(); $user = $this->userManager->get($uid); - $userData = $this->accountManager->getUser($user); + $account = $this->accountManager->getAccount($user); // make sure FS is setup before querying storage related stuff... \OC_Util::setupFS($user->getUID()); @@ -110,7 +111,7 @@ class PersonalInfo implements ISettings { $languageParameters = $this->getLanguages($user); $localeParameters = $this->getLocales($user); - $messageParameters = $this->getMessageParameters($userData); + $messageParameters = $this->getMessageParameters($account); $parameters = [ 'total_space' => $totalSpace, @@ -119,23 +120,23 @@ class PersonalInfo implements ISettings { 'quota' => $storageInfo['quota'], 'avatarChangeSupported' => $user->canChangeAvatar(), 'lookupServerUploadEnabled' => $lookupServerUploadEnabled, - 'avatarScope' => $userData[IAccountManager::PROPERTY_AVATAR]['scope'], + 'avatarScope' => $account->getProperty(IAccountManager::PROPERTY_AVATAR)->getScope(), 'displayNameChangeSupported' => $user->canChangeDisplayName(), - 'displayName' => $userData[IAccountManager::PROPERTY_DISPLAYNAME]['value'], - 'displayNameScope' => $userData[IAccountManager::PROPERTY_DISPLAYNAME]['scope'], - 'email' => $userData[IAccountManager::PROPERTY_EMAIL]['value'], - 'emailScope' => $userData[IAccountManager::PROPERTY_EMAIL]['scope'], - 'emailVerification' => $userData[IAccountManager::PROPERTY_EMAIL]['verified'], - 'phone' => $userData[IAccountManager::PROPERTY_PHONE]['value'], - 'phoneScope' => $userData[IAccountManager::PROPERTY_PHONE]['scope'], - 'address' => $userData[IAccountManager::PROPERTY_ADDRESS]['value'], - 'addressScope' => $userData[IAccountManager::PROPERTY_ADDRESS]['scope'], - 'website' => $userData[IAccountManager::PROPERTY_WEBSITE]['value'], - 'websiteScope' => $userData[IAccountManager::PROPERTY_WEBSITE]['scope'], - 'websiteVerification' => $userData[IAccountManager::PROPERTY_WEBSITE]['verified'], - 'twitter' => $userData[IAccountManager::PROPERTY_TWITTER]['value'], - 'twitterScope' => $userData[IAccountManager::PROPERTY_TWITTER]['scope'], - 'twitterVerification' => $userData[IAccountManager::PROPERTY_TWITTER]['verified'], + 'displayName' => $account->getProperty(IAccountManager::PROPERTY_DISPLAYNAME)->getValue(), + 'displayNameScope' => $account->getProperty(IAccountManager::PROPERTY_DISPLAYNAME)->getScope(), + 'email' => $account->getProperty(IAccountManager::PROPERTY_EMAIL)->getValue(), + 'emailScope' => $account->getProperty(IAccountManager::PROPERTY_EMAIL)->getScope(), + 'emailVerification' => $account->getProperty(IAccountManager::PROPERTY_EMAIL)->getVerified(), + 'phone' => $account->getProperty(IAccountManager::PROPERTY_PHONE)->getValue(), + 'phoneScope' => $account->getProperty(IAccountManager::PROPERTY_PHONE)->getScope(), + 'address' => $account->getProperty(IAccountManager::PROPERTY_ADDRESS)->getValue(), + 'addressScope' => $account->getProperty(IAccountManager::PROPERTY_ADDRESS)->getScope(), + 'website' => $account->getProperty(IAccountManager::PROPERTY_WEBSITE)->getValue(), + 'websiteScope' => $account->getProperty(IAccountManager::PROPERTY_WEBSITE)->getScope(), + 'websiteVerification' => $account->getProperty(IAccountManager::PROPERTY_WEBSITE)->getVerified(), + 'twitter' => $account->getProperty(IAccountManager::PROPERTY_TWITTER)->getValue(), + 'twitterScope' => $account->getProperty(IAccountManager::PROPERTY_TWITTER)->getScope(), + 'twitterVerification' => $account->getProperty(IAccountManager::PROPERTY_TWITTER)->getVerified(), 'groups' => $this->getGroups($user), ] + $messageParameters + $languageParameters + $localeParameters; @@ -263,14 +264,14 @@ class PersonalInfo implements ISettings { } /** - * @param array $userData + * @param IAccount $account * @return array */ - private function getMessageParameters(array $userData): array { + private function getMessageParameters(IAccount $account): array { $needVerifyMessage = [IAccountManager::PROPERTY_EMAIL, IAccountManager::PROPERTY_WEBSITE, IAccountManager::PROPERTY_TWITTER]; $messageParameters = []; foreach ($needVerifyMessage as $property) { - switch ($userData[$property]['verified']) { + switch ($account->getProperty($property)->getVerified()) { case AccountManager::VERIFIED: $message = $this->l->t('Verifying'); break; diff --git a/apps/settings/templates/settings/personal/personal.info.php b/apps/settings/templates/settings/personal/personal.info.php index 84198b3c0c..f2e3a51aad 100644 --- a/apps/settings/templates/settings/personal/personal.info.php +++ b/apps/settings/templates/settings/personal/personal.info.php @@ -34,8 +34,8 @@ script('settings', [ ]); ?> -
-
+
+

@@ -68,9 +68,7 @@ script('settings', [

- -
@@ -125,7 +123,7 @@ script('settings', [ - +
@@ -179,7 +177,6 @@ script('settings', [
-

@@ -190,21 +187,15 @@ script('settings', [

- + - -
- -

@@ -215,21 +206,15 @@ script('settings', [

- + - -
- -

@@ -273,19 +258,12 @@ script('settings', [ /> - -

- -

@@ -329,18 +307,12 @@ script('settings', [ /> - -

-
diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index b14e8d00d6..f9652053de 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -190,6 +190,7 @@ class UsersControllerTest extends \Test\TestCase { public function testSetUserSettings($email, $validEmail, $expectedStatus) { $controller = $this->getController(false, ['saveUserSettings']); $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('johndoe'); $this->userSession->method('getUser')->willReturn($user); @@ -208,41 +209,41 @@ class UsersControllerTest extends \Test\TestCase { IAccountManager::PROPERTY_DISPLAYNAME => [ 'value' => 'Display name', - 'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY, + 'scope' => AccountManager::SCOPE_FEDERATED, 'verified' => AccountManager::NOT_VERIFIED, ], IAccountManager::PROPERTY_ADDRESS => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, 'verified' => AccountManager::NOT_VERIFIED, ], IAccountManager::PROPERTY_WEBSITE => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, 'verified' => AccountManager::NOT_VERIFIED, ], IAccountManager::PROPERTY_EMAIL => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY, + 'scope' => AccountManager::SCOPE_FEDERATED, 'verified' => AccountManager::NOT_VERIFIED, ], IAccountManager::PROPERTY_AVATAR => [ - 'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY + 'scope' => AccountManager::SCOPE_FEDERATED ], IAccountManager::PROPERTY_PHONE => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, 'verified' => AccountManager::NOT_VERIFIED, ], IAccountManager::PROPERTY_TWITTER => [ 'value' => '', - 'scope' => AccountManager::VISIBILITY_PRIVATE, + 'scope' => AccountManager::SCOPE_LOCAL, 'verified' => AccountManager::NOT_VERIFIED, ], ]); @@ -255,19 +256,19 @@ class UsersControllerTest extends \Test\TestCase { } $result = $controller->setUserSettings(// - AccountManager::VISIBILITY_CONTACTS_ONLY, + AccountManager::SCOPE_FEDERATED, 'displayName', - AccountManager::VISIBILITY_CONTACTS_ONLY, + AccountManager::SCOPE_FEDERATED, '47658468', - AccountManager::VISIBILITY_CONTACTS_ONLY, + AccountManager::SCOPE_FEDERATED, $email, - AccountManager::VISIBILITY_CONTACTS_ONLY, + AccountManager::SCOPE_FEDERATED, 'nextcloud.com', - AccountManager::VISIBILITY_CONTACTS_ONLY, + AccountManager::SCOPE_FEDERATED, 'street and city', - AccountManager::VISIBILITY_CONTACTS_ONLY, + AccountManager::SCOPE_FEDERATED, '@nextclouders', - AccountManager::VISIBILITY_CONTACTS_ONLY + AccountManager::SCOPE_FEDERATED ); $this->assertSame($expectedStatus, $result->getStatus()); diff --git a/build/integration/features/bootstrap/Provisioning.php b/build/integration/features/bootstrap/Provisioning.php index 0ec19f27c6..cbe11403ba 100644 --- a/build/integration/features/bootstrap/Provisioning.php +++ b/build/integration/features/bootstrap/Provisioning.php @@ -157,7 +157,11 @@ trait Provisioning { $fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/cloud/users/$user"; $client = new Client(); $options = []; - $options['auth'] = $this->adminUser; + if ($this->currentUser === 'admin') { + $options['auth'] = $this->adminUser; + } else { + $options['auth'] = [$this->currentUser, $this->regularUser]; + } $options['headers'] = [ 'OCS-APIREQUEST' => 'true', ]; diff --git a/build/integration/features/provisioning-v1.feature b/build/integration/features/provisioning-v1.feature index 717aa04e4b..03aaad4b85 100644 --- a/build/integration/features/provisioning-v1.feature +++ b/build/integration/features/provisioning-v1.feature @@ -103,6 +103,82 @@ Feature: provisioning | website | https://nextcloud.com | | twitter | Nextcloud | + Scenario: Edit a user account properties scopes + Given user "brand-new-user" exists + And As an "brand-new-user" + When sending "PUT" to "/cloud/users/brand-new-user" with + | key | phoneScope | + | value | v2-private | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + When sending "PUT" to "/cloud/users/brand-new-user" with + | key | twitterScope | + | value | v2-local | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + When sending "PUT" to "/cloud/users/brand-new-user" with + | key | addressScope | + | value | v2-federated | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + When sending "PUT" to "/cloud/users/brand-new-user" with + | key | emailScope | + | value | v2-published | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + When sending "PUT" to "/cloud/users/brand-new-user" with + | key | websiteScope | + | value | public | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + When sending "PUT" to "/cloud/users/brand-new-user" with + | key | displaynameScope | + | value | contacts | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + When sending "PUT" to "/cloud/users/brand-new-user" with + | key | avatarScope | + | value | private | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + Then user "brand-new-user" has + | id | brand-new-user | + | phoneScope | v2-private | + | twitterScope | v2-local | + | addressScope | v2-federated | + | emailScope | v2-published | + | websiteScope | v2-published | + | displaynameScope | v2-federated | + | avatarScope | v2-local | + + Scenario: Edit a user account properties scopes with invalid or unsupported value + Given user "brand-new-user" exists + And As an "brand-new-user" + When sending "PUT" to "/cloud/users/brand-new-user" with + | key | phoneScope | + | value | invalid | + Then the OCS status code should be "102" + And the HTTP status code should be "200" + When sending "PUT" to "/cloud/users/brand-new-user" with + | key | displaynameScope | + | value | v2-private | + Then the OCS status code should be "102" + And the HTTP status code should be "200" + When sending "PUT" to "/cloud/users/brand-new-user" with + | key | emailScope | + | value | v2-private | + Then the OCS status code should be "102" + And the HTTP status code should be "200" + + Scenario: An admin cannot edit user account property scopes + Given As an "admin" + And user "brand-new-user" exists + When sending "PUT" to "/cloud/users/brand-new-user" with + | key | phoneScope | + | value | v2-private | + Then the OCS status code should be "997" + And the HTTP status code should be "401" + Scenario: Search by phone number Given As an "admin" And user "phone-user" exists @@ -612,4 +688,3 @@ Feature: provisioning And As an "user0" When sending "GET" with exact url to "/index.php/apps/files" And the HTTP status code should be "403" - diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index de3c0e83cc..65a317495b 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3155,6 +3155,14 @@ InMemoryFile + + + ISimpleFile + + + $data + + ISimpleFile diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 3c538a2a59..da9e5ffa6b 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -738,6 +738,7 @@ return array( 'OC\\Avatar\\Avatar' => $baseDir . '/lib/private/Avatar/Avatar.php', 'OC\\Avatar\\AvatarManager' => $baseDir . '/lib/private/Avatar/AvatarManager.php', 'OC\\Avatar\\GuestAvatar' => $baseDir . '/lib/private/Avatar/GuestAvatar.php', + 'OC\\Avatar\\PlaceholderAvatar' => $baseDir . '/lib/private/Avatar/PlaceholderAvatar.php', 'OC\\Avatar\\UserAvatar' => $baseDir . '/lib/private/Avatar/UserAvatar.php', 'OC\\BackgroundJob\\Job' => $baseDir . '/lib/private/BackgroundJob/Job.php', 'OC\\BackgroundJob\\JobList' => $baseDir . '/lib/private/BackgroundJob/JobList.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 0b1967e91d..c0e023b9f8 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -767,6 +767,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Avatar\\Avatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/Avatar.php', 'OC\\Avatar\\AvatarManager' => __DIR__ . '/../../..' . '/lib/private/Avatar/AvatarManager.php', 'OC\\Avatar\\GuestAvatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/GuestAvatar.php', + 'OC\\Avatar\\PlaceholderAvatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/PlaceholderAvatar.php', 'OC\\Avatar\\UserAvatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/UserAvatar.php', 'OC\\BackgroundJob\\Job' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/Job.php', 'OC\\BackgroundJob\\JobList' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/JobList.php', diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index c5a0f21319..6198f8dbdd 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -144,6 +144,42 @@ class AccountManager implements IAccountManager { } } + $allowedScopes = [ + self::SCOPE_PRIVATE, + self::SCOPE_LOCAL, + self::SCOPE_FEDERATED, + self::SCOPE_PUBLISHED, + self::VISIBILITY_PRIVATE, + self::VISIBILITY_CONTACTS_ONLY, + self::VISIBILITY_PUBLIC, + ]; + + // validate and convert scope values + foreach ($data as $propertyName => $propertyData) { + if (isset($propertyData['scope'])) { + if ($throwOnData && !in_array($propertyData['scope'], $allowedScopes, true)) { + throw new \InvalidArgumentException('scope'); + } + + if ( + $propertyData['scope'] === self::SCOPE_PRIVATE + && ($propertyName === self::PROPERTY_DISPLAYNAME || $propertyName === self::PROPERTY_EMAIL) + ) { + if ($throwOnData) { + // v2-private is not available for these fields + throw new \InvalidArgumentException('scope'); + } else { + // default to local + $data[$propertyName]['scope'] = self::SCOPE_LOCAL; + } + } else { + // migrate scope values to the new format + // invalid scopes are mapped to a default value + $data[$propertyName]['scope'] = AccountProperty::mapScopeToV2($propertyData['scope']); + } + } + } + if (empty($userData)) { $this->insertNewUser($user, $data); } elseif ($userData !== $data) { @@ -198,6 +234,8 @@ class AccountManager implements IAccountManager { * * @param IUser $user * @return array + * + * @deprecated use getAccount instead to make sure migrated properties work correctly */ public function getUser(IUser $user) { $uid = $user->getUID(); @@ -405,7 +443,7 @@ class AccountManager implements IAccountManager { } $query->setParameter('name', $propertyName) - ->setParameter('value', $property['value']); + ->setParameter('value', $property['value'] ?? ''); $query->execute(); } } @@ -421,41 +459,41 @@ class AccountManager implements IAccountManager { self::PROPERTY_DISPLAYNAME => [ 'value' => $user->getDisplayName(), - 'scope' => self::VISIBILITY_CONTACTS_ONLY, + 'scope' => self::SCOPE_FEDERATED, 'verified' => self::NOT_VERIFIED, ], self::PROPERTY_ADDRESS => [ 'value' => '', - 'scope' => self::VISIBILITY_PRIVATE, + 'scope' => self::SCOPE_LOCAL, 'verified' => self::NOT_VERIFIED, ], self::PROPERTY_WEBSITE => [ 'value' => '', - 'scope' => self::VISIBILITY_PRIVATE, + 'scope' => self::SCOPE_LOCAL, 'verified' => self::NOT_VERIFIED, ], self::PROPERTY_EMAIL => [ 'value' => $user->getEMailAddress(), - 'scope' => self::VISIBILITY_CONTACTS_ONLY, + 'scope' => self::SCOPE_FEDERATED, 'verified' => self::NOT_VERIFIED, ], self::PROPERTY_AVATAR => [ - 'scope' => self::VISIBILITY_CONTACTS_ONLY + 'scope' => self::SCOPE_FEDERATED ], self::PROPERTY_PHONE => [ 'value' => '', - 'scope' => self::VISIBILITY_PRIVATE, + 'scope' => self::SCOPE_LOCAL, 'verified' => self::NOT_VERIFIED, ], self::PROPERTY_TWITTER => [ 'value' => '', - 'scope' => self::VISIBILITY_PRIVATE, + 'scope' => self::SCOPE_LOCAL, 'verified' => self::NOT_VERIFIED, ], ]; @@ -464,7 +502,7 @@ class AccountManager implements IAccountManager { private function parseAccountData(IUser $user, $data): Account { $account = new Account($user); foreach ($data as $property => $accountData) { - $account->setProperty($property, $accountData['value'] ?? '', $accountData['scope'] ?? self::VISIBILITY_PRIVATE, $accountData['verified'] ?? self::NOT_VERIFIED); + $account->setProperty($property, $accountData['value'] ?? '', $accountData['scope'] ?? self::SCOPE_LOCAL, $accountData['verified'] ?? self::NOT_VERIFIED); } return $account; } diff --git a/lib/private/Accounts/AccountProperty.php b/lib/private/Accounts/AccountProperty.php index 97f9b1c356..850f39df9e 100644 --- a/lib/private/Accounts/AccountProperty.php +++ b/lib/private/Accounts/AccountProperty.php @@ -26,6 +26,7 @@ declare(strict_types=1); namespace OC\Accounts; +use OCP\Accounts\IAccountManager; use OCP\Accounts\IAccountProperty; class AccountProperty implements IAccountProperty { @@ -42,7 +43,7 @@ class AccountProperty implements IAccountProperty { public function __construct(string $name, string $value, string $scope, string $verified) { $this->name = $name; $this->value = $value; - $this->scope = $scope; + $this->scope = $this->mapScopeToV2($scope); $this->verified = $verified; } @@ -77,7 +78,7 @@ class AccountProperty implements IAccountProperty { * @return IAccountProperty */ public function setScope(string $scope): IAccountProperty { - $this->scope = $scope; + $this->scope = $this->mapScopeToV2($scope); return $this; } @@ -127,6 +128,23 @@ class AccountProperty implements IAccountProperty { return $this->scope; } + public static function mapScopeToV2($scope) { + if (strpos($scope, 'v2-') === 0) { + return $scope; + } + + switch ($scope) { + case IAccountManager::VISIBILITY_PRIVATE: + return IAccountManager::SCOPE_LOCAL; + case IAccountManager::VISIBILITY_CONTACTS_ONLY: + return IAccountManager::SCOPE_FEDERATED; + case IAccountManager::VISIBILITY_PUBLIC: + return IAccountManager::SCOPE_PUBLISHED; + } + + return IAccountManager::SCOPE_LOCAL; + } + /** * Get the verification status of a property * diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 5102396224..04d3a72102 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -34,8 +34,10 @@ declare(strict_types=1); namespace OC\Avatar; +use OC\KnownUser\KnownUserService; use OC\User\Manager; use OC\User\NoUserException; +use OCP\Accounts\IAccountManager; use OCP\Files\IAppData; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; @@ -44,12 +46,16 @@ use OCP\IAvatarManager; use OCP\IConfig; use OCP\IL10N; use OCP\ILogger; +use OCP\IUserSession; /** * This class implements methods to access Avatar functionality */ class AvatarManager implements IAvatarManager { + /** @var IUserSession */ + private $userSession; + /** @var Manager */ private $userManager; @@ -65,6 +71,12 @@ class AvatarManager implements IAvatarManager { /** @var IConfig */ private $config; + /** @var IAccountManager */ + private $accountManager; + + /** @var KnownUserService */ + private $knownUserService; + /** * AvatarManager constructor. * @@ -73,18 +85,26 @@ class AvatarManager implements IAvatarManager { * @param IL10N $l * @param ILogger $logger * @param IConfig $config + * @param IUserSession $userSession */ public function __construct( + IUserSession $userSession, Manager $userManager, IAppData $appData, IL10N $l, ILogger $logger, - IConfig $config) { + IConfig $config, + IAccountManager $accountManager, + KnownUserService $knownUserService + ) { + $this->userSession = $userSession; $this->userManager = $userManager; $this->appData = $appData; $this->l = $l; $this->logger = $logger; $this->config = $config; + $this->accountManager = $accountManager; + $this->knownUserService = $knownUserService; } /** @@ -104,12 +124,34 @@ class AvatarManager implements IAvatarManager { // sanitize userID - fixes casing issue (needed for the filesystem stuff that is done below) $userId = $user->getUID(); + $requestingUser = null; + if ($this->userSession !== null) { + $requestingUser = $this->userSession->getUser(); + } + try { $folder = $this->appData->getFolder($userId); } catch (NotFoundException $e) { $folder = $this->appData->newFolder($userId); } + $account = $this->accountManager->getAccount($user); + $avatarProperties = $account->getProperty(IAccountManager::PROPERTY_AVATAR); + $avatarScope = $avatarProperties->getScope(); + + if ( + // v2-private scope hides the avatar from public access and from unknown users + $avatarScope === IAccountManager::SCOPE_PRIVATE + && ( + // accessing from public link + $requestingUser === null + // logged in, but unknown to user + || !$this->knownUserService->isKnownToUser($requestingUser->getUID(), $userId) + )) { + // use a placeholder avatar which caches the generated images + return new PlaceholderAvatar($folder, $user, $this->logger); + } + return new UserAvatar($folder, $this->l, $user, $this->logger, $this->config); } diff --git a/lib/private/Avatar/PlaceholderAvatar.php b/lib/private/Avatar/PlaceholderAvatar.php new file mode 100644 index 0000000000..5883fe531a --- /dev/null +++ b/lib/private/Avatar/PlaceholderAvatar.php @@ -0,0 +1,183 @@ + + * + * @author Arthur Schiwon + * @author Christoph Wurst + * @author Joas Schilling + * @author Michael Weimann + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace OC\Avatar; + +use OC\NotSquareException; +use OC\User\User; +use OCP\Files\NotFoundException; +use OCP\Files\NotPermittedException; +use OCP\Files\SimpleFS\ISimpleFile; +use OCP\Files\SimpleFS\ISimpleFolder; +use OCP\IConfig; +use OCP\IImage; +use OCP\IL10N; +use OCP\ILogger; + +/** + * This class represents a registered user's placeholder avatar. + * + * It generates an image based on the user's initials and caches it on storage + * for faster retrieval, unlike the GuestAvatar. + */ +class PlaceholderAvatar extends Avatar { + /** @var ISimpleFolder */ + private $folder; + + /** @var User */ + private $user; + + /** + * UserAvatar constructor. + * + * @param IConfig $config The configuration + * @param ISimpleFolder $folder The avatar files folder + * @param IL10N $l The localization helper + * @param User $user The user this class manages the avatar for + * @param ILogger $logger The logger + */ + public function __construct( + ISimpleFolder $folder, + $user, + ILogger $logger) { + parent::__construct($logger); + + $this->folder = $folder; + $this->user = $user; + } + + /** + * Check if an avatar exists for the user + * + * @return bool + */ + public function exists() { + return true; + } + + /** + * Sets the users avatar. + * + * @param IImage|resource|string $data An image object, imagedata or path to set a new avatar + * @throws \Exception if the provided file is not a jpg or png image + * @throws \Exception if the provided image is not valid + * @throws NotSquareException if the image is not square + * @return void + */ + public function set($data) { + // unimplemented for placeholder avatars + } + + /** + * Removes the users avatar. + */ + public function remove(bool $silent = false) { + $avatars = $this->folder->getDirectoryListing(); + + foreach ($avatars as $avatar) { + $avatar->delete(); + } + } + + /** + * Returns the avatar for an user. + * + * If there is no avatar file yet, one is generated. + * + * @param int $size + * @return ISimpleFile + * @throws NotFoundException + * @throws \OCP\Files\NotPermittedException + * @throws \OCP\PreConditionNotMetException + */ + public function getFile($size) { + $size = (int) $size; + + $ext = 'png'; + + if ($size === -1) { + $path = 'avatar-placeholder.' . $ext; + } else { + $path = 'avatar-placeholder.' . $size . '.' . $ext; + } + + try { + $file = $this->folder->getFile($path); + } catch (NotFoundException $e) { + if ($size <= 0) { + throw new NotFoundException; + } + + if (!$data = $this->generateAvatarFromSvg($size)) { + $data = $this->generateAvatar($this->getDisplayName(), $size); + } + + try { + $file = $this->folder->newFile($path); + $file->putContent($data); + } catch (NotPermittedException $e) { + $this->logger->error('Failed to save avatar placeholder for ' . $this->user->getUID()); + throw new NotFoundException(); + } + } + + return $file; + } + + /** + * Returns the user display name. + * + * @return string + */ + public function getDisplayName(): string { + return $this->user->getDisplayName(); + } + + /** + * Handles user changes. + * + * @param string $feature The changed feature + * @param mixed $oldValue The previous value + * @param mixed $newValue The new value + * @throws NotPermittedException + * @throws \OCP\PreConditionNotMetException + */ + public function userChanged($feature, $oldValue, $newValue) { + $this->remove(); + } + + /** + * Check if the avatar of a user is a custom uploaded one + * + * @return bool + */ + public function isCustomAvatar(): bool { + return false; + } +} diff --git a/lib/private/Avatar/UserAvatar.php b/lib/private/Avatar/UserAvatar.php index f7ace429f7..f47809425e 100644 --- a/lib/private/Avatar/UserAvatar.php +++ b/lib/private/Avatar/UserAvatar.php @@ -270,6 +270,7 @@ class UserAvatar extends Avatar { throw new NotFoundException; } + // TODO: rework to integrate with the PlaceholderAvatar in a compatible way if ($this->folder->fileExists('generated')) { if (!$data = $this->generateAvatarFromSvg($size)) { $data = $this->generateAvatar($this->getDisplayName(), $size); diff --git a/lib/private/KnownUser/KnownUserService.php b/lib/private/KnownUser/KnownUserService.php index 96af21c836..1f300a9f8e 100644 --- a/lib/private/KnownUser/KnownUserService.php +++ b/lib/private/KnownUser/KnownUserService.php @@ -74,6 +74,10 @@ class KnownUserService { * @return bool */ public function isKnownToUser(string $knownTo, string $contactUserId): bool { + if ($knownTo === $contactUserId) { + return true; + } + if (!isset($this->knownUsers[$knownTo])) { $entities = $this->mapper->getKnownUsers($knownTo); $this->knownUsers[$knownTo] = []; diff --git a/lib/private/Server.php b/lib/private/Server.php index c0d6afbaaf..26c76125e5 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -104,6 +104,7 @@ use OC\IntegrityCheck\Checker; use OC\IntegrityCheck\Helpers\AppLocator; use OC\IntegrityCheck\Helpers\EnvironmentHelper; use OC\IntegrityCheck\Helpers\FileAccessHelper; +use OC\KnownUser\KnownUserService; use OC\Lock\DBLockingProvider; use OC\Lock\MemcacheLockingProvider; use OC\Lock\NoopLockingProvider; @@ -720,11 +721,14 @@ class Server extends ServerContainer implements IServerContainer { $this->registerService(AvatarManager::class, function (Server $c) { return new AvatarManager( + $c->get(IUserSession::class), $c->get(\OC\User\Manager::class), $c->getAppDataDir('avatar'), $c->getL10N('lib'), $c->get(ILogger::class), - $c->get(\OCP\IConfig::class) + $c->get(\OCP\IConfig::class), + $c->get(IAccountManager::class), + $c->get(KnownUserService::class) ); }); $this->registerAlias(IAvatarManager::class, AvatarManager::class); diff --git a/lib/public/Accounts/IAccountManager.php b/lib/public/Accounts/IAccountManager.php index ae70d8963b..e88fd32f67 100644 --- a/lib/public/Accounts/IAccountManager.php +++ b/lib/public/Accounts/IAccountManager.php @@ -38,11 +38,54 @@ use OCP\IUser; */ interface IAccountManager { - /** nobody can see my account details */ + /** + * Contact details visible locally only + * + * @since 21.0.1 + */ + public const SCOPE_PRIVATE = 'v2-private'; + + /** + * Contact details visible locally and through public link access on local instance + * + * @since 21.0.1 + */ + public const SCOPE_LOCAL = 'v2-local'; + + /** + * Contact details visible locally, through public link access and on trusted federated servers. + * + * @since 21.0.1 + */ + public const SCOPE_FEDERATED = 'v2-federated'; + + /** + * Contact details visible locally, through public link access, on trusted federated servers + * and published to the public lookup server. + * + * @since 21.0.1 + */ + public const SCOPE_PUBLISHED = 'v2-published'; + + /** + * Contact details only visible locally + * + * @deprecated 21.0.1 + */ public const VISIBILITY_PRIVATE = 'private'; - /** only contacts, especially trusted servers can see my contact details */ + + /** + * Contact details visible on trusted federated servers. + * + * @deprecated 21.0.1 + */ public const VISIBILITY_CONTACTS_ONLY = 'contacts'; - /** every body ca see my contact detail, will be published to the lookup server */ + + /** + * Contact details visible on trusted federated servers and in the public lookup server. + * + * @deprecated 21.0.1 + */ public const VISIBILITY_PUBLIC = 'public'; public const PROPERTY_AVATAR = 'avatar'; diff --git a/tests/lib/Accounts/AccountManagerTest.php b/tests/lib/Accounts/AccountManagerTest.php index fcd1a78add..27ebed6979 100644 --- a/tests/lib/Accounts/AccountManagerTest.php +++ b/tests/lib/Accounts/AccountManagerTest.php @@ -153,6 +153,129 @@ class AccountManagerTest extends TestCase { ]; } + public function updateUserSetScopeProvider() { + return [ + // regular scope switching + [ + [ + IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_PUBLISHED], + IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_PUBLISHED], + IAccountManager::PROPERTY_AVATAR => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED], + IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED], + IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_FEDERATED], + IAccountManager::PROPERTY_ADDRESS => ['value' => 'some street', 'scope' => IAccountManager::SCOPE_LOCAL], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_PRIVATE], + ], + [ + IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_LOCAL], + IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_FEDERATED], + IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PRIVATE], + IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_LOCAL], + IAccountManager::PROPERTY_ADDRESS => ['value' => 'some street', 'scope' => IAccountManager::SCOPE_FEDERATED], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_PUBLISHED], + ], + [ + IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_LOCAL], + IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_FEDERATED], + IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PRIVATE], + IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_LOCAL], + IAccountManager::PROPERTY_ADDRESS => ['value' => 'some street', 'scope' => IAccountManager::SCOPE_FEDERATED], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_PUBLISHED], + ], + ], + // legacy scope mapping, the given visibility values get converted to scopes + [ + [ + IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED], + IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_FEDERATED], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_PRIVATE], + ], + [ + IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::VISIBILITY_PUBLIC], + IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::VISIBILITY_CONTACTS_ONLY], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::VISIBILITY_PRIVATE], + ], + [ + IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED], + IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_FEDERATED], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => IAccountManager::SCOPE_LOCAL], + ], + ], + // invalid or unsupported scope values get converted to SCOPE_LOCAL + [ + [ + IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_PUBLISHED], + IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_PUBLISHED], + IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED], + IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_FEDERATED], + ], + [ + // SCOPE_PRIVATE is not allowed for display name and email + IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_PRIVATE], + IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_PRIVATE], + IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => 'invalid'], + IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => ''], + ], + [ + IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_LOCAL], + IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_LOCAL], + IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_LOCAL], + IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_LOCAL], + ], + // don't throw but fall back + false, false, + ], + // invalid or unsupported scope values throw an exception when passing $throwOnData=true + [ + [IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_PUBLISHED]], + [IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_PRIVATE]], + null, + // throw exception + true, true, + ], + [ + [IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_PUBLISHED]], + [IAccountManager::PROPERTY_EMAIL => ['value' => 'test@example.org', 'scope' => IAccountManager::SCOPE_PRIVATE]], + null, + // throw exception + true, true, + ], + [ + [IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_PUBLISHED]], + [IAccountManager::PROPERTY_TWITTER => ['value' => '@sometwitter', 'scope' => 'invalid']], + null, + // throw exception + true, true, + ], + ]; + } + + /** + * @dataProvider updateUserSetScopeProvider + */ + public function testUpdateUserSetScope($oldData, $newData, $savedData, $throwOnData = true, $expectedThrow = false) { + $accountManager = $this->getInstance(['getUser', 'insertNewUser', 'updateExistingUser', 'updateVerifyStatus', 'checkEmailVerification']); + /** @var IUser $user */ + $user = $this->createMock(IUser::class); + + $accountManager->expects($this->once())->method('getUser')->with($user)->willReturn($oldData); + + if ($expectedThrow) { + $accountManager->expects($this->never())->method('updateExistingUser'); + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('scope'); + } else { + $accountManager->expects($this->once())->method('checkEmailVerification') + ->with($oldData, $savedData, $user)->willReturn($savedData); + $accountManager->expects($this->once())->method('updateVerifyStatus') + ->with($oldData, $savedData)->willReturn($savedData); + $accountManager->expects($this->once())->method('updateExistingUser') + ->with($user, $savedData); + $accountManager->expects($this->never())->method('insertNewUser'); + } + + $accountManager->updateUser($user, $newData, $throwOnData); + } /** * @dataProvider dataTestGetUser @@ -278,26 +401,26 @@ class AccountManagerTest extends TestCase { IAccountManager::PROPERTY_TWITTER => [ 'value' => '@twitterhandle', - 'scope' => IAccountManager::VISIBILITY_PRIVATE, + 'scope' => IAccountManager::SCOPE_LOCAL, 'verified' => IAccountManager::NOT_VERIFIED, ], IAccountManager::PROPERTY_EMAIL => [ 'value' => 'test@example.com', - 'scope' => IAccountManager::VISIBILITY_PUBLIC, + 'scope' => IAccountManager::SCOPE_PUBLISHED, 'verified' => IAccountManager::VERIFICATION_IN_PROGRESS, ], IAccountManager::PROPERTY_WEBSITE => [ 'value' => 'https://example.com', - 'scope' => IAccountManager::VISIBILITY_CONTACTS_ONLY, + 'scope' => IAccountManager::SCOPE_FEDERATED, 'verified' => IAccountManager::VERIFIED, ], ]; $expected = new Account($user); - $expected->setProperty(IAccountManager::PROPERTY_TWITTER, '@twitterhandle', IAccountManager::VISIBILITY_PRIVATE, IAccountManager::NOT_VERIFIED); - $expected->setProperty(IAccountManager::PROPERTY_EMAIL, 'test@example.com', IAccountManager::VISIBILITY_PUBLIC, IAccountManager::VERIFICATION_IN_PROGRESS); - $expected->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::VISIBILITY_CONTACTS_ONLY, IAccountManager::VERIFIED); + $expected->setProperty(IAccountManager::PROPERTY_TWITTER, '@twitterhandle', IAccountManager::SCOPE_LOCAL, IAccountManager::NOT_VERIFIED); + $expected->setProperty(IAccountManager::PROPERTY_EMAIL, 'test@example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFICATION_IN_PROGRESS); + $expected->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_FEDERATED, IAccountManager::VERIFIED); $accountManager->expects($this->once()) ->method('getUser') diff --git a/tests/lib/Accounts/AccountPropertyTest.php b/tests/lib/Accounts/AccountPropertyTest.php index afd807a44b..f99abc21f8 100644 --- a/tests/lib/Accounts/AccountPropertyTest.php +++ b/tests/lib/Accounts/AccountPropertyTest.php @@ -37,12 +37,12 @@ class AccountPropertyTest extends TestCase { $accountProperty = new AccountProperty( IAccountManager::PROPERTY_WEBSITE, 'https://example.com', - IAccountManager::VISIBILITY_PUBLIC, + IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED ); $this->assertEquals(IAccountManager::PROPERTY_WEBSITE, $accountProperty->getName()); $this->assertEquals('https://example.com', $accountProperty->getValue()); - $this->assertEquals(IAccountManager::VISIBILITY_PUBLIC, $accountProperty->getScope()); + $this->assertEquals(IAccountManager::SCOPE_PUBLISHED, $accountProperty->getScope()); $this->assertEquals(IAccountManager::VERIFIED, $accountProperty->getVerified()); } @@ -50,7 +50,7 @@ class AccountPropertyTest extends TestCase { $accountProperty = new AccountProperty( IAccountManager::PROPERTY_WEBSITE, 'https://example.com', - IAccountManager::VISIBILITY_PUBLIC, + IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED ); $actualReturn = $accountProperty->setValue('https://example.org'); @@ -62,19 +62,48 @@ class AccountPropertyTest extends TestCase { $accountProperty = new AccountProperty( IAccountManager::PROPERTY_WEBSITE, 'https://example.com', - IAccountManager::VISIBILITY_PUBLIC, + IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED ); - $actualReturn = $accountProperty->setScope(IAccountManager::VISIBILITY_PRIVATE); - $this->assertEquals(IAccountManager::VISIBILITY_PRIVATE, $accountProperty->getScope()); - $this->assertEquals(IAccountManager::VISIBILITY_PRIVATE, $actualReturn->getScope()); + $actualReturn = $accountProperty->setScope(IAccountManager::SCOPE_LOCAL); + $this->assertEquals(IAccountManager::SCOPE_LOCAL, $accountProperty->getScope()); + $this->assertEquals(IAccountManager::SCOPE_LOCAL, $actualReturn->getScope()); + } + + public function scopesProvider() { + return [ + // current values + [IAccountManager::SCOPE_PRIVATE, IAccountManager::SCOPE_PRIVATE], + [IAccountManager::SCOPE_LOCAL, IAccountManager::SCOPE_LOCAL], + [IAccountManager::SCOPE_PUBLISHED, IAccountManager::SCOPE_PUBLISHED], + // legacy values + [IAccountManager::VISIBILITY_PRIVATE, IAccountManager::SCOPE_LOCAL], + [IAccountManager::VISIBILITY_CONTACTS_ONLY, IAccountManager::SCOPE_FEDERATED], + [IAccountManager::VISIBILITY_PUBLIC, IAccountManager::SCOPE_PUBLISHED], + // fallback + ['', IAccountManager::SCOPE_LOCAL], + ['unknown', IAccountManager::SCOPE_LOCAL], + ]; + } + + /** + * @dataProvider scopesProvider + */ + public function testSetScopeMapping($storedScope, $returnedScope) { + $accountProperty = new AccountProperty( + IAccountManager::PROPERTY_WEBSITE, + 'https://example.com', + $storedScope, + IAccountManager::VERIFIED + ); + $this->assertEquals($returnedScope, $accountProperty->getScope()); } public function testSetVerified() { $accountProperty = new AccountProperty( IAccountManager::PROPERTY_WEBSITE, 'https://example.com', - IAccountManager::VISIBILITY_PUBLIC, + IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED ); $actualReturn = $accountProperty->setVerified(IAccountManager::NOT_VERIFIED); @@ -86,13 +115,13 @@ class AccountPropertyTest extends TestCase { $accountProperty = new AccountProperty( IAccountManager::PROPERTY_WEBSITE, 'https://example.com', - IAccountManager::VISIBILITY_PUBLIC, + IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED ); $this->assertEquals([ 'name' => IAccountManager::PROPERTY_WEBSITE, 'value' => 'https://example.com', - 'scope' => IAccountManager::VISIBILITY_PUBLIC, + 'scope' => IAccountManager::SCOPE_PUBLISHED, 'verified' => IAccountManager::VERIFIED ], $accountProperty->jsonSerialize()); } diff --git a/tests/lib/Accounts/AccountTest.php b/tests/lib/Accounts/AccountTest.php index 11b13637bd..8afcc44afd 100644 --- a/tests/lib/Accounts/AccountTest.php +++ b/tests/lib/Accounts/AccountTest.php @@ -43,21 +43,21 @@ class AccountTest extends TestCase { public function testSetProperty() { $user = $this->createMock(IUser::class); - $property = new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::VISIBILITY_PUBLIC, 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::VISIBILITY_PUBLIC, IAccountManager::NOT_VERIFIED); + $account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED); $this->assertEquals($property, $account->getProperty(IAccountManager::PROPERTY_WEBSITE)); } public function testGetProperties() { $user = $this->createMock(IUser::class); $properties = [ - IAccountManager::PROPERTY_WEBSITE => new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::VISIBILITY_PUBLIC, IAccountManager::NOT_VERIFIED), - IAccountManager::PROPERTY_EMAIL => new AccountProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::VISIBILITY_PRIVATE, 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::VISIBILITY_PUBLIC, IAccountManager::NOT_VERIFIED); - $account->setProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::VISIBILITY_PRIVATE, IAccountManager::VERIFIED); + $account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED); + $account->setProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::VERIFIED); $this->assertEquals($properties, $account->getProperties()); } @@ -65,14 +65,14 @@ 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::VISIBILITY_PUBLIC, IAccountManager::NOT_VERIFIED), - IAccountManager::PROPERTY_EMAIL => new AccountProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::VISIBILITY_PRIVATE, IAccountManager::VERIFIED), - IAccountManager::PROPERTY_PHONE => new AccountProperty(IAccountManager::PROPERTY_PHONE, '123456', IAccountManager::VISIBILITY_PUBLIC, 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::VISIBILITY_PUBLIC, IAccountManager::NOT_VERIFIED); - $account->setProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::VISIBILITY_PRIVATE, IAccountManager::VERIFIED); - $account->setProperty(IAccountManager::PROPERTY_PHONE, '123456', IAccountManager::VISIBILITY_PUBLIC, IAccountManager::VERIFIED); + $account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED); + $account->setProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::VERIFIED); + $account->setProperty(IAccountManager::PROPERTY_PHONE, '123456', IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED); $this->assertEquals( @@ -80,7 +80,7 @@ class AccountTest extends TestCase { IAccountManager::PROPERTY_WEBSITE => $properties[IAccountManager::PROPERTY_WEBSITE], IAccountManager::PROPERTY_PHONE => $properties[IAccountManager::PROPERTY_PHONE], ], - $account->getFilteredProperties(IAccountManager::VISIBILITY_PUBLIC) + $account->getFilteredProperties(IAccountManager::SCOPE_PUBLISHED) ); $this->assertEquals( [ @@ -91,19 +91,19 @@ class AccountTest extends TestCase { ); $this->assertEquals( [IAccountManager::PROPERTY_PHONE => $properties[IAccountManager::PROPERTY_PHONE]], - $account->getFilteredProperties(IAccountManager::VISIBILITY_PUBLIC, IAccountManager::VERIFIED) + $account->getFilteredProperties(IAccountManager::SCOPE_PUBLISHED, IAccountManager::VERIFIED) ); } public function testJsonSerialize() { $user = $this->createMock(IUser::class); $properties = [ - IAccountManager::PROPERTY_WEBSITE => new AccountProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::VISIBILITY_PUBLIC, IAccountManager::NOT_VERIFIED), - IAccountManager::PROPERTY_EMAIL => new AccountProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::VISIBILITY_PRIVATE, 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::VISIBILITY_PUBLIC, IAccountManager::NOT_VERIFIED); - $account->setProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::VISIBILITY_PRIVATE, IAccountManager::VERIFIED); + $account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED); + $account->setProperty(IAccountManager::PROPERTY_EMAIL, 'user@example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::VERIFIED); $this->assertEquals($properties, $account->jsonSerialize()); } diff --git a/tests/lib/Avatar/AvatarManagerTest.php b/tests/lib/Avatar/AvatarManagerTest.php index 5a061cd10e..d3bc60efb5 100644 --- a/tests/lib/Avatar/AvatarManagerTest.php +++ b/tests/lib/Avatar/AvatarManagerTest.php @@ -25,19 +25,27 @@ namespace Test\Avatar; use OC\Avatar\AvatarManager; +use OC\Avatar\PlaceholderAvatar; use OC\Avatar\UserAvatar; +use OC\KnownUser\KnownUserService; use OC\User\Manager; +use OCP\Accounts\IAccount; +use OCP\Accounts\IAccountManager; +use OCP\Accounts\IAccountProperty; use OCP\Files\IAppData; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IConfig; use OCP\IL10N; use OCP\ILogger; use OCP\IUser; +use OCP\IUserSession; /** * Class AvatarManagerTest */ class AvatarManagerTest extends \Test\TestCase { + /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ + private $userSession; /** @var Manager|\PHPUnit\Framework\MockObject\MockObject */ private $userManager; /** @var IAppData|\PHPUnit\Framework\MockObject\MockObject */ @@ -48,28 +56,37 @@ class AvatarManagerTest extends \Test\TestCase { private $logger; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ private $config; + /** @var IAccountManager|\PHPUnit\Framework\MockObject\MockObject */ + private $accountManager; /** @var AvatarManager | \PHPUnit\Framework\MockObject\MockObject */ private $avatarManager; + /** @var KnownUserService | \PHPUnit\Framework\MockObject\MockObject */ + private $knownUserService; protected function setUp(): void { parent::setUp(); + $this->userSession = $this->createMock(IUserSession::class); $this->userManager = $this->createMock(Manager::class); $this->appData = $this->createMock(IAppData::class); $this->l10n = $this->createMock(IL10N::class); $this->logger = $this->createMock(ILogger::class); $this->config = $this->createMock(IConfig::class); + $this->accountManager = $this->createMock(IAccountManager::class); + $this->knownUserService = $this->createMock(KnownUserService::class); $this->avatarManager = new AvatarManager( + $this->userSession, $this->userManager, $this->appData, $this->l10n, $this->logger, - $this->config + $this->config, + $this->accountManager, + $this->knownUserService ); } - public function testGetAvatarInvalidUser() { $this->expectException(\Exception::class); $this->expectExceptionMessage('user does not exist'); @@ -83,17 +100,45 @@ class AvatarManagerTest extends \Test\TestCase { $this->avatarManager->getAvatar('invalidUser'); } - public function testGetAvatarValidUser() { + public function testGetAvatarForSelf() { $user = $this->createMock(IUser::class); $user - ->expects($this->once()) + ->expects($this->any()) ->method('getUID') ->willReturn('valid-user'); + + // requesting user + $this->userSession->expects($this->once()) + ->method('getUser') + ->willReturn($user); + $this->userManager ->expects($this->once()) ->method('get') ->with('valid-user') ->willReturn($user); + + $account = $this->createMock(IAccount::class); + $this->accountManager->expects($this->once()) + ->method('getAccount') + ->with($user) + ->willReturn($account); + + $property = $this->createMock(IAccountProperty::class); + $account->expects($this->once()) + ->method('getProperty') + ->with(IAccountManager::PROPERTY_AVATAR) + ->willReturn($property); + + $property->expects($this->once()) + ->method('getScope') + ->willReturn(IAccountManager::SCOPE_PRIVATE); + + $this->knownUserService->expects($this->any()) + ->method('isKnownToUser') + ->with('valid-user', 'valid-user') + ->willReturn(true); + $folder = $this->createMock(ISimpleFolder::class); $this->appData ->expects($this->once()) @@ -126,4 +171,86 @@ class AvatarManagerTest extends \Test\TestCase { $expected = new UserAvatar($folder, $this->l10n, $user, $this->logger, $this->config); $this->assertEquals($expected, $this->avatarManager->getAvatar('vaLid-USER')); } + + public function knownUnknownProvider() { + return [ + [IAccountManager::SCOPE_LOCAL, false, false, false], + [IAccountManager::SCOPE_LOCAL, true, false, false], + + // public access cannot see real avatar + [IAccountManager::SCOPE_PRIVATE, true, false, true], + // unknown users cannot see real avatar + [IAccountManager::SCOPE_PRIVATE, false, false, true], + // known users can see real avatar + [IAccountManager::SCOPE_PRIVATE, false, true, false], + ]; + } + + /** + * @dataProvider knownUnknownProvider + */ + public function testGetAvatarScopes($avatarScope, $isPublicCall, $isKnownUser, $expectedPlaceholder) { + if ($isPublicCall) { + $requestingUser = null; + } else { + $requestingUser = $this->createMock(IUser::class); + $requestingUser->method('getUID')->willReturn('requesting-user'); + } + + // requesting user + $this->userSession->expects($this->once()) + ->method('getUser') + ->willReturn($requestingUser); + + $user = $this->createMock(IUser::class); + $user + ->expects($this->once()) + ->method('getUID') + ->willReturn('valid-user'); + $this->userManager + ->expects($this->once()) + ->method('get') + ->with('valid-user') + ->willReturn($user); + + $account = $this->createMock(IAccount::class); + $this->accountManager->expects($this->once()) + ->method('getAccount') + ->with($user) + ->willReturn($account); + + $property = $this->createMock(IAccountProperty::class); + $account->expects($this->once()) + ->method('getProperty') + ->with(IAccountManager::PROPERTY_AVATAR) + ->willReturn($property); + + $property->expects($this->once()) + ->method('getScope') + ->willReturn($avatarScope); + + $folder = $this->createMock(ISimpleFolder::class); + $this->appData + ->expects($this->once()) + ->method('getFolder') + ->with('valid-user') + ->willReturn($folder); + + if (!$isPublicCall) { + $this->knownUserService->expects($this->any()) + ->method('isKnownToUser') + ->with('requesting-user', 'valid-user') + ->willReturn($isKnownUser); + } else { + $this->knownUserService->expects($this->never()) + ->method('isKnownToUser'); + } + + if ($expectedPlaceholder) { + $expected = new PlaceholderAvatar($folder, $user, $this->createMock(ILogger::class)); + } else { + $expected = new UserAvatar($folder, $this->l10n, $user, $this->logger, $this->config); + } + $this->assertEquals($expected, $this->avatarManager->getAvatar('valid-user')); + } }