Skip to content

Commit c3b59d8

Browse files
committed
fix(accounts): enforce per-property scope restrictions in AccountManager
Adds `UNPUBLISHED_PROPERTIES` to `IAccountManager` for profile fields that must never be federated or published to the global lookup server (biography, birthdate, headline, organisation, role), matching the frontend's `UNPUBLISHED_READABLE_PROPERTIES` constant. Enforces two new constraints in `testPropertyScope`: - `UNPUBLISHED_PROPERTIES` may not use `SCOPE_FEDERATED` or `SCOPE_PUBLISHED`, even when set via the API. - `SCOPE_PUBLISHED` is rejected for all properties unless the admin has enabled lookup server upload (`files_sharing.lookupServerUploadEnabled`). Previously the `PUT /ocs/v2.php/cloud/users/<uid>` endpoint accepted any valid scope value regardless of these restrictions, allowing users to bypass the visibility limits enforced by the frontend UI. Fixes #59225 Signed-off-by: Anna Larch <anna@nextcloud.com> AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent fcb956b commit c3b59d8

3 files changed

Lines changed: 94 additions & 0 deletions

File tree

lib/private/Accounts/AccountManager.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,30 @@ protected function testPropertyScope(IAccountProperty $property, array $allowedS
120120
throw new InvalidArgumentException('scope');
121121
}
122122

123+
// Properties that are local-only must not be set to FEDERATED or PUBLISHED scope.
124+
if (
125+
in_array($property->getName(), self::UNPUBLISHED_PROPERTIES, true)
126+
&& in_array($property->getScope(), [self::SCOPE_FEDERATED, self::SCOPE_PUBLISHED], true)
127+
) {
128+
if ($throwOnData) {
129+
throw new InvalidArgumentException('scope');
130+
} else {
131+
$property->setScope(self::SCOPE_LOCAL);
132+
}
133+
}
134+
135+
// PUBLISHED scope requires the lookup server upload to be enabled by the admin.
136+
if ($property->getScope() === self::SCOPE_PUBLISHED) {
137+
$lookupServerUploadEnabled = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') === 'yes';
138+
if (!$lookupServerUploadEnabled) {
139+
if ($throwOnData) {
140+
throw new InvalidArgumentException('scope');
141+
} else {
142+
$property->setScope(self::SCOPE_LOCAL);
143+
}
144+
}
145+
}
146+
123147
if (
124148
$property->getScope() === self::SCOPE_PRIVATE
125149
&& in_array($property->getName(), [self::PROPERTY_DISPLAYNAME, self::PROPERTY_EMAIL])

lib/public/Accounts/IAccountManager.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,21 @@ interface IAccountManager {
146146
*/
147147
public const PROPERTY_PRONOUNS = 'pronouns';
148148

149+
/**
150+
* Properties that are only visible within the local instance and must not be
151+
* published to the global lookup server or shared via federation.
152+
* This matches the frontend's UNPUBLISHED_READABLE_PROPERTIES constant.
153+
*
154+
* @since 32.0.0
155+
*/
156+
public const UNPUBLISHED_PROPERTIES = [
157+
self::PROPERTY_BIOGRAPHY,
158+
self::PROPERTY_BIRTHDATE,
159+
self::PROPERTY_HEADLINE,
160+
self::PROPERTY_ORGANISATION,
161+
self::PROPERTY_ROLE,
162+
];
163+
149164
/**
150165
* The list of allowed properties
151166
*

tests/lib/Accounts/AccountManagerTest.php

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,4 +1062,59 @@ public function testSetDefaultPropertyScopes(array $propertyScopes, array $expec
10621062
$this->assertEquals($expectedResultScopeValue, $resultScope, "The result scope doesn't follow the value set into the config or defaults correctly.");
10631063
}
10641064
}
1065+
1066+
public static function dataUpdateAccountRejectsInvalidScopeForUnpublishedProperty(): array {
1067+
$cases = [];
1068+
foreach (IAccountManager::UNPUBLISHED_PROPERTIES as $property) {
1069+
$cases[] = [$property, IAccountManager::SCOPE_FEDERATED];
1070+
$cases[] = [$property, IAccountManager::SCOPE_PUBLISHED];
1071+
}
1072+
return $cases;
1073+
}
1074+
1075+
#[\PHPUnit\Framework\Attributes\DataProvider('dataUpdateAccountRejectsInvalidScopeForUnpublishedProperty')]
1076+
public function testUpdateAccountRejectsInvalidScopeForUnpublishedProperty(string $property, string $scope): void {
1077+
$user = $this->createMock(IUser::class);
1078+
$account = new Account($user);
1079+
$account->setProperty($property, 'some value', $scope, IAccountManager::NOT_VERIFIED);
1080+
1081+
$manager = $this->getInstance(['getUser', 'updateUser']);
1082+
$manager->method('getUser')->with($user, false)->willReturn([]);
1083+
1084+
$this->expectException(\InvalidArgumentException::class);
1085+
$this->expectExceptionMessage('scope');
1086+
$manager->updateAccount($account);
1087+
}
1088+
1089+
public function testUpdateAccountRejectsPublishedScopeWhenLookupServerDisabled(): void {
1090+
$user = $this->createMock(IUser::class);
1091+
$account = new Account($user);
1092+
$account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED);
1093+
1094+
$manager = $this->getInstance(['getUser', 'updateUser']);
1095+
$manager->method('getUser')->with($user, false)->willReturn([]);
1096+
$this->config->method('getAppValue')
1097+
->with('files_sharing', 'lookupServerUploadEnabled', 'no')
1098+
->willReturn('no');
1099+
1100+
$this->expectException(\InvalidArgumentException::class);
1101+
$this->expectExceptionMessage('scope');
1102+
$manager->updateAccount($account);
1103+
}
1104+
1105+
public function testUpdateAccountAllowsPublishedScopeWhenLookupServerEnabled(): void {
1106+
$user = $this->createMock(IUser::class);
1107+
$account = new Account($user);
1108+
$account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_PUBLISHED, IAccountManager::NOT_VERIFIED);
1109+
1110+
$manager = $this->getInstance(['getUser', 'updateUser']);
1111+
$manager->method('getUser')->with($user, false)->willReturn([]);
1112+
$this->config->method('getSystemValueString')->willReturn('');
1113+
$this->config->method('getAppValue')
1114+
->with('files_sharing', 'lookupServerUploadEnabled', 'no')
1115+
->willReturn('yes');
1116+
$manager->expects($this->once())->method('updateUser');
1117+
1118+
$manager->updateAccount($account);
1119+
}
10651120
}

0 commit comments

Comments
 (0)