refactor(users-edit): migrate to actionEditContext (step 6)
Cluster-2 pilot — first migration of a standard edit action whose
tenant-scope semantics use null = "manage all" (instead of the
boolean-flag pattern in cluster 1). The CONTEXT-stage vorspiel
collapses into one declarative actionEditContext call; everything
below the vorspiel (conditional audit, custom fields, security
artifacts, two-level submit-authorize, mergeTenantIdsPreservingOutOfScope,
post-save theme/locale/session hooks) stays callsite — domain logic.
Confirms the analyst hypothesis: actionEditContext +
tenantScopeFlagKey:'can_manage_tenants' is enough — no helper
extension. The override key was built in step 1, unit-tested at the
building-block level, and now production-validated.
Two callsite tenant-filter rewrites (GET line 98-105, POST line
190-208) replace is_array($allowedTenantIds) with
$tenantScope['scope']/$tenantScope['ids'] discrimination.
mergeTenantIdsPreservingOutOfScope still receives a list<int> — only
the argument source shifts; the function itself is unchanged.
Three drift decisions reproduced: notFoundFlashScopeKey:'user_not_found',
t() consistency on Flash::success('User updated'), defensive
$canManageAllTenants = $tenantScope['scope'] === 'all'. The legacy
$canManageTenants capability boolean stays alongside (it still gates
strict-mode fallback — both variables now coexist by design).
DetailDrawerFragmentContractTest gets an additive recognizer for
actionEditContext / actionCreateContext / actionFragmentContext
ability-key extraction. Without it the test couldn't see the
aggregator-mediated authorize call in users-edit, so the auth-parity
check against users/view-fragment would regress. Pure addition; the
legacy direct-authorize() regex path is untouched.
ActionContextCsrfPairingContractTest now covers five callers
(departments, tenants, roles, permissions, users) and stays green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -40,26 +40,50 @@ $passwordHints = $userPasswordPolicyService->hints();
|
||||
$currentUserId = (int) ($session['user']['id'] ?? 0);
|
||||
$uuid = trim((string) ($id ?? ''));
|
||||
$editTarget = requestPathWithReturnTarget("admin/users/edit/{$uuid}", $returnTarget);
|
||||
$user = $uuid !== '' ? $userAccountService->findByUuid($uuid) : null;
|
||||
if (!$user) {
|
||||
Flash::error('User not found', $closeTarget, 'user_not_found');
|
||||
Router::redirect($closeTarget);
|
||||
}
|
||||
|
||||
// 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;
|
||||
$userId = (int) ($user['id'] ?? 0);
|
||||
$contextDecision = $authorizationService->authorize(UserAuthorizationPolicy::ABILITY_ADMIN_USERS_EDIT_CONTEXT, [
|
||||
|
||||
$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',
|
||||
]);
|
||||
if (!$contextDecision->isAllowed()) {
|
||||
Router::redirect('error/forbidden');
|
||||
return;
|
||||
}
|
||||
$user = is_array($context['model']) ? $context['model'] : $user;
|
||||
$capabilities = $context['capabilities'];
|
||||
$tenantScope = $context['tenantScope'];
|
||||
$viewAuth = $context['viewAuth'];
|
||||
|
||||
$capabilities = $contextDecision->attribute('capabilities', []);
|
||||
if (!is_array($capabilities)) {
|
||||
$capabilities = [];
|
||||
}
|
||||
$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 {
|
||||
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, $allowedTenantIds, true);
|
||||
return $tenantId > 0 && in_array($tenantId, $tenantScope['ids'], true);
|
||||
}));
|
||||
} elseif (!$canManageTenants && $strictTenantScope) {
|
||||
} 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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user