From 4b9ce4bbadad36486fc56209aa1ca7b359da19d3 Mon Sep 17 00:00:00 2001 From: fs Date: Fri, 24 Apr 2026 14:46:38 +0200 Subject: [PATCH] Consolidate audit permissions to audit namespace --- db/init/init.sql | 28 ++--- ...-24-audit-permission-key-consolidation.sql | 71 ++++++++++++ i18n/default_de.json | 8 ++ i18n/default_en.json | 8 ++ modules/audit/i18n/default_de.json | 8 ++ modules/audit/i18n/default_en.json | 8 ++ .../Module/Audit/AuditAuthorizationPolicy.php | 12 +- .../Audit/AuditAuthorizationPolicyTest.php | 103 ++++++++++++++++++ 8 files changed, 227 insertions(+), 19 deletions(-) create mode 100644 db/updates/2026-04-24-audit-permission-key-consolidation.sql create mode 100644 modules/audit/tests/Module/Audit/AuditAuthorizationPolicyTest.php diff --git a/db/init/init.sql b/db/init/init.sql index 588cc79..7f84e85 100644 --- a/db/init/init.sql +++ b/db/init/init.sql @@ -1429,22 +1429,24 @@ VALUES ('settings.update', 'Can update settings', 1, 1), ('tenant.scope.global', 'Can bypass tenant scope globally', 1, 1), ('imports.view', 'Can view imports', 1, 1), - ('imports.audit.view', 'Can view import audit logs', 1, 1), + ('audit.imports.view', 'Can view import audit logs', 1, 1), + ('audit.imports.purge', 'Can purge import audit logs', 1, 1), ('jobs.view', 'Can view scheduled jobs', 1, 1), ('jobs.manage', 'Can manage scheduled jobs', 1, 1), ('jobs.run_now', 'Can run scheduled jobs manually', 1, 1), - ('user_lifecycle_audit.view', 'Can view user lifecycle audit logs', 1, 1), + ('audit.user_lifecycle.view', 'Can view user lifecycle audit logs', 1, 1), ('users.import', 'Can import users', 1, 1), ('users.import_assignments', 'Can assign tenant, role and department during user import', 1, 1), - ('users.lifecycle_restore', 'Can restore users from lifecycle audit', 1, 1), + ('audit.user_lifecycle.restore', 'Can restore users from lifecycle audit', 1, 1), ('custom_fields.manage', 'Can manage tenant custom field definitions', 1, 1), ('custom_fields.edit_values', 'Can edit user custom field values', 1, 1), ('api_docs.view', 'Can view API documentation', 1, 1), ('docs.view', 'Can view developer documentation', 1, 1), ('mail_log.view', 'Can view mail logs', 1, 1), - ('api_audit.view', 'Can view API audit logs', 1, 1), - ('system_audit.view', 'Can view system audit logs', 1, 1), - ('system_audit.purge', 'Can purge system audit logs', 1, 1), + ('audit.api.view', 'Can view API audit logs', 1, 1), + ('audit.api.purge', 'Can purge API audit logs', 1, 1), + ('audit.system.view', 'Can view system audit logs', 1, 1), + ('audit.system.purge', 'Can purge system audit logs', 1, 1), ('api_tokens.manage', 'Can manage user API tokens', 1, 1), ('stats.view', 'Can view statistics', 1, 1), ('system_info.view', 'Can view system info and health status', 1, 1), @@ -1542,13 +1544,13 @@ JOIN permissions p ON p.`key` IN ( 'roles.view', 'roles.create', 'roles.update', 'roles.delete', 'permissions.view', 'permissions.create', 'permissions.update', 'permissions.delete', 'settings.view', 'settings.update', 'tenant.scope.global', - 'imports.view', 'imports.audit.view', 'jobs.view', 'jobs.manage', 'jobs.run_now', - 'user_lifecycle_audit.view', 'users.import', 'users.import_assignments', - 'users.lifecycle_restore', + 'imports.view', 'audit.imports.view', 'audit.imports.purge', 'jobs.view', 'jobs.manage', 'jobs.run_now', + 'audit.user_lifecycle.view', 'users.import', 'users.import_assignments', + 'audit.user_lifecycle.restore', 'custom_fields.manage', 'custom_fields.edit_values', 'api_docs.view', 'docs.view', 'api_tokens.manage', - 'mail_log.view', 'api_audit.view', 'system_audit.view', 'system_audit.purge', + 'mail_log.view', 'audit.api.view', 'audit.api.purge', 'audit.system.view', 'audit.system.purge', 'stats.view', 'system_info.view', 'roles.assign_all', 'helpdesk.access', 'helpdesk.settings.manage', 'helpdesk.team-workload.view', 'helpdesk.risk-radar.view', 'helpdesk.software-products.manage', @@ -1598,9 +1600,9 @@ JOIN permissions p ON p.`key` IN ( 'address_book.view', 'tenants.view', 'departments.view', 'roles.view', 'permissions.view', 'settings.view', - 'imports.view', 'imports.audit.view', - 'user_lifecycle_audit.view', - 'mail_log.view', 'api_audit.view', 'system_audit.view', + 'imports.view', 'audit.imports.view', + 'audit.user_lifecycle.view', + 'mail_log.view', 'audit.api.view', 'audit.system.view', 'stats.view', 'jobs.view' ) WHERE r.code = 'AUDITOR' diff --git a/db/updates/2026-04-24-audit-permission-key-consolidation.sql b/db/updates/2026-04-24-audit-permission-key-consolidation.sql new file mode 100644 index 0000000..f406b92 --- /dev/null +++ b/db/updates/2026-04-24-audit-permission-key-consolidation.sql @@ -0,0 +1,71 @@ +-- Idempotent update: consolidate legacy audit permission keys to audit.* namespace. + +INSERT INTO `permissions` (`key`, `description`, `active`, `is_system`) +VALUES + ('audit.api.view', 'Can view API audit logs', 1, 1), + ('audit.api.purge', 'Can purge API audit logs', 1, 1), + ('audit.system.view', 'Can view system audit logs', 1, 1), + ('audit.system.purge', 'Can purge system audit logs', 1, 1), + ('audit.imports.view', 'Can view import audit logs', 1, 1), + ('audit.imports.purge', 'Can purge import audit logs', 1, 1), + ('audit.user_lifecycle.view', 'Can view user lifecycle audit logs', 1, 1), + ('audit.user_lifecycle.restore', 'Can restore users from lifecycle audit', 1, 1) +ON DUPLICATE KEY UPDATE + `description` = VALUES(`description`), + `active` = VALUES(`active`), + `is_system` = VALUES(`is_system`); + +-- Preserve existing role grants by copying role_permissions from legacy keys. +INSERT INTO `role_permissions` (`role_id`, `permission_id`, `created`) +SELECT DISTINCT rp.`role_id`, p_new.`id`, NOW() +FROM `role_permissions` rp +JOIN `permissions` p_old ON p_old.`id` = rp.`permission_id` +JOIN `permissions` p_new ON p_new.`key` = ( + CASE p_old.`key` + WHEN 'api_audit.view' THEN 'audit.api.view' + WHEN 'system_audit.view' THEN 'audit.system.view' + WHEN 'system_audit.purge' THEN 'audit.system.purge' + WHEN 'imports.audit.view' THEN 'audit.imports.view' + WHEN 'user_lifecycle_audit.view' THEN 'audit.user_lifecycle.view' + WHEN 'users.lifecycle_restore' THEN 'audit.user_lifecycle.restore' + ELSE NULL + END +) +WHERE p_old.`key` IN ( + 'api_audit.view', + 'system_audit.view', + 'system_audit.purge', + 'imports.audit.view', + 'user_lifecycle_audit.view', + 'users.lifecycle_restore' +) +ON DUPLICATE KEY UPDATE + `role_id` = `role_id`; + +-- Keep admin baseline complete for purge/restore actions. +INSERT INTO `role_permissions` (`role_id`, `permission_id`, `created`) +SELECT r.`id`, p.`id`, NOW() +FROM `roles` r +JOIN `permissions` p ON p.`key` IN ( + 'audit.api.purge', + 'audit.imports.purge', + 'audit.system.purge', + 'audit.user_lifecycle.restore' +) +WHERE r.`description` IN ('Admin', 'Administrator') OR r.`id` = 1 +ON DUPLICATE KEY UPDATE + `role_id` = `role_id`; + +-- Legacy keys are retained for history but deactivated to avoid future drift. +UPDATE `permissions` +SET + `active` = 0, + `is_system` = 1 +WHERE `key` IN ( + 'api_audit.view', + 'system_audit.view', + 'system_audit.purge', + 'imports.audit.view', + 'user_lifecycle_audit.view', + 'users.lifecycle_restore' +); diff --git a/i18n/default_de.json b/i18n/default_de.json index f701780..f48b3f4 100644 --- a/i18n/default_de.json +++ b/i18n/default_de.json @@ -184,6 +184,14 @@ "perm.jobs.view": "Geplante Aufgaben anzeigen", "perm.jobs.manage": "Geplante Aufgaben verwalten", "perm.jobs.run_now": "Geplante Aufgaben manuell ausführen", + "perm.audit.api.view": "API-Protokolle anzeigen", + "perm.audit.api.purge": "API-Protokolle bereinigen", + "perm.audit.system.view": "System-Protokolle anzeigen", + "perm.audit.system.purge": "System-Protokolle bereinigen", + "perm.audit.imports.view": "Import-Protokolle anzeigen", + "perm.audit.imports.purge": "Import-Protokolle bereinigen", + "perm.audit.user_lifecycle.view": "Benutzer-Lifecycle-Protokolle anzeigen", + "perm.audit.user_lifecycle.restore": "Benutzer aus Lifecycle-Protokoll wiederherstellen", "perm.user_lifecycle_audit.view": "Benutzer-Lifecycle-Protokolle anzeigen", "perm.users.lifecycle_restore": "Benutzer aus Lifecycle-Protokoll wiederherstellen", "Select all": "Alle auswählen", diff --git a/i18n/default_en.json b/i18n/default_en.json index 488575f..3f79d53 100644 --- a/i18n/default_en.json +++ b/i18n/default_en.json @@ -184,6 +184,14 @@ "perm.jobs.view": "View scheduled jobs", "perm.jobs.manage": "Manage scheduled jobs", "perm.jobs.run_now": "Run scheduled jobs manually", + "perm.audit.api.view": "View API logs", + "perm.audit.api.purge": "Purge API logs", + "perm.audit.system.view": "View system logs", + "perm.audit.system.purge": "Purge system logs", + "perm.audit.imports.view": "View import audit logs", + "perm.audit.imports.purge": "Purge import audit logs", + "perm.audit.user_lifecycle.view": "View user lifecycle logs", + "perm.audit.user_lifecycle.restore": "Restore users from lifecycle audit", "perm.user_lifecycle_audit.view": "View user lifecycle logs", "perm.users.lifecycle_restore": "Restore users from lifecycle audit", "Select all": "Select all", diff --git a/modules/audit/i18n/default_de.json b/modules/audit/i18n/default_de.json index 778a710..c6429ed 100644 --- a/modules/audit/i18n/default_de.json +++ b/modules/audit/i18n/default_de.json @@ -1,5 +1,13 @@ { "Telemetry helps detect recurring UI issues and failed requests in production.": "Telemetry hilft, wiederkehrende UI-Probleme und fehlgeschlagene Requests in Produktion zu erkennen.", + "perm.audit.api.view": "API-Protokolle anzeigen", + "perm.audit.api.purge": "API-Protokolle bereinigen", + "perm.audit.system.view": "System-Protokolle anzeigen", + "perm.audit.system.purge": "System-Protokolle bereinigen", + "perm.audit.imports.view": "Import-Protokolle anzeigen", + "perm.audit.imports.purge": "Import-Protokolle bereinigen", + "perm.audit.user_lifecycle.view": "Benutzer-Lifecycle-Protokolle anzeigen", + "perm.audit.user_lifecycle.restore": "Benutzer aus Lifecycle-Protokoll wiederherstellen", "perm.user_lifecycle_audit.view": "Benutzer-Lifecycle-Protokolle anzeigen", "perm.users.lifecycle_restore": "Benutzer aus Lifecycle-Protokoll wiederherstellen", "User lifecycle": "Benutzer-Lebenszyklus", diff --git a/modules/audit/i18n/default_en.json b/modules/audit/i18n/default_en.json index 466b665..1d89d65 100644 --- a/modules/audit/i18n/default_en.json +++ b/modules/audit/i18n/default_en.json @@ -1,5 +1,13 @@ { "Telemetry helps detect recurring UI issues and failed requests in production.": "Telemetry helps detect recurring UI issues and failed requests in production.", + "perm.audit.api.view": "View API logs", + "perm.audit.api.purge": "Purge API logs", + "perm.audit.system.view": "View system logs", + "perm.audit.system.purge": "Purge system logs", + "perm.audit.imports.view": "View import audit logs", + "perm.audit.imports.purge": "Purge import audit logs", + "perm.audit.user_lifecycle.view": "View user lifecycle logs", + "perm.audit.user_lifecycle.restore": "Restore users from lifecycle audit", "perm.user_lifecycle_audit.view": "View user lifecycle logs", "perm.users.lifecycle_restore": "Restore users from lifecycle audit", "User lifecycle": "User lifecycle", diff --git a/modules/audit/lib/Module/Audit/AuditAuthorizationPolicy.php b/modules/audit/lib/Module/Audit/AuditAuthorizationPolicy.php index c99da49..cc89cc0 100644 --- a/modules/audit/lib/Module/Audit/AuditAuthorizationPolicy.php +++ b/modules/audit/lib/Module/Audit/AuditAuthorizationPolicy.php @@ -18,14 +18,14 @@ final class AuditAuthorizationPolicy implements AuthorizationPolicyInterface public const ABILITY_USER_LIFECYCLE_RESTORE = 'audit.user_lifecycle.restore'; private const ABILITY_PERMISSION_MAP = [ - self::ABILITY_API_AUDIT_VIEW => 'api_audit.view', + self::ABILITY_API_AUDIT_VIEW => 'audit.api.view', self::ABILITY_API_AUDIT_PURGE => 'audit.api.purge', - self::ABILITY_SYSTEM_AUDIT_VIEW => 'system_audit.view', - self::ABILITY_SYSTEM_AUDIT_PURGE => 'system_audit.purge', - self::ABILITY_IMPORTS_AUDIT_VIEW => 'imports.audit.view', + self::ABILITY_SYSTEM_AUDIT_VIEW => 'audit.system.view', + self::ABILITY_SYSTEM_AUDIT_PURGE => 'audit.system.purge', + self::ABILITY_IMPORTS_AUDIT_VIEW => 'audit.imports.view', self::ABILITY_IMPORTS_AUDIT_PURGE => 'audit.imports.purge', - self::ABILITY_USER_LIFECYCLE_VIEW => 'user_lifecycle_audit.view', - self::ABILITY_USER_LIFECYCLE_RESTORE => 'users.lifecycle_restore', + self::ABILITY_USER_LIFECYCLE_VIEW => 'audit.user_lifecycle.view', + self::ABILITY_USER_LIFECYCLE_RESTORE => 'audit.user_lifecycle.restore', ]; public function __construct( diff --git a/modules/audit/tests/Module/Audit/AuditAuthorizationPolicyTest.php b/modules/audit/tests/Module/Audit/AuditAuthorizationPolicyTest.php new file mode 100644 index 0000000..24c3a23 --- /dev/null +++ b/modules/audit/tests/Module/Audit/AuditAuthorizationPolicyTest.php @@ -0,0 +1,103 @@ +permissionService = $this->createMock(PermissionService::class); + $this->policy = new AuditAuthorizationPolicy($this->permissionService); + } + + public static function abilityPermissionProvider(): array + { + return [ + [AuditAuthorizationPolicy::ABILITY_API_AUDIT_VIEW, 'audit.api.view'], + [AuditAuthorizationPolicy::ABILITY_API_AUDIT_PURGE, 'audit.api.purge'], + [AuditAuthorizationPolicy::ABILITY_SYSTEM_AUDIT_VIEW, 'audit.system.view'], + [AuditAuthorizationPolicy::ABILITY_SYSTEM_AUDIT_PURGE, 'audit.system.purge'], + [AuditAuthorizationPolicy::ABILITY_IMPORTS_AUDIT_VIEW, 'audit.imports.view'], + [AuditAuthorizationPolicy::ABILITY_IMPORTS_AUDIT_PURGE, 'audit.imports.purge'], + [AuditAuthorizationPolicy::ABILITY_USER_LIFECYCLE_VIEW, 'audit.user_lifecycle.view'], + [AuditAuthorizationPolicy::ABILITY_USER_LIFECYCLE_RESTORE, 'audit.user_lifecycle.restore'], + ]; + } + + #[DataProvider('abilityPermissionProvider')] + public function testSupportsAllAuditAbilities(string $ability, string $permission): void + { + self::assertTrue($this->policy->supports($ability)); + self::assertNotSame('', $permission); + } + + public function testSupportsRejectsUnknownAbility(): void + { + self::assertFalse($this->policy->supports('audit.unknown.ability')); + } + + #[DataProvider('abilityPermissionProvider')] + public function testAuthorizeUsesMappedPermissionKey(string $ability, string $permission): void + { + $this->permissionService->expects($this->once()) + ->method('userHas') + ->with(17, $permission) + ->willReturn(true); + + $decision = $this->policy->authorize($ability, ['actor_user_id' => 17]); + + self::assertTrue($decision->isAllowed()); + } + + public function testAuthorizeDeniesMissingActorIdWithoutPermissionLookup(): void + { + $this->permissionService->expects($this->never())->method('userHas'); + + $decision = $this->policy->authorize(AuditAuthorizationPolicy::ABILITY_API_AUDIT_VIEW, []); + + self::assertFalse($decision->isAllowed()); + self::assertSame(403, $decision->status()); + } + + public function testAuthorizeDeniesUnknownAbilityWithoutPermissionLookup(): void + { + $this->permissionService->expects($this->never())->method('userHas'); + + $decision = $this->policy->authorize('audit.unknown.ability', ['actor_user_id' => 17]); + + self::assertFalse($decision->isAllowed()); + self::assertSame(403, $decision->status()); + } + + public function testPolicyPermissionKeysMatchManifestPermissionKeys(): void + { + $manifest = require dirname(__DIR__, 3) . '/module.php'; + $manifestPermissions = is_array($manifest['permissions'] ?? null) ? $manifest['permissions'] : []; + + $manifestKeys = []; + foreach ($manifestPermissions as $permission) { + if (is_array($permission) && isset($permission['key'])) { + $manifestKeys[] = (string) $permission['key']; + } + } + sort($manifestKeys); + + $policyKeys = []; + foreach (self::abilityPermissionProvider() as $pair) { + $policyKeys[] = $pair[1]; + } + $policyKeys = array_values(array_unique($policyKeys)); + sort($policyKeys); + + self::assertSame($manifestKeys, $policyKeys); + } +}