Refactor permission handling to leverage standardized "forbidden" responses and enhance unit test coverage.

This commit is contained in:
Jeppe Bundgaard
2026-03-19 15:59:25 +01:00
parent 3752fdec4c
commit b547a8b029
17 changed files with 321 additions and 89 deletions
+84 -5
View File
@@ -321,6 +321,71 @@ trait route_t
return null;
}
/**
* Convert a permission definition to its canonical string key.
*/
private function permissionKey(string|permission_node $permission): string
{
return $permission instanceof permission_node
? (string)$permission->permission
: (string)$permission;
}
/**
* Normalize and de-duplicate permission keys for forbidden responses.
* Accepts string keys and permission_node definitions.
*
* @param array<int, string|permission_node> $permissions
* @return array<int, string>
*/
private function normalizePermissionKeys(array $permissions): array
{
$keys = [];
foreach ($permissions as $permission) {
if ($permission instanceof permission_node) {
$key = trim((string)$permission->permission);
} elseif (is_string($permission)) {
$key = trim($permission);
} else {
continue;
}
if ($key !== '') {
$keys[] = $key;
}
}
return array_values(array_unique($keys));
}
/**
* Emit standardized forbidden response payload with missing permission keys.
* Extracted to allow focused unit tests by overriding this method.
*
* @param array<int, string|permission_node> $permissions
*/
protected function emitForbidden(array $permissions): void
{
global $response;
$response->forbidden($this->normalizePermissionKeys($permissions));
}
/**
* Emit a forbidden response for missing department scope access.
* Optionally include bypass permissions if they are relevant and missing.
*
* @param int $departmentId
* @param array<int, string|permission_node> $optionalBypassPermissions
*/
public function forbidDepartmentAccess(int $departmentId, array $optionalBypassPermissions = []): void
{
$missing = ['department_access_' . $departmentId];
foreach ($optionalBypassPermissions as $permission) {
if (!$this->hasPermission($permission)) {
$missing[] = $permission;
}
}
$this->emitForbidden($missing);
}
/**
* Centralized permission evaluation used by both requirePermission and hasPermission.
* - Honors subusers permission nodes without falling back to classic user permissions when a node is defined.
@@ -340,7 +405,7 @@ trait route_t
if ($resolvedCustomer === null) {
(new logs_o())->add('global', 'global', 1, $subuser->id ?? 0, 'PERMISSION_DENIED', 'Missing customer context for subuser permission evaluation: ' . $permission->permission);
if ($throwOnDeny) {
$response->error('Permission denied. Missing customer context for subuser.', 403);
$this->emitForbidden([$permission]);
}
return false;
}
@@ -367,7 +432,7 @@ trait route_t
if (!$subuser_has_permission && $throwOnDeny) {
(new logs_o())->add('global', 'global', 1, $subuser->id ?? 0, 'PERMISSION_DENIED', 'Permission denied via subuser node: ' . $permission->permission . ' (Node: ' . $permission->subusers_node_key->name . ', Customer: ' . $resolvedCustomer . ')');
$response->error('Permission denied for subuser. Missing permission: ' . $permission->permission . ' (Customer context: ' . $resolvedCustomer . ')', 403);
$this->emitForbidden([$permission]);
}
return $subuser_has_permission;
}
@@ -404,7 +469,7 @@ trait route_t
if (!$allowed && $throwOnDeny) {
(new logs_o())->add('global', 'global', 1, $user->id, 'PERMISSION_DENIED', 'Permission denied. Missing permission: ' . $perm_string);
$response->error('Permission denied. Missing permission: ' . $perm_string . ' for user: ' . $user->id . ' In group: ' . $user->group_id->value(), 403);
$this->emitForbidden([$perm_string]);
}
return $allowed;
} catch (Exception $e) {
@@ -528,7 +593,21 @@ trait route_t
}
if (!$allowed) {
$response->error($denyMessage ?? 'Permission denied.', 403);
$missingPermissions = [];
if (!$hasOwn) {
$missingPermissions[] = $permissionOwn;
}
if (!$hasOther) {
$missingPermissions[] = $permissionOther;
}
// In own-scope failures (wrong customer/guard failure), report missing elevated permission only.
if ($hasOwn && !$hasOther) {
$missingPermissions[] = $permissionOther;
}
if (count($missingPermissions) === 0) {
$missingPermissions[] = $permissionOther;
}
$this->emitForbidden($missingPermissions);
}
return $allowed;
}
@@ -816,4 +895,4 @@ trait route_t
// Check if route is the same, or if it matches the regex pattern
return $route === $this->route || preg_match($route, $this->route);
}
}
}