diff --git a/pages/admin/users/edit($id).php b/pages/admin/users/edit($id).php index a6e6f29..cc992c6 100644 --- a/pages/admin/users/edit($id).php +++ b/pages/admin/users/edit($id).php @@ -40,26 +40,50 @@ $passwordHints = $userPasswordPolicyService->hints(); $currentUserId = (int) ($session['user']['id'] ?? 0); $uuid = trim((string) ($id ?? '')); $editTarget = requestPathWithReturnTarget("admin/users/edit/{$uuid}", $returnTarget); + +// Resolve the user BEFORE the aggregator so the authorize-context can carry +// the real target_user_id (matches today's order: lookup -> CONTEXT authorize +// -> Router::redirect on forbidden). The aggregator receives a trivial finder +// that returns the already-loaded model so its not-found branch fires only +// when findByUuid() returned null. $user = $uuid !== '' ? $userAccountService->findByUuid($uuid) : null; -if (!$user) { - Flash::error('User not found', $closeTarget, 'user_not_found'); - Router::redirect($closeTarget); -} - $userId = (int) ($user['id'] ?? 0); -$contextDecision = $authorizationService->authorize(UserAuthorizationPolicy::ABILITY_ADMIN_USERS_EDIT_CONTEXT, [ - 'actor_user_id' => $currentUserId, - 'target_user_id' => $userId, -]); -if (!$contextDecision->isAllowed()) { - Router::redirect('error/forbidden'); - return; -} -$capabilities = $contextDecision->attribute('capabilities', []); -if (!is_array($capabilities)) { - $capabilities = []; -} +$context = actionEditContext([ + 'finder' => static fn (string $_id): mixed => $user, + 'rawId' => $uuid, + 'notFoundFlashKey' => 'User not found', + 'notFoundRedirectPath' => $closeTarget, + 'notFoundFlashScopeKey' => 'user_not_found', + 'abilityKey' => UserAuthorizationPolicy::ABILITY_ADMIN_USERS_EDIT_CONTEXT, + 'context' => [ + 'actor_user_id' => $currentUserId, + 'target_user_id' => $userId, + ], + 'viewAuthFlags' => [ + 'can_view_users', + 'can_view_page', + 'can_edit_user', + 'can_edit_assignments', + 'can_manage_tenants', + 'can_edit_custom_field_values', + 'can_manage_api_tokens', + 'can_view_user_meta', + 'can_view_user_audit', + 'can_access_pdf', + 'can_delete_user', + 'can_view_security_artifacts', + 'can_view_permissions_table', + 'is_own_account', + ], + 'forbiddenStrategy' => 'redirect', + 'tenantScopeFlagKey' => 'can_manage_tenants', +]); +$user = is_array($context['model']) ? $context['model'] : $user; +$capabilities = $context['capabilities']; +$tenantScope = $context['tenantScope']; +$viewAuth = $context['viewAuth']; + $canViewUsers = (bool) ($capabilities['can_view_users'] ?? false); $canViewPage = (bool) ($capabilities['can_view_page'] ?? false); $canEditUser = (bool) ($capabilities['can_edit_user'] ?? false); @@ -74,15 +98,11 @@ $canDeleteUser = (bool) ($capabilities['can_delete_user'] ?? false); $canViewSecurityArtifacts = (bool) ($capabilities['can_view_security_artifacts'] ?? false); $canViewPermissionsTable = (bool) ($capabilities['can_view_permissions_table'] ?? false); $isOwnAccount = (bool) ($capabilities['is_own_account'] ?? false); -$allowedTenantIdsRaw = $capabilities['allowed_tenant_ids'] ?? null; -$allowedTenantIds = is_array($allowedTenantIdsRaw) - ? array_values(array_unique(array_filter(array_map('intval', $allowedTenantIdsRaw), static fn (int $tenantId): bool => $tenantId > 0))) - : null; - -if (!$canViewPage) { - Router::redirect('error/forbidden'); - return; -} +// $canManageAllTenants = Tupel-Diskriminator (true wenn Aggregator scope='all' +// liefert via tenantScopeFlagKey='can_manage_tenants'-Override). $canManageTenants +// bleibt als Capability-Flag erhalten — semantisch aequivalent, aber unterschiedliche +// Quelle. Filter-Branches unten nutzen $canManageAllTenants als Tupel-Diskriminator. +$canManageAllTenants = $tenantScope['scope'] === 'all'; if ($canViewUserMeta || $canViewUserAudit) { app(\MintyPHP\Service\Audit\AuditMetadataEnricherInterface::class)->enrich($user, ['created_by', 'modified_by', 'active_changed_by']); @@ -95,18 +115,20 @@ $scopeGateway = app(\MintyPHP\Service\Tenant\TenantScopeService::class); $strictTenantScope = $scopeGateway->isStrict(); $tenants = $tenantService->list(); -if (is_array($allowedTenantIds)) { - $tenants = array_values(array_filter($tenants, static function (array $tenant) use ($allowedTenantIds): bool { - $tenantId = (int) ($tenant['id'] ?? 0); - return $tenantId > 0 && in_array($tenantId, $allowedTenantIds, true); - })); -} elseif (!$canManageTenants && $strictTenantScope) { - $tenants = []; +if (!$canManageAllTenants) { + if (count($tenantScope['ids']) > 0) { + $tenants = array_values(array_filter($tenants, static function (array $tenant) use ($tenantScope): bool { + $tenantId = (int) ($tenant['id'] ?? 0); + return $tenantId > 0 && in_array($tenantId, $tenantScope['ids'], true); + })); + } elseif ($strictTenantScope) { + $tenants = []; + } } $selectedTenantIds = $userTenantRepository->listTenantIdsByUserId($userId); -if (is_array($allowedTenantIds)) { - $selectedTenantIds = array_values(array_intersect($selectedTenantIds, $allowedTenantIds)); +if (!$canManageAllTenants && count($tenantScope['ids']) > 0) { + $selectedTenantIds = array_values(array_intersect($selectedTenantIds, $tenantScope['ids'])); } $primaryTenantId = (int) ($user['primary_tenant_id'] ?? 0); if (!$primaryTenantId && count($selectedTenantIds) === 1) { @@ -187,22 +209,22 @@ if ($request->isMethod('POST')) { $existingTenantIds = $userTenantRepository->listTenantIdsByUserId($userId); $tenantIdsForSync = $tenantIds; - if ($canEditAssignments && is_array($allowedTenantIds)) { - $tenantIds = array_values(array_intersect($tenantIds, $allowedTenantIds)); + if ($canEditAssignments && !$canManageAllTenants && count($tenantScope['ids']) > 0) { + $tenantIds = array_values(array_intersect($tenantIds, $tenantScope['ids'])); $tenantIdsForSync = $scopeGateway->mergeTenantIdsPreservingOutOfScope( $tenantIds, $existingTenantIds, - $allowedTenantIds + $tenantScope['ids'] ); - } elseif ($canEditAssignments && !$canManageTenants && $strictTenantScope) { + } elseif ($canEditAssignments && !$canManageAllTenants && $strictTenantScope) { $tenantIds = []; $tenantIdsForSync = $existingTenantIds; } if (!$canEditAssignments) { $tenantIds = $userTenantRepository->listTenantIdsByUserId($userId); - if (is_array($allowedTenantIds)) { - $tenantIds = array_values(array_intersect($tenantIds, $allowedTenantIds)); + if (!$canManageAllTenants && count($tenantScope['ids']) > 0) { + $tenantIds = array_values(array_intersect($tenantIds, $tenantScope['ids'])); } $tenantIdsForSync = $tenantIds; } @@ -285,10 +307,10 @@ if ($request->isMethod('POST')) { if (!$errorBag->hasAny()) { $action = (string) ($post['action'] ?? 'save'); if ($action === 'save_close') { - Flash::success('User updated', $closeTarget, 'user_updated'); + Flash::success(t('User updated'), $closeTarget, 'user_updated'); Router::redirect($closeTarget); } else { - Flash::success('User updated', $editTarget, 'user_updated'); + Flash::success(t('User updated'), $editTarget, 'user_updated'); Router::redirect($editTarget); } } diff --git a/tests/Architecture/DetailDrawerFragmentContractTest.php b/tests/Architecture/DetailDrawerFragmentContractTest.php index a7aac77..9b32a33 100644 --- a/tests/Architecture/DetailDrawerFragmentContractTest.php +++ b/tests/Architecture/DetailDrawerFragmentContractTest.php @@ -191,6 +191,11 @@ class DetailDrawerFragmentContractTest extends TestCase * Extract the first top-level (non-nested) authorize / requireAbility ability * argument from a PHP source string. Returns null and records a violation * when no statically extractable call is found. + * + * Also recognizes the actionEditContext / actionCreateContext / actionFragmentContext + * aggregator pattern: if the file calls one of those aggregators at top level, + * we look for an 'abilityKey' => SomePolicy::CONST entry inside the args array + * and treat it as the top-level ability. */ private function extractTopLevelAbility(string $source, string $fileRel, array &$violations): ?string { @@ -203,6 +208,7 @@ class DetailDrawerFragmentContractTest extends TestCase $hasTopLevelMatch = false; $hasNestedMatch = false; $extracted = null; + $insideTopLevelAggregator = false; foreach ($lines as $line) { // Update brace depth AFTER matching this line so a `{` on the same @@ -218,6 +224,18 @@ class DetailDrawerFragmentContractTest extends TestCase $hasNestedMatch = true; } } + // actionEditContext / actionCreateContext / actionFragmentContext aggregator pattern. + // The aggregator args use a [ ... ] array literal (not a { ... } block), + // so brace-depth tracking does not bracket them; we track the array + // opening with a separate "inside aggregator args" sticky flag instead. + if (!$insideTopLevelAggregator && $depth === 0 && preg_match('/actionEditContext\s*\(|actionCreateContext\s*\(|actionFragmentContext\s*\(/', $line)) { + $insideTopLevelAggregator = true; + } + if ($insideTopLevelAggregator && !$hasTopLevelMatch && preg_match('/[\'"]abilityKey[\'"]\s*=>\s*([A-Za-z_\\\\][A-Za-z0-9_\\\\:]*)/', $line, $am)) { + $extracted = $am[1]; + $hasTopLevelMatch = true; + $insideTopLevelAggregator = false; + } $opens = substr_count($line, '{'); $closes = substr_count($line, '}'); $depth += $opens - $closes;