diff --git a/lib/Service/User/UserAccountService.php b/lib/Service/User/UserAccountService.php index baec91b..ec8fdee 100644 --- a/lib/Service/User/UserAccountService.php +++ b/lib/Service/User/UserAccountService.php @@ -221,9 +221,14 @@ class UserAccountService $createdUser = $this->userReadRepository->findByEmail($form['email']); $uuid = $createdUser['uuid'] ?? null; $userId = (int) ($createdUser['id'] ?? 0); + if ($userId <= 0) { + $this->rollbackQuietly(); + return ['ok' => false, 'errors' => [t('User can not be registered')], 'form' => $form]; + } - if ($userId > 0 && $tenantIds) { - $this->userAssignmentService->syncTenants($userId, $tenantIds); + if ($tenantIds && !$this->userAssignmentService->syncTenants($userId, $tenantIds)) { + $this->rollbackQuietly(); + return ['ok' => false, 'errors' => [t('User can not be registered')], 'form' => $form]; } $roleIds = $this->userAssignmentService->normalizeIdInput($input['role_ids'] ?? []); @@ -233,8 +238,9 @@ class UserAccountService $roleIds = [$defaultRoleId]; } } - if ($userId > 0 && $roleIds) { - $this->userAssignmentService->syncRoles($userId, $roleIds); + if ($roleIds && !$this->userAssignmentService->syncRoles($userId, $roleIds)) { + $this->rollbackQuietly(); + return ['ok' => false, 'errors' => [t('User can not be registered')], 'form' => $form]; } $departmentIds = $this->userAssignmentService->normalizeIdInput($input['department_ids'] ?? []); @@ -244,8 +250,9 @@ class UserAccountService $departmentIds = [$defaultDepartmentId]; } } - if ($userId > 0 && $departmentIds) { - $this->userAssignmentService->syncDepartments($userId, $departmentIds); + if ($departmentIds && !$this->userAssignmentService->syncDepartments($userId, $departmentIds)) { + $this->rollbackQuietly(); + return ['ok' => false, 'errors' => [t('User can not be registered')], 'form' => $form]; } $this->databaseSessionRepository->commitTransaction(); @@ -260,7 +267,7 @@ class UserAccountService $this->systemAuditService->record('admin.users.create', 'success', [ 'actor_user_id' => $currentUserId > 0 ? $currentUserId : null, 'target_type' => 'user', - 'target_id' => $userId > 0 ? $userId : null, + 'target_id' => $userId, 'target_uuid' => is_string($uuid) ? $uuid : '', 'metadata' => [ 'assigned_tenant_ids' => $tenantIds, @@ -522,18 +529,24 @@ class UserAccountService $createdUser = $this->userReadRepository->findByEmail($form['email']); $userId = (int) ($createdUser['id'] ?? 0); - if ($userId > 0) { - if ($defaultTenantId) { - $this->userAssignmentService->syncTenants($userId, [$defaultTenantId]); - } - $defaultRoleId = $this->settingsGateway->getDefaultRoleId(); - if ($defaultRoleId) { - $this->userAssignmentService->syncRoles($userId, [$defaultRoleId]); - } - $defaultDepartmentId = $this->settingsGateway->getDefaultDepartmentId(); - if ($defaultDepartmentId) { - $this->userAssignmentService->syncDepartments($userId, [$defaultDepartmentId]); - } + if ($userId <= 0) { + $this->rollbackQuietly(); + return ['ok' => false, 'error' => t('User can not be registered')]; + } + + if ($defaultTenantId && !$this->userAssignmentService->syncTenants($userId, [$defaultTenantId])) { + $this->rollbackQuietly(); + return ['ok' => false, 'error' => t('User can not be registered')]; + } + $defaultRoleId = $this->settingsGateway->getDefaultRoleId(); + if ($defaultRoleId && !$this->userAssignmentService->syncRoles($userId, [$defaultRoleId])) { + $this->rollbackQuietly(); + return ['ok' => false, 'error' => t('User can not be registered')]; + } + $defaultDepartmentId = $this->settingsGateway->getDefaultDepartmentId(); + if ($defaultDepartmentId && !$this->userAssignmentService->syncDepartments($userId, [$defaultDepartmentId])) { + $this->rollbackQuietly(); + return ['ok' => false, 'error' => t('User can not be registered')]; } $this->databaseSessionRepository->commitTransaction(); diff --git a/pages/admin/permissions/edit($id).php b/pages/admin/permissions/edit($id).php index 8585349..8a5f0c2 100644 --- a/pages/admin/permissions/edit($id).php +++ b/pages/admin/permissions/edit($id).php @@ -68,6 +68,7 @@ if ($request->hasBody('key')) { $roleIds = [$roleIds]; } $roleIds = array_values(array_unique(array_map('intval', $roleIds))); + $syncSucceeded = false; $dbSession = app(\MintyPHP\Repository\Support\DatabaseSessionRepository::class); $dbSession->beginTransaction(); try { @@ -78,6 +79,7 @@ if ($request->hasBody('key')) { $userWriteRepository->bumpAuthzVersionByUserIds($affectedUserIds); } $dbSession->commitTransaction(); + $syncSucceeded = true; } catch (\Throwable) { try { $dbSession->rollbackTransaction(); @@ -85,13 +87,15 @@ if ($request->hasBody('key')) { } $errorBag->merge([t('Failed to update permission roles')]); } - $action = (string) $request->body('action', 'save'); - if ($action === 'save_close') { - Flash::success('Permission updated', 'admin/permissions', 'permission_updated'); - Router::redirect('admin/permissions'); - } else { - Flash::success('Permission updated', "admin/permissions/edit/{$id}", 'permission_updated'); - Router::redirect("admin/permissions/edit/{$id}"); + if ($syncSucceeded) { + $action = (string) $request->body('action', 'save'); + if ($action === 'save_close') { + Flash::success('Permission updated', 'admin/permissions', 'permission_updated'); + Router::redirect('admin/permissions'); + } else { + Flash::success('Permission updated', "admin/permissions/edit/{$id}", 'permission_updated'); + Router::redirect("admin/permissions/edit/{$id}"); + } } } } diff --git a/pages/admin/roles/create().php b/pages/admin/roles/create().php index ed7f0f8..6309d4c 100644 --- a/pages/admin/roles/create().php +++ b/pages/admin/roles/create().php @@ -33,50 +33,62 @@ $permissions = $permissionRepository->listActive(); $selectedPermissionIds = []; if ($request->hasBody('description')) { - $result = app(\MintyPHP\Service\Access\RoleService::class)->createFromAdmin($request->bodyAll(), $currentUserId); - $form = $result['form'] ?? $form; - $errorBag->merge($result['errors'] ?? []); - $selectedPermissionIds = $request->body('permission_ids', $selectedPermissionIds); - if (!is_array($selectedPermissionIds)) { - $selectedPermissionIds = [$selectedPermissionIds]; - } - $selectedPermissionIds = array_values(array_unique(array_map('intval', $selectedPermissionIds))); + $dbSession = app(\MintyPHP\Repository\Support\DatabaseSessionRepository::class); + $transactionStarted = false; - if (($result['ok'] ?? false) && !$errorBag->hasAny()) { - $permissionIds = $request->body('permission_ids', []); - if (!is_array($permissionIds)) { - $permissionIds = [$permissionIds]; + try { + $dbSession->beginTransaction(); + $transactionStarted = true; + + $result = app(\MintyPHP\Service\Access\RoleService::class)->createFromAdmin($request->bodyAll(), $currentUserId); + $form = $result['form'] ?? $form; + $errorBag->merge($result['errors'] ?? []); + $selectedPermissionIds = $request->body('permission_ids', $selectedPermissionIds); + if (!is_array($selectedPermissionIds)) { + $selectedPermissionIds = [$selectedPermissionIds]; } - $permissionIds = array_values(array_unique(array_map('intval', $permissionIds))); - $createdId = (int) ($result['id'] ?? 0); - if ($createdId > 0) { - $dbSession = app(\MintyPHP\Repository\Support\DatabaseSessionRepository::class); - $dbSession->beginTransaction(); - try { - $rolePermissionRepository->replaceForRole($createdId, $permissionIds); - $dbSession->commitTransaction(); - } catch (\Throwable) { - try { - $dbSession->rollbackTransaction(); - } catch (\Throwable) { + $selectedPermissionIds = array_values(array_unique(array_map('intval', $selectedPermissionIds))); + + if (($result['ok'] ?? false) && !$errorBag->hasAny()) { + $permissionIds = $request->body('permission_ids', []); + if (!is_array($permissionIds)) { + $permissionIds = [$permissionIds]; + } + $permissionIds = array_values(array_unique(array_map('intval', $permissionIds))); + $createdId = (int) ($result['id'] ?? 0); + if ($createdId <= 0) { + throw new \RuntimeException('role_create_missing_id'); + } + $rolePermissionRepository->replaceForRole($createdId, $permissionIds); + $dbSession->commitTransaction(); + $transactionStarted = false; + + $action = (string) $request->body('action', 'create'); + if ($action === 'create_close') { + Flash::success('Role created', 'admin/roles', 'role_created'); + Router::redirect('admin/roles'); + } else { + $uuid = (string) ($result['uuid'] ?? ''); + if ($uuid !== '') { + $target = "admin/roles/edit/{$uuid}"; + Flash::success('Role created', $target, 'role_created'); + Router::redirect($target); + } else { + Flash::success('Role created', 'admin/roles', 'role_created'); + Router::redirect('admin/roles'); } } } - $action = (string) $request->body('action', 'create'); - if ($action === 'create_close') { - Flash::success('Role created', 'admin/roles', 'role_created'); - Router::redirect('admin/roles'); - } else { - $uuid = (string) ($result['uuid'] ?? ''); - if ($uuid !== '') { - $target = "admin/roles/edit/{$uuid}"; - Flash::success('Role created', $target, 'role_created'); - Router::redirect($target); - } else { - Flash::success('Role created', 'admin/roles', 'role_created'); - Router::redirect('admin/roles'); + } catch (\Throwable) { + if ($transactionStarted) { + try { + $dbSession->rollbackTransaction(); + } catch (\Throwable) { } } + if (!$errorBag->hasAny()) { + $errorBag->merge([t('Failed to update role permissions')]); + } } } diff --git a/pages/admin/roles/edit($id).php b/pages/admin/roles/edit($id).php index 8332337..b6f3ea7 100644 --- a/pages/admin/roles/edit($id).php +++ b/pages/admin/roles/edit($id).php @@ -86,6 +86,7 @@ if ($request->hasBody('description')) { $permissionIds = [$permissionIds]; } $permissionIds = array_values(array_unique(array_map('intval', $permissionIds))); + $syncSucceeded = false; $dbSession = app(\MintyPHP\Repository\Support\DatabaseSessionRepository::class); $dbSession->beginTransaction(); try { @@ -95,6 +96,7 @@ if ($request->hasBody('description')) { $userWriteRepository->bumpAuthzVersionByUserIds($affectedUserIds); } $dbSession->commitTransaction(); + $syncSucceeded = true; } catch (\Throwable) { try { $dbSession->rollbackTransaction(); @@ -102,13 +104,15 @@ if ($request->hasBody('description')) { } $errorBag->merge([t('Failed to update role permissions')]); } - $action = (string) $request->body('action', 'save'); - if ($action === 'save_close') { - Flash::success('Role updated', 'admin/roles', 'role_updated'); - Router::redirect('admin/roles'); - } else { - Flash::success('Role updated', "admin/roles/edit/{$uuid}", 'role_updated'); - Router::redirect("admin/roles/edit/{$uuid}"); + if ($syncSucceeded) { + $action = (string) $request->body('action', 'save'); + if ($action === 'save_close') { + Flash::success('Role updated', 'admin/roles', 'role_updated'); + Router::redirect('admin/roles'); + } else { + Flash::success('Role updated', "admin/roles/edit/{$uuid}", 'role_updated'); + Router::redirect("admin/roles/edit/{$uuid}"); + } } } } diff --git a/tests/Service/User/UserAccountServiceTest.php b/tests/Service/User/UserAccountServiceTest.php index 5700557..a687f72 100644 --- a/tests/Service/User/UserAccountServiceTest.php +++ b/tests/Service/User/UserAccountServiceTest.php @@ -260,9 +260,9 @@ class UserAccountServiceTest extends TestCase ); $assignmentService = $this->createMock(UserAssignmentService::class); - $assignmentService->expects($this->once())->method('syncTenants')->with(77, [3]); - $assignmentService->expects($this->once())->method('syncRoles')->with(77, [4]); - $assignmentService->expects($this->once())->method('syncDepartments')->with(77, [5]); + $assignmentService->expects($this->once())->method('syncTenants')->with(77, [3])->willReturn(true); + $assignmentService->expects($this->once())->method('syncRoles')->with(77, [4])->willReturn(true); + $assignmentService->expects($this->once())->method('syncDepartments')->with(77, [5])->willReturn(true); $service = $this->newService( $readRepository, @@ -284,6 +284,128 @@ class UserAccountServiceTest extends TestCase $this->assertTrue($result['ok']); } + public function testCreateFromAdminRollsBackTransactionWhenTenantSyncFails(): void + { + $passwordService = $this->createMock(UserPasswordService::class); + $passwordService->method('validatePassword')->willReturn([]); + + $settingsGateway = $this->createMock(UserSettingsGateway::class); + $settingsGateway->method('getDefaultTenantId')->willReturn(null); + $settingsGateway->method('getDefaultRoleId')->willReturn(null); + $settingsGateway->method('getDefaultDepartmentId')->willReturn(null); + $settingsGateway->method('getAppTheme')->willReturn('light'); + + $writeRepository = $this->createMock(UserWriteRepositoryInterface::class); + $writeRepository->expects($this->once())->method('create')->willReturn(88); + + $readRepository = $this->createMock(UserReadRepositoryInterface::class); + $readRepository->expects($this->exactly(2)) + ->method('findByEmail') + ->with('create@example.com') + ->willReturnOnConsecutiveCalls( + null, + ['id' => 88, 'uuid' => 'user-uuid-88'] + ); + + $assignmentService = $this->createMock(UserAssignmentService::class); + $assignmentService->expects($this->once())->method('normalizeTenantIds')->with([3])->willReturn([3]); + $assignmentService->expects($this->once())->method('syncTenants')->with(88, [3])->willReturn(false); + $assignmentService->expects($this->never())->method('syncRoles'); + $assignmentService->expects($this->never())->method('syncDepartments'); + + $auditService = $this->createMock(SystemAuditService::class); + $auditService->expects($this->never())->method('record'); + + $databaseSessionRepository = $this->createMock(DatabaseSessionRepository::class); + $databaseSessionRepository->expects($this->once())->method('beginTransaction'); + $databaseSessionRepository->expects($this->never())->method('commitTransaction'); + $databaseSessionRepository->expects($this->once())->method('rollbackTransaction'); + + $service = $this->newService( + $readRepository, + $writeRepository, + null, + $assignmentService, + $passwordService, + $settingsGateway, + null, + null, + $auditService, + $databaseSessionRepository + ); + + $result = $service->createFromAdmin([ + 'first_name' => 'Max', + 'last_name' => 'Mustermann', + 'email' => 'create@example.com', + 'password' => 'StrongPass1!', + 'password2' => 'StrongPass1!', + 'tenant_ids' => [3], + 'active' => 1, + ], 42); + + $this->assertFalse($result['ok']); + $this->assertNotEmpty($result['errors'] ?? []); + } + + public function testRegisterRollsBackTransactionWhenRoleSyncFails(): void + { + $passwordService = $this->createMock(UserPasswordService::class); + $passwordService->method('validatePassword')->willReturn([]); + + $settingsGateway = $this->createMock(UserSettingsGateway::class); + $settingsGateway->method('getAppTheme')->willReturn('dark'); + $settingsGateway->method('getDefaultTenantId')->willReturn(3); + $settingsGateway->method('getDefaultRoleId')->willReturn(4); + $settingsGateway->method('getDefaultDepartmentId')->willReturn(5); + + $writeRepository = $this->createMock(UserWriteRepositoryInterface::class); + $writeRepository->expects($this->once())->method('create')->willReturn(77); + + $readRepository = $this->createMock(UserReadRepositoryInterface::class); + $readRepository->expects($this->exactly(2)) + ->method('findByEmail') + ->with('user@example.com') + ->willReturnOnConsecutiveCalls( + null, + ['id' => 77, 'uuid' => 'user-uuid'] + ); + + $assignmentService = $this->createMock(UserAssignmentService::class); + $assignmentService->expects($this->once())->method('syncTenants')->with(77, [3])->willReturn(true); + $assignmentService->expects($this->once())->method('syncRoles')->with(77, [4])->willReturn(false); + $assignmentService->expects($this->never())->method('syncDepartments'); + + $databaseSessionRepository = $this->createMock(DatabaseSessionRepository::class); + $databaseSessionRepository->expects($this->once())->method('beginTransaction'); + $databaseSessionRepository->expects($this->never())->method('commitTransaction'); + $databaseSessionRepository->expects($this->once())->method('rollbackTransaction'); + + $service = $this->newService( + $readRepository, + $writeRepository, + null, + $assignmentService, + $passwordService, + $settingsGateway, + null, + null, + null, + $databaseSessionRepository + ); + + $result = $service->register([ + 'first_name' => 'Max', + 'last_name' => 'Mustermann', + 'email' => 'user@example.com', + 'password' => 'StrongPass1!', + 'password2' => 'StrongPass1!', + ]); + + $this->assertFalse($result['ok']); + $this->assertNotSame('', trim((string) ($result['error'] ?? ''))); + } + public function testUpdateSelfProfileReturnsErrorWhenRequestedTenantIsNotAssigned(): void { $readRepository = $this->createMock(UserReadRepositoryInterface::class);