Skip to content

Commit 6a3c483

Browse files
committed
fix(controller): guard target user policy endpoints
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
1 parent 444610a commit 6a3c483

1 file changed

Lines changed: 59 additions & 3 deletions

File tree

lib/Controller/PolicyController.php

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,17 @@ public function getGroup(string $groupId, string $policyKey): DataResponse {
141141
*
142142
* @param string $userId Target user identifier that receives the policy preference.
143143
* @param string $policyKey Policy identifier to read for the selected user.
144-
* @return DataResponse<Http::STATUS_OK, LibresignUserPolicyResponse, array{}>
144+
* @return DataResponse<Http::STATUS_OK, LibresignUserPolicyResponse, array{}>|DataResponse<Http::STATUS_FORBIDDEN, LibresignErrorResponse, array{}>
145145
*
146146
* 200: OK
147+
* 403: Forbidden
147148
*/
148149
#[ApiRoute(verb: 'GET', url: '/api/{apiVersion}/policies/user/{userId}/{policyKey}', requirements: ['apiVersion' => '(v1)', 'userId' => '[^/]+', 'policyKey' => '[a-z0-9_]+'])]
149150
public function getUserPolicyForUser(string $userId, string $policyKey): DataResponse {
151+
if (!$this->canManageUserPolicy($userId)) {
152+
return $this->forbiddenUserPolicyResponse();
153+
}
154+
150155
$policy = $this->policyService->getUserPreferenceForUserId($policyKey, $userId);
151156

152157
/** @var LibresignUserPolicyResponse $data */
@@ -317,13 +322,18 @@ public function setUserPreference(string $policyKey, null|bool|int|float|string
317322
* @param string $userId Target user identifier that receives the policy preference.
318323
* @param string $policyKey Policy identifier to persist for the target user.
319324
* @param null|bool|int|float|string $value Policy value to persist as target user preference.
320-
* @return DataResponse<Http::STATUS_OK, LibresignUserPolicyWriteResponse, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, LibresignErrorResponse, array{}>
325+
* @return DataResponse<Http::STATUS_OK, LibresignUserPolicyWriteResponse, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, LibresignErrorResponse, array{}>|DataResponse<Http::STATUS_FORBIDDEN, LibresignErrorResponse, array{}>
321326
*
322327
* 200: OK
323328
* 400: Invalid policy value
329+
* 403: Forbidden
324330
*/
325331
#[ApiRoute(verb: 'PUT', url: '/api/{apiVersion}/policies/user/{userId}/{policyKey}', requirements: ['apiVersion' => '(v1)', 'userId' => '[^/]+', 'policyKey' => '[a-z0-9_]+'])]
326332
public function setUserPolicyForUser(string $userId, string $policyKey, null|bool|int|float|string $value = null): DataResponse {
333+
if (!$this->canManageUserPolicy($userId)) {
334+
return $this->forbiddenUserPolicyResponse();
335+
}
336+
327337
$value = $this->readScalarParam('value', $value);
328338

329339
try {
@@ -371,12 +381,17 @@ public function clearUserPreference(string $policyKey): DataResponse {
371381
*
372382
* @param string $userId Target user identifier that receives the policy preference removal.
373383
* @param string $policyKey Policy identifier to clear for the target user.
374-
* @return DataResponse<Http::STATUS_OK, LibresignUserPolicyWriteResponse, array{}>
384+
* @return DataResponse<Http::STATUS_OK, LibresignUserPolicyWriteResponse, array{}>|DataResponse<Http::STATUS_FORBIDDEN, LibresignErrorResponse, array{}>
375385
*
376386
* 200: OK
387+
* 403: Forbidden
377388
*/
378389
#[ApiRoute(verb: 'DELETE', url: '/api/{apiVersion}/policies/user/{userId}/{policyKey}', requirements: ['apiVersion' => '(v1)', 'userId' => '[^/]+', 'policyKey' => '[a-z0-9_]+'])]
379390
public function clearUserPolicyForUser(string $userId, string $policyKey): DataResponse {
391+
if (!$this->canManageUserPolicy($userId)) {
392+
return $this->forbiddenUserPolicyResponse();
393+
}
394+
380395
$policy = $this->policyService->clearUserPreferenceForUserId($policyKey, $userId);
381396
/** @var LibresignUserPolicyWriteResponse $data */
382397
$data = [
@@ -428,6 +443,37 @@ private function canManageGroupPolicy(string $groupId): bool {
428443
return $this->subAdmin->isSubAdminOfGroup($user, $group);
429444
}
430445

446+
private function canManageUserPolicy(string $userId): bool {
447+
$user = $this->userSession->getUser();
448+
if ($user === null) {
449+
return false;
450+
}
451+
452+
if ($this->groupManager->isAdmin($user->getUID())) {
453+
return true;
454+
}
455+
456+
if (!$this->subAdmin->isSubAdmin($user)) {
457+
return false;
458+
}
459+
460+
$targetUser = $this->userManager->get($userId);
461+
if (!$targetUser instanceof IUser) {
462+
return false;
463+
}
464+
465+
$managedGroupIds = array_values(array_map(
466+
static fn ($group): string => $group->getGID(),
467+
$this->subAdmin->getSubAdminsGroups($user),
468+
));
469+
if ($managedGroupIds === []) {
470+
return false;
471+
}
472+
473+
$targetGroupIds = $this->groupManager->getUserGroupIds($targetUser);
474+
return array_intersect($managedGroupIds, $targetGroupIds) !== [];
475+
}
476+
431477
/**
432478
* @return array<string, array{groupCount: int, userCount: int}>
433479
*/
@@ -483,4 +529,14 @@ private function forbiddenGroupPolicyResponse(): DataResponse {
483529

484530
return new DataResponse($data, Http::STATUS_FORBIDDEN);
485531
}
532+
533+
/** @return DataResponse<Http::STATUS_FORBIDDEN, LibresignErrorResponse, array{}> */
534+
private function forbiddenUserPolicyResponse(): DataResponse {
535+
/** @var LibresignErrorResponse $data */
536+
$data = [
537+
'error' => $this->l10n->t('Not allowed to manage this user policy'),
538+
];
539+
540+
return new DataResponse($data, Http::STATUS_FORBIDDEN);
541+
}
486542
}

0 commit comments

Comments
 (0)