Skip to content

Commit 3b3cf6a

Browse files
miaulalalamiaulalala
committed
feat(accounts): add admin-configurable per-property scope ceiling
Admins can now set a maximum allowed visibility scope per account property via the system config key `account_manager.max_property_scope` (array of property name => max scope, e.g. `['email' => 'v2-local', 'website' => 'v2-local']`). Backend (`AccountManager::testPropertyScope`) rejects any scope that exceeds the configured ceiling, returning `Invalid scope` via the API. The frontend (`FederationControl.vue`) also filters out disallowed scope options so users only see choices their admin permits. `PersonalInfo.php` sanitises and passes the config to the frontend via initial state. `IAccountManager` adds `PROPERTY_SCOPE_ORDER` documenting the visibility ordering used for ceiling comparisons. Also fixes a latent mutation bug in `FederationControl.vue` where `supportedScopes` was calling `.push()` on the inner arrays of the `Object.freeze`d `PROPERTY_READABLE_SUPPORTED_SCOPES_ENUM` constant, causing duplicate entries to accumulate across re-renders. Signed-off-by: Anna Larch <anna@nextcloud.com> AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 4082691 commit 3b3cf6a

8 files changed

Lines changed: 139 additions & 13 deletions

File tree

‎apps/settings/lib/Settings/Personal/PersonalInfo.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,19 @@ public function getForm(): TemplateResponse {
115115
'pronouns' => $this->getProperty($account, IAccountManager::PROPERTY_PRONOUNS),
116116
];
117117

118+
$maxPropertyScopes = array_filter(
119+
$this->config->getSystemValue('account_manager.max_property_scope', []),
120+
static fn (string $scope, string $property): bool => in_array($property, IAccountManager::ALLOWED_PROPERTIES, true) && in_array($scope, IAccountManager::ALLOWED_SCOPES, true),
121+
ARRAY_FILTER_USE_BOTH,
122+
);
123+
118124
$accountParameters = [
119125
'avatarChangeSupported' => $user->canChangeAvatar(),
120126
'displayNameChangeSupported' => $user->canChangeDisplayName(),
121127
'emailChangeSupported' => $user->canChangeEmail(),
122128
'federationEnabled' => $federationEnabled,
123129
'lookupServerUploadEnabled' => $lookupServerUploadEnabled,
130+
'maxPropertyScopes' => $maxPropertyScopes,
124131
];
125132

126133
$profileParameters = [

‎apps/settings/src/components/PersonalInfo/shared/FederationControl.vue‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import { handleError } from '../../../utils/handlers.ts'
5252
const {
5353
federationEnabled,
5454
lookupServerUploadEnabled,
55+
maxPropertyScopes,
5556
} = loadState('settings', 'accountParameters', {})
5657
5758
export default {
@@ -123,18 +124,24 @@ export default {
123124
},
124125
125126
supportedScopes() {
126-
const scopes = PROPERTY_READABLE_SUPPORTED_SCOPES_ENUM[this.readable]
127-
128-
if (UNPUBLISHED_READABLE_PROPERTIES.includes(this.readable)) {
129-
return scopes
130-
}
131-
132-
if (federationEnabled) {
133-
scopes.push(SCOPE_ENUM.FEDERATED)
127+
const scopes = [...PROPERTY_READABLE_SUPPORTED_SCOPES_ENUM[this.readable]]
128+
129+
if (!UNPUBLISHED_READABLE_PROPERTIES.includes(this.readable)) {
130+
if (federationEnabled) {
131+
scopes.push(SCOPE_ENUM.FEDERATED)
132+
}
133+
if (lookupServerUploadEnabled) {
134+
scopes.push(SCOPE_ENUM.PUBLISHED)
135+
}
134136
}
135137
136-
if (lookupServerUploadEnabled) {
137-
scopes.push(SCOPE_ENUM.PUBLISHED)
138+
// Apply admin-configured scope ceiling for this property.
139+
const propertyKey = PROPERTY_READABLE_KEYS_ENUM[this.readable]
140+
const maxScope = propertyKey && maxPropertyScopes?.[propertyKey]
141+
if (maxScope) {
142+
const order = [SCOPE_ENUM.PRIVATE, SCOPE_ENUM.LOCAL, SCOPE_ENUM.FEDERATED, SCOPE_ENUM.PUBLISHED]
143+
const maxIndex = order.indexOf(maxScope)
144+
return scopes.filter((scope) => order.indexOf(scope) <= maxIndex)
138145
}
139146
140147
return scopes

‎config/config.sample.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2886,6 +2886,29 @@
28862886
*/
28872887
'account_manager.default_property_scope' => [],
28882888

2889+
/**
2890+
* Set a maximum allowed visibility scope for individual account properties.
2891+
* Users cannot set a property to a scope more visible than the configured
2892+
* ceiling, neither through the UI nor the API.
2893+
*
2894+
* Valid property names and scope values are defined in
2895+
* ``OCP\Accounts\IAccountManager``.
2896+
*
2897+
* Example: Prevent users from making their email or website visible beyond
2898+
* the local instance:
2899+
* ``[
2900+
* \OCP\Accounts\IAccountManager::PROPERTY_EMAIL => \OCP\Accounts\IAccountManager::SCOPE_LOCAL,
2901+
* \OCP\Accounts\IAccountManager::PROPERTY_WEBSITE => \OCP\Accounts\IAccountManager::SCOPE_LOCAL,
2902+
* ]``
2903+
*
2904+
* WARNING: Restricting the scope of properties that are required for
2905+
* federation (``displayname``, ``email``, ``avatar``, ``pronouns``) below
2906+
* ``SCOPE_FEDERATED`` will break federated sharing and other cross-instance
2907+
* features that depend on those fields being visible to trusted remote
2908+
* servers.
2909+
*/
2910+
'account_manager.max_property_scope' => [],
2911+
28892912
/**
28902913
* Enable the deprecated Projects feature, superseded by Related Resources since
28912914
* Nextcloud 25.

‎dist/settings-vue-settings-personal-info.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/settings-vue-settings-personal-info.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/private/Accounts/AccountManager.php‎

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

123+
// Enforce admin-configured per-property scope ceiling.
124+
$maxScopes = $this->config->getSystemValue('account_manager.max_property_scope', []);
125+
if (isset($maxScopes[$property->getName()])) {
126+
$maxScope = $maxScopes[$property->getName()];
127+
$currentOrder = self::SCOPE_ORDER[$property->getScope()] ?? 0;
128+
$maxOrder = self::SCOPE_ORDER[$maxScope] ?? PHP_INT_MAX;
129+
if ($currentOrder > $maxOrder) {
130+
if ($throwOnData) {
131+
throw new InvalidArgumentException('scope');
132+
} else {
133+
$property->setScope($maxScope);
134+
}
135+
}
136+
}
137+
123138
if (
124139
$property->getScope() === self::SCOPE_PRIVATE
125140
&& in_array($property->getName(), [self::PROPERTY_DISPLAYNAME, self::PROPERTY_EMAIL])

‎lib/public/Accounts/IAccountManager.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,25 @@ interface IAccountManager {
5959
self::SCOPE_PUBLISHED,
6060
];
6161

62+
/**
63+
* Visibility order of scopes from least to most visible.
64+
* Used to compare scope levels when enforcing admin-configured ceilings
65+
* via the ``account_manager.max_property_scope`` system config key.
66+
*
67+
* Warning: restricting properties that federation depends on
68+
* (``PROPERTY_DISPLAYNAME``, ``PROPERTY_EMAIL``, ``PROPERTY_AVATAR``,
69+
* ``PROPERTY_PRONOUNS``) below ``SCOPE_FEDERATED`` will break federated
70+
* sharing and other cross-instance features.
71+
*
72+
* @since 32.0.0
73+
*/
74+
public const SCOPE_ORDER = [
75+
self::SCOPE_PRIVATE => 0,
76+
self::SCOPE_LOCAL => 1,
77+
self::SCOPE_FEDERATED => 2,
78+
self::SCOPE_PUBLISHED => 3,
79+
];
80+
6281
/**
6382
* @since 15.0.0
6483
*/

‎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 function testUpdateAccountRejectsScoperAboveAdminCeiling(): void {
1067+
$user = $this->createMock(IUser::class);
1068+
$account = new Account($user);
1069+
$account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_FEDERATED, IAccountManager::NOT_VERIFIED);
1070+
1071+
$manager = $this->getInstance(['getUser', 'updateUser']);
1072+
$manager->method('getUser')->with($user, false)->willReturn([]);
1073+
$this->config->method('getSystemValue')
1074+
->willReturnMap([
1075+
['account_manager.default_property_scope', [], []],
1076+
['account_manager.max_property_scope', [], [IAccountManager::PROPERTY_WEBSITE => IAccountManager::SCOPE_LOCAL]],
1077+
]);
1078+
1079+
$this->expectException(\InvalidArgumentException::class);
1080+
$this->expectExceptionMessage('scope');
1081+
$manager->updateAccount($account);
1082+
}
1083+
1084+
public function testUpdateAccountAllowsScopeAtOrBelowAdminCeiling(): void {
1085+
$user = $this->createMock(IUser::class);
1086+
$account = new Account($user);
1087+
$account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_LOCAL, IAccountManager::NOT_VERIFIED);
1088+
1089+
$manager = $this->getInstance(['getUser', 'updateUser']);
1090+
$manager->method('getUser')->with($user, false)->willReturn([]);
1091+
$this->config->method('getSystemValueString')->willReturn('');
1092+
$this->config->method('getSystemValue')
1093+
->willReturnMap([
1094+
['account_manager.default_property_scope', [], []],
1095+
['account_manager.max_property_scope', [], [IAccountManager::PROPERTY_WEBSITE => IAccountManager::SCOPE_LOCAL]],
1096+
]);
1097+
$manager->expects($this->once())->method('updateUser');
1098+
1099+
$manager->updateAccount($account);
1100+
}
1101+
1102+
public function testUpdateAccountIgnoresInvalidMaxScopeConfig(): void {
1103+
$user = $this->createMock(IUser::class);
1104+
$account = new Account($user);
1105+
$account->setProperty(IAccountManager::PROPERTY_WEBSITE, 'https://example.com', IAccountManager::SCOPE_FEDERATED, IAccountManager::NOT_VERIFIED);
1106+
1107+
$manager = $this->getInstance(['getUser', 'updateUser']);
1108+
$manager->method('getUser')->with($user, false)->willReturn([]);
1109+
$this->config->method('getSystemValueString')->willReturn('');
1110+
$this->config->method('getSystemValue')
1111+
->willReturnMap([
1112+
['account_manager.default_property_scope', [], []],
1113+
// 'not-a-scope' is not a valid scope value, so no ceiling applies
1114+
['account_manager.max_property_scope', [], [IAccountManager::PROPERTY_WEBSITE => 'not-a-scope']],
1115+
]);
1116+
$manager->expects($this->once())->method('updateUser');
1117+
1118+
$manager->updateAccount($account);
1119+
}
10651120
}

0 commit comments

Comments
 (0)