Allow to tweak default scopes for accounts
Close #6582 Signed-off-by: Thomas Citharel <tcit@tcit.fr>
This commit is contained in:
parent
ab0548e4ed
commit
4d26a9afa0
|
@ -2168,4 +2168,18 @@ $CONFIG = [
|
|||
* the database storage.
|
||||
*/
|
||||
'enable_file_metadata' => true,
|
||||
|
||||
/**
|
||||
* Allows to override the default scopes for Account data.
|
||||
* The list of overridable properties and valid values for scopes are in
|
||||
* OCP\Accounts\IAccountManager. Values added here are merged with
|
||||
* default values, which are in OC\Accounts\AccountManager
|
||||
*
|
||||
* For instance, if the phone property should default to the private scope
|
||||
* instead of the local one:
|
||||
* [
|
||||
* \OCP\Accounts\IAccountManager::PROPERTY_PHONE => \OCP\Accounts\IAccountManager::SCOPE_PRIVATE
|
||||
* ]
|
||||
*/
|
||||
'account_manager.default_property_scope' => []
|
||||
];
|
||||
|
|
|
@ -14,6 +14,7 @@
|
|||
* @author Lukas Reschke <lukas@statuscode.ch>
|
||||
* @author Morris Jobke <hey@morrisjobke.de>
|
||||
* @author Roeland Jago Douma <roeland@famdouma.nl>
|
||||
* @author Thomas Citharel <nextcloud@tcit.fr>
|
||||
* @author Vincent Petry <vincent@nextcloud.com>
|
||||
*
|
||||
* @license AGPL-3.0
|
||||
|
@ -119,6 +120,23 @@ class AccountManager implements IAccountManager {
|
|||
private $l10nfactory;
|
||||
private CappedMemoryCache $internalCache;
|
||||
|
||||
/**
|
||||
* The list of default scopes for each property.
|
||||
*/
|
||||
public const DEFAULT_SCOPES = [
|
||||
self::PROPERTY_DISPLAYNAME => self::SCOPE_FEDERATED,
|
||||
self::PROPERTY_ADDRESS => self::SCOPE_LOCAL,
|
||||
self::PROPERTY_WEBSITE => self::SCOPE_LOCAL,
|
||||
self::PROPERTY_EMAIL => self::SCOPE_FEDERATED,
|
||||
self::PROPERTY_AVATAR => self::SCOPE_FEDERATED,
|
||||
self::PROPERTY_PHONE => self::SCOPE_LOCAL,
|
||||
self::PROPERTY_TWITTER => self::SCOPE_LOCAL,
|
||||
self::PROPERTY_ORGANISATION => self::SCOPE_LOCAL,
|
||||
self::PROPERTY_ROLE => self::SCOPE_LOCAL,
|
||||
self::PROPERTY_HEADLINE => self::SCOPE_LOCAL,
|
||||
self::PROPERTY_BIOGRAPHY => self::SCOPE_LOCAL,
|
||||
];
|
||||
|
||||
public function __construct(
|
||||
IDBConnection $connection,
|
||||
IConfig $config,
|
||||
|
@ -649,81 +667,84 @@ class AccountManager implements IAccountManager {
|
|||
|
||||
/**
|
||||
* build default user record in case not data set exists yet
|
||||
*
|
||||
* @param IUser $user
|
||||
* @return array
|
||||
*/
|
||||
protected function buildDefaultUserRecord(IUser $user) {
|
||||
protected function buildDefaultUserRecord(IUser $user): array {
|
||||
$scopes = array_merge(self::DEFAULT_SCOPES, array_filter($this->config->getSystemValue('account_manager.default_property_scope', []), static function (string $scope, string $property) {
|
||||
return in_array($property, self::ALLOWED_PROPERTIES, true) && in_array($scope, self::ALLOWED_SCOPES, true);
|
||||
}, ARRAY_FILTER_USE_BOTH));
|
||||
|
||||
return [
|
||||
[
|
||||
'name' => self::PROPERTY_DISPLAYNAME,
|
||||
'value' => $user->getDisplayName(),
|
||||
'scope' => self::SCOPE_FEDERATED,
|
||||
// Display name must be at least SCOPE_LOCAL
|
||||
'scope' => $scopes[self::PROPERTY_DISPLAYNAME] === self::SCOPE_PRIVATE ? self::SCOPE_LOCAL : $scopes[self::PROPERTY_DISPLAYNAME],
|
||||
'verified' => self::NOT_VERIFIED,
|
||||
],
|
||||
|
||||
[
|
||||
'name' => self::PROPERTY_ADDRESS,
|
||||
'value' => '',
|
||||
'scope' => self::SCOPE_LOCAL,
|
||||
'scope' => $scopes[self::PROPERTY_ADDRESS],
|
||||
'verified' => self::NOT_VERIFIED,
|
||||
],
|
||||
|
||||
[
|
||||
'name' => self::PROPERTY_WEBSITE,
|
||||
'value' => '',
|
||||
'scope' => self::SCOPE_LOCAL,
|
||||
'scope' => $scopes[self::PROPERTY_WEBSITE],
|
||||
'verified' => self::NOT_VERIFIED,
|
||||
],
|
||||
|
||||
[
|
||||
'name' => self::PROPERTY_EMAIL,
|
||||
'value' => $user->getEMailAddress(),
|
||||
'scope' => self::SCOPE_FEDERATED,
|
||||
// Email must be at least SCOPE_LOCAL
|
||||
'scope' => $scopes[self::PROPERTY_EMAIL] === self::SCOPE_PRIVATE ? self::SCOPE_LOCAL : $scopes[self::PROPERTY_EMAIL],
|
||||
'verified' => self::NOT_VERIFIED,
|
||||
],
|
||||
|
||||
[
|
||||
'name' => self::PROPERTY_AVATAR,
|
||||
'scope' => self::SCOPE_FEDERATED
|
||||
'scope' => $scopes[self::PROPERTY_AVATAR],
|
||||
],
|
||||
|
||||
[
|
||||
'name' => self::PROPERTY_PHONE,
|
||||
'value' => '',
|
||||
'scope' => self::SCOPE_LOCAL,
|
||||
'scope' => $scopes[self::PROPERTY_PHONE],
|
||||
'verified' => self::NOT_VERIFIED,
|
||||
],
|
||||
|
||||
[
|
||||
'name' => self::PROPERTY_TWITTER,
|
||||
'value' => '',
|
||||
'scope' => self::SCOPE_LOCAL,
|
||||
'scope' => $scopes[self::PROPERTY_TWITTER],
|
||||
'verified' => self::NOT_VERIFIED,
|
||||
],
|
||||
|
||||
[
|
||||
'name' => self::PROPERTY_ORGANISATION,
|
||||
'value' => '',
|
||||
'scope' => self::SCOPE_LOCAL,
|
||||
'scope' => $scopes[self::PROPERTY_ORGANISATION],
|
||||
],
|
||||
|
||||
[
|
||||
'name' => self::PROPERTY_ROLE,
|
||||
'value' => '',
|
||||
'scope' => self::SCOPE_LOCAL,
|
||||
'scope' => $scopes[self::PROPERTY_ROLE],
|
||||
],
|
||||
|
||||
[
|
||||
'name' => self::PROPERTY_HEADLINE,
|
||||
'value' => '',
|
||||
'scope' => self::SCOPE_LOCAL,
|
||||
'scope' => $scopes[self::PROPERTY_HEADLINE],
|
||||
],
|
||||
|
||||
[
|
||||
'name' => self::PROPERTY_BIOGRAPHY,
|
||||
'value' => '',
|
||||
'scope' => self::SCOPE_LOCAL,
|
||||
'scope' => $scopes[self::PROPERTY_BIOGRAPHY],
|
||||
],
|
||||
|
||||
[
|
||||
|
@ -790,17 +811,8 @@ class AccountManager implements IAccountManager {
|
|||
// valid case, nothing to do
|
||||
}
|
||||
|
||||
static $allowedScopes = [
|
||||
self::SCOPE_PRIVATE,
|
||||
self::SCOPE_LOCAL,
|
||||
self::SCOPE_FEDERATED,
|
||||
self::SCOPE_PUBLISHED,
|
||||
self::VISIBILITY_PRIVATE,
|
||||
self::VISIBILITY_CONTACTS_ONLY,
|
||||
self::VISIBILITY_PUBLIC,
|
||||
];
|
||||
foreach ($account->getAllProperties() as $property) {
|
||||
$this->testPropertyScope($property, $allowedScopes, true);
|
||||
$this->testPropertyScope($property, self::ALLOWED_SCOPES, true);
|
||||
}
|
||||
|
||||
$oldData = $this->getUser($account->getUser(), false);
|
||||
|
|
|
@ -8,6 +8,7 @@ declare(strict_types=1);
|
|||
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
|
||||
* @author Joas Schilling <coding@schilljs.com>
|
||||
* @author Julius Härtl <jus@bitgrid.net>
|
||||
* @author Thomas Citharel <nextcloud@tcit.fr>
|
||||
* @author Vincent Petry <vincent@nextcloud.com>
|
||||
*
|
||||
* @license GNU AGPL version 3 or any later version
|
||||
|
@ -89,6 +90,21 @@ interface IAccountManager {
|
|||
*/
|
||||
public const VISIBILITY_PUBLIC = 'public';
|
||||
|
||||
/**
|
||||
* The list of allowed scopes
|
||||
*
|
||||
* @since 25.0.0
|
||||
*/
|
||||
public const ALLOWED_SCOPES = [
|
||||
self::SCOPE_PRIVATE,
|
||||
self::SCOPE_LOCAL,
|
||||
self::SCOPE_FEDERATED,
|
||||
self::SCOPE_PUBLISHED,
|
||||
self::VISIBILITY_PRIVATE,
|
||||
self::VISIBILITY_CONTACTS_ONLY,
|
||||
self::VISIBILITY_PUBLIC,
|
||||
];
|
||||
|
||||
public const PROPERTY_AVATAR = 'avatar';
|
||||
public const PROPERTY_DISPLAYNAME = 'displayname';
|
||||
public const PROPERTY_PHONE = 'phone';
|
||||
|
@ -122,6 +138,26 @@ interface IAccountManager {
|
|||
*/
|
||||
public const PROPERTY_PROFILE_ENABLED = 'profile_enabled';
|
||||
|
||||
/**
|
||||
* The list of allowed properties
|
||||
*
|
||||
* @since 25.0.0
|
||||
*/
|
||||
public const ALLOWED_PROPERTIES = [
|
||||
self::PROPERTY_AVATAR,
|
||||
self::PROPERTY_DISPLAYNAME,
|
||||
self::PROPERTY_PHONE,
|
||||
self::PROPERTY_EMAIL,
|
||||
self::PROPERTY_WEBSITE,
|
||||
self::PROPERTY_ADDRESS,
|
||||
self::PROPERTY_TWITTER,
|
||||
self::PROPERTY_ORGANISATION,
|
||||
self::PROPERTY_ROLE,
|
||||
self::PROPERTY_HEADLINE,
|
||||
self::PROPERTY_BIOGRAPHY,
|
||||
self::PROPERTY_PROFILE_ENABLED,
|
||||
];
|
||||
|
||||
public const COLLECTION_EMAIL = 'additional_mail';
|
||||
|
||||
public const NOT_VERIFIED = '0';
|
||||
|
|
|
@ -1,9 +1,11 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* @author Björn Schießle <schiessle@owncloud.com>
|
||||
*
|
||||
* @copyright Copyright (c) 2016, ownCloud, Inc.
|
||||
*
|
||||
* @author Björn Schießle <schiessle@owncloud.com>
|
||||
* @author Thomas Citharel <nextcloud@tcit.fr>
|
||||
*
|
||||
* @license AGPL-3.0
|
||||
*
|
||||
* This code is free software: you can redistribute it and/or modify
|
||||
|
@ -62,26 +64,25 @@ class AccountManagerTest extends TestCase {
|
|||
/** @var IFactory|MockObject */
|
||||
protected $l10nFactory;
|
||||
|
||||
/** @var \OCP\IDBConnection */
|
||||
/** @var IDBConnection */
|
||||
private $connection;
|
||||
|
||||
/** @var IConfig|MockObject */
|
||||
/** @var IConfig|MockObject */
|
||||
private $config;
|
||||
|
||||
/** @var EventDispatcherInterface|MockObject */
|
||||
private $eventDispatcher;
|
||||
|
||||
/** @var IJobList|MockObject */
|
||||
/** @var IJobList|MockObject */
|
||||
private $jobList;
|
||||
|
||||
/** @var string accounts table name */
|
||||
private $table = 'accounts';
|
||||
/** accounts table name */
|
||||
private string $table = 'accounts';
|
||||
|
||||
/** @var LoggerInterface|MockObject */
|
||||
private $logger;
|
||||
|
||||
/** @var AccountManager */
|
||||
private $accountManager;
|
||||
private AccountManager $accountManager;
|
||||
|
||||
protected function setUp(): void {
|
||||
parent::setUp();
|
||||
|
@ -115,7 +116,7 @@ class AccountManagerTest extends TestCase {
|
|||
protected function tearDown(): void {
|
||||
parent::tearDown();
|
||||
$query = $this->connection->getQueryBuilder();
|
||||
$query->delete($this->table)->execute();
|
||||
$query->delete($this->table)->executeStatement();
|
||||
}
|
||||
|
||||
protected function makeUser(string $uid, string $name, string $email = null): IUser {
|
||||
|
@ -423,6 +424,7 @@ class AccountManagerTest extends TestCase {
|
|||
],
|
||||
],
|
||||
];
|
||||
$this->config->expects($this->exactly(count($users)))->method('getSystemValue')->with('account_manager.default_property_scope', [])->willReturn([]);
|
||||
foreach ($users as $userInfo) {
|
||||
$this->invokePrivate($this->accountManager, 'updateUser', [$userInfo['user'], $userInfo['data'], null, false]);
|
||||
}
|
||||
|
@ -431,10 +433,9 @@ class AccountManagerTest extends TestCase {
|
|||
/**
|
||||
* get a instance of the accountManager
|
||||
*
|
||||
* @param array $mockedMethods list of methods which should be mocked
|
||||
* @return MockObject | AccountManager
|
||||
*/
|
||||
public function getInstance($mockedMethods = null) {
|
||||
public function getInstance(?array $mockedMethods = null) {
|
||||
return $this->getMockBuilder(AccountManager::class)
|
||||
->setConstructorArgs([
|
||||
$this->connection,
|
||||
|
@ -449,19 +450,15 @@ class AccountManagerTest extends TestCase {
|
|||
$this->urlGenerator,
|
||||
$this->crypto
|
||||
])
|
||||
->setMethods($mockedMethods)
|
||||
->onlyMethods($mockedMethods)
|
||||
->getMock();
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider dataTrueFalse
|
||||
*
|
||||
* @param array $newData
|
||||
* @param array $oldData
|
||||
* @param bool $insertNew
|
||||
* @param bool $updateExisting
|
||||
*/
|
||||
public function testUpdateUser($newData, $oldData, $insertNew, $updateExisting) {
|
||||
public function testUpdateUser(array $newData, array $oldData, bool $insertNew, bool $updateExisting) {
|
||||
$accountManager = $this->getInstance(['getUser', 'insertNewUser', 'updateExistingUser']);
|
||||
/** @var IUser $user */
|
||||
$user = $this->createMock(IUser::class);
|
||||
|
@ -487,7 +484,6 @@ class AccountManagerTest extends TestCase {
|
|||
function ($eventName, $event) use ($user, $newData) {
|
||||
$this->assertSame('OC\AccountManager::userUpdated', $eventName);
|
||||
$this->assertInstanceOf(GenericEvent::class, $event);
|
||||
/** @var GenericEvent $event */
|
||||
$this->assertSame($user, $event->getSubject());
|
||||
$this->assertSame($newData, $event->getArguments());
|
||||
}
|
||||
|
@ -603,6 +599,7 @@ class AccountManagerTest extends TestCase {
|
|||
'value' => '1',
|
||||
],
|
||||
];
|
||||
$this->config->expects($this->once())->method('getSystemValue')->with('account_manager.default_property_scope', [])->willReturn([]);
|
||||
|
||||
$defaultUserRecord = $this->invokePrivate($this->accountManager, 'buildDefaultUserRecord', [$user]);
|
||||
$result = $this->invokePrivate($this->accountManager, 'addMissingDefaultValues', [$input, $defaultUserRecord]);
|
||||
|
@ -610,18 +607,6 @@ class AccountManagerTest extends TestCase {
|
|||
$this->assertSame($expected, $result);
|
||||
}
|
||||
|
||||
private function addDummyValuesToTable($uid, $data) {
|
||||
$query = $this->connection->getQueryBuilder();
|
||||
$query->insert($this->table)
|
||||
->values(
|
||||
[
|
||||
'uid' => $query->createNamedParameter($uid),
|
||||
'data' => $query->createNamedParameter(json_encode($data)),
|
||||
]
|
||||
)
|
||||
->execute();
|
||||
}
|
||||
|
||||
public function testGetAccount() {
|
||||
$accountManager = $this->getInstance(['getUser']);
|
||||
/** @var IUser $user */
|
||||
|
@ -668,9 +653,6 @@ class AccountManagerTest extends TestCase {
|
|||
|
||||
/**
|
||||
* @dataProvider dataParsePhoneNumber
|
||||
* @param string $phoneInput
|
||||
* @param string $defaultRegion
|
||||
* @param string|null $phoneNumber
|
||||
*/
|
||||
public function testParsePhoneNumber(string $phoneInput, string $defaultRegion, ?string $phoneNumber): void {
|
||||
$this->config->method('getSystemValueString')
|
||||
|
@ -790,6 +772,8 @@ class AccountManagerTest extends TestCase {
|
|||
* @dataProvider dataCheckEmailVerification
|
||||
*/
|
||||
public function testCheckEmailVerification(IUser $user, ?string $newEmail): void {
|
||||
// Once because of getAccount, once because of getUser
|
||||
$this->config->expects($this->exactly(2))->method('getSystemValue')->with('account_manager.default_property_scope', [])->willReturn([]);
|
||||
$account = $this->accountManager->getAccount($user);
|
||||
$emailUpdated = false;
|
||||
|
||||
|
@ -812,4 +796,58 @@ class AccountManagerTest extends TestCase {
|
|||
$oldData = $this->invokePrivate($this->accountManager, 'getUser', [$user, false]);
|
||||
$this->invokePrivate($this->accountManager, 'checkEmailVerification', [$account, $oldData]);
|
||||
}
|
||||
|
||||
public function dataSetDefaultPropertyScopes(): array {
|
||||
return [
|
||||
[
|
||||
[],
|
||||
[
|
||||
IAccountManager::PROPERTY_DISPLAYNAME => IAccountManager::SCOPE_FEDERATED,
|
||||
IAccountManager::PROPERTY_ADDRESS => IAccountManager::SCOPE_LOCAL,
|
||||
IAccountManager::PROPERTY_EMAIL => IAccountManager::SCOPE_FEDERATED,
|
||||
IAccountManager::PROPERTY_ROLE => IAccountManager::SCOPE_LOCAL,
|
||||
]
|
||||
],
|
||||
[
|
||||
[
|
||||
IAccountManager::PROPERTY_DISPLAYNAME => IAccountManager::SCOPE_FEDERATED,
|
||||
IAccountManager::PROPERTY_EMAIL => IAccountManager::SCOPE_LOCAL,
|
||||
IAccountManager::PROPERTY_ROLE => IAccountManager::SCOPE_PRIVATE,
|
||||
], [
|
||||
IAccountManager::PROPERTY_DISPLAYNAME => IAccountManager::SCOPE_FEDERATED,
|
||||
IAccountManager::PROPERTY_EMAIL => IAccountManager::SCOPE_LOCAL,
|
||||
IAccountManager::PROPERTY_ROLE => IAccountManager::SCOPE_PRIVATE,
|
||||
]
|
||||
],
|
||||
[
|
||||
[
|
||||
IAccountManager::PROPERTY_ADDRESS => 'invalid scope',
|
||||
'invalid property' => IAccountManager::SCOPE_LOCAL,
|
||||
IAccountManager::PROPERTY_ROLE => IAccountManager::SCOPE_PRIVATE,
|
||||
],
|
||||
[
|
||||
IAccountManager::PROPERTY_ADDRESS => IAccountManager::SCOPE_LOCAL,
|
||||
IAccountManager::PROPERTY_EMAIL => IAccountManager::SCOPE_FEDERATED,
|
||||
IAccountManager::PROPERTY_ROLE => IAccountManager::SCOPE_PRIVATE,
|
||||
]
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider dataSetDefaultPropertyScopes
|
||||
*/
|
||||
public function testSetDefaultPropertyScopes(array $propertyScopes, array $expectedResultScopes): void {
|
||||
$user = $this->makeUser('steve', 'Steve Smith', 'steve@steve.steve');
|
||||
$this->config->expects($this->once())->method('getSystemValue')->with('account_manager.default_property_scope', [])->willReturn($propertyScopes);
|
||||
|
||||
$result = $this->invokePrivate($this->accountManager, 'buildDefaultUserRecord', [$user]);
|
||||
$resultProperties = array_column($result, 'name');
|
||||
|
||||
$this->assertEmpty(array_diff($resultProperties, IAccountManager::ALLOWED_PROPERTIES), "Building default user record returned non-allowed properties");
|
||||
foreach ($expectedResultScopes as $expectedResultScopeKey => $expectedResultScopeValue) {
|
||||
$resultScope = $result[array_search($expectedResultScopeKey, $resultProperties)]['scope'];
|
||||
$this->assertEquals($expectedResultScopeValue, $resultScope, "The result scope doesn't follow the value set into the config or defaults correctly.");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue