diff options
| -rw-r--r-- | apps/provisioning_api/lib/Controller/UsersController.php | 2 | ||||
| -rw-r--r-- | lib/private/Accounts/AccountProperty.php | 31 | ||||
| -rw-r--r-- | lib/public/Accounts/IAccountProperty.php | 3 | ||||
| -rw-r--r-- | tests/lib/Accounts/AccountManagerTest.php | 20 | ||||
| -rw-r--r-- | tests/lib/Accounts/AccountPropertyTest.php | 12 |
5 files changed, 49 insertions, 19 deletions
diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 84fdaf8bc89..f254620f330 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -759,8 +759,8 @@ class UsersController extends AUserData { $userAccount = $this->accountManager->getAccount($targetUser); $userProperty = $userAccount->getProperty($propertyName); if ($userProperty->getScope() !== $value) { - $userProperty->setScope($value); try { + $userProperty->setScope($value); $this->accountManager->updateAccount($userAccount); } catch (\InvalidArgumentException $e) { throw new OCSException('Invalid ' . $e->getMessage(), 102); diff --git a/lib/private/Accounts/AccountProperty.php b/lib/private/Accounts/AccountProperty.php index 850f39df9e3..0152137f6de 100644 --- a/lib/private/Accounts/AccountProperty.php +++ b/lib/private/Accounts/AccountProperty.php @@ -43,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 = $this->mapScopeToV2($scope); + $this->setScope($scope); $this->verified = $verified; } @@ -78,7 +78,16 @@ class AccountProperty implements IAccountProperty { * @return IAccountProperty */ public function setScope(string $scope): IAccountProperty { - $this->scope = $this->mapScopeToV2($scope); + $newScope = $this->mapScopeToV2($scope); + if (!in_array($newScope, [ + IAccountManager::SCOPE_LOCAL, + IAccountManager::SCOPE_FEDERATED, + IAccountManager::SCOPE_PRIVATE, + IAccountManager::SCOPE_PUBLISHED + ])) { + throw new \InvalidArgumentException('Invalid scope'); + } + $this->scope = $newScope; return $this; } @@ -128,21 +137,21 @@ class AccountProperty implements IAccountProperty { return $this->scope; } - public static function mapScopeToV2($scope) { + public static function mapScopeToV2(string $scope): string { 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; + 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; + default: + return $scope; } - - return IAccountManager::SCOPE_LOCAL; } /** diff --git a/lib/public/Accounts/IAccountProperty.php b/lib/public/Accounts/IAccountProperty.php index 657121a27e8..1366ddd9543 100644 --- a/lib/public/Accounts/IAccountProperty.php +++ b/lib/public/Accounts/IAccountProperty.php @@ -26,6 +26,8 @@ declare(strict_types=1); namespace OCP\Accounts; +use InvalidArgumentException; + /** * Interface IAccountProperty * @@ -50,6 +52,7 @@ interface IAccountProperty extends \JsonSerializable { * * @param string $scope * @return IAccountProperty + * @throws InvalidArgumentException (since 22.0.0) */ public function setScope(string $scope): IAccountProperty; diff --git a/tests/lib/Accounts/AccountManagerTest.php b/tests/lib/Accounts/AccountManagerTest.php index 687ae29ff7b..1da128ae124 100644 --- a/tests/lib/Accounts/AccountManagerTest.php +++ b/tests/lib/Accounts/AccountManagerTest.php @@ -213,8 +213,8 @@ class AccountManagerTest extends TestCase { // 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_TWITTER => ['value' => '@sometwitter', 'scope' => IAccountManager::SCOPE_LOCAL], + IAccountManager::PROPERTY_PHONE => ['value' => '+491601231212', 'scope' => IAccountManager::SCOPE_LOCAL], ], [ IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_LOCAL], @@ -222,9 +222,23 @@ class AccountManagerTest extends TestCase { 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, ], + // illegal scope values + [ + [ + 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_PHONE => ['value' => '+491601231212', 'scope' => ''], + IAccountManager::PROPERTY_ADDRESS => ['value' => 'some street', 'scope' => 'v2-invalid'], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://example.org', 'scope' => 'invalid'], + ], + [], + true, true + ], // invalid or unsupported scope values throw an exception when passing $throwOnData=true [ [IAccountManager::PROPERTY_DISPLAYNAME => ['value' => 'Display Name', 'scope' => IAccountManager::SCOPE_PUBLISHED]], diff --git a/tests/lib/Accounts/AccountPropertyTest.php b/tests/lib/Accounts/AccountPropertyTest.php index f99abc21f83..84059bbc35d 100644 --- a/tests/lib/Accounts/AccountPropertyTest.php +++ b/tests/lib/Accounts/AccountPropertyTest.php @@ -80,16 +80,20 @@ class AccountPropertyTest extends TestCase { [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], + // invalid values + ['', null], + ['unknown', null], + ['v2-unknown', null], ]; } /** * @dataProvider scopesProvider */ - public function testSetScopeMapping($storedScope, $returnedScope) { + public function testSetScopeMapping(string $storedScope, ?string $returnedScope) { + if ($returnedScope === null) { + $this->expectException(\InvalidArgumentException::class); + } $accountProperty = new AccountProperty( IAccountManager::PROPERTY_WEBSITE, 'https://example.com', |