diff --git a/src/Controller/OrganizationController.php b/src/Controller/OrganizationController.php index 9873ed4..d8f5843 100644 --- a/src/Controller/OrganizationController.php +++ b/src/Controller/OrganizationController.php @@ -98,7 +98,7 @@ class OrganizationController extends AbstractController $this->addFlash('success', 'Organisation crée avec succès.'); return $this->redirectToRoute('organization_index'); } catch (Exception $e) { - $this->addFlash('error', 'Erreur lors de la création de l\'organization'); + $this->addFlash('danger', 'Erreur lors de la création de l\'organization'); $this->loggerService->logError('Error creating organization', ['acting_user_id' => $actingUser->getId(), 'error' => $e->getMessage()]); } } @@ -124,7 +124,7 @@ class OrganizationController extends AbstractController 'org_id' => $id, 'message' => 'Organization not found for edit'], $actingUser->getId() ); - $this->addFlash('error', 'Erreur, l\'organization est introuvable.'); + $this->addFlash('danger', 'Erreur, l\'organization est introuvable.'); return $this->redirectToRoute('organization_index'); } if (!$this->isGranted("ROLE_SUPER_ADMIN")) { @@ -136,7 +136,7 @@ class OrganizationController extends AbstractController 'org_id' => $organization->getId(), 'message' => 'UO link not found for edit organization' ], $actingUser->getId()); - $this->addFlash('error', 'Erreur, accès refusé.'); + $this->addFlash('danger', 'Erreur, accès refusé.'); return $this->redirectToRoute('organization_index'); } $roleAdmin = $this->entityManager->getRepository(Roles::class)->findOneBy(['name' => 'ADMIN']); @@ -147,7 +147,7 @@ class OrganizationController extends AbstractController 'role_id' => $roleAdmin->getId(), 'message' => 'UOA link not found for edit organization, user is not admin of organization' ], $actingUser->getId()); - $this->addFlash('error', 'Erreur, accès refusé.'); + $this->addFlash('danger', 'Erreur, accès refusé.'); return $this->redirectToRoute('organization_index'); } } @@ -169,7 +169,7 @@ class OrganizationController extends AbstractController $this->addFlash('success', 'Organisation modifiée avec succès.'); return $this->redirectToRoute('organization_index'); }catch (Exception $e) { - $this->addFlash('error', 'Erreur lors de la modification de l\'organization'); + $this->addFlash('danger', 'Erreur lors de la modification de l\'organization'); $this->loggerService->logError('Error editing organization', ['acting_user_id' => $actingUser->getId(), 'error' => $e->getMessage()]); } } @@ -190,13 +190,13 @@ class OrganizationController extends AbstractController 'org_id' => $id, 'message' => 'Organization not found for view' ], $actingUser->getId()); - $this->addFlash('error', 'Erreur, l\'organization est introuvable.'); + $this->addFlash('danger', 'Erreur, l\'organization est introuvable.'); return $this->redirectToRoute('organization_index'); } //check if the user is admin of the organization if (!$this->userService->isAdminOfOrganization($organization) && !$this->isGranted("ROLE_ADMIN")) { $this->loggerService->logAccessDenied($actingUser->getId()); - $this->addFlash('error', 'Erreur, accès refusé.'); + $this->addFlash('danger', 'Erreur, accès refusé.'); throw new AccessDeniedHttpException('Access denied'); } @@ -227,7 +227,7 @@ class OrganizationController extends AbstractController 'org_id' => $id, 'message' => 'Organization not found for delete' ], $actingUser->getId()); - $this->addFlash('error', 'Erreur, l\'organization est introuvable.'); + $this->addFlash('danger', 'Erreur, l\'organization est introuvable.'); throw $this->createNotFoundException(self::NOT_FOUND); } try { @@ -247,7 +247,7 @@ class OrganizationController extends AbstractController $this->addFlash('success', 'Organisation supprimée avec succès.'); }catch (\Exception $e){ $this->loggerService->logError($actingUser->getId(), ['message' => 'Error deleting organization: '.$e->getMessage()]); - $this->addFlash('error', 'Erreur lors de la suppression de l\'organization.'); + $this->addFlash('danger', 'Erreur lors de la suppression de l\'organization.'); } return $this->redirectToRoute('organization_index'); @@ -264,7 +264,7 @@ class OrganizationController extends AbstractController 'org_id' => $id, 'message' => 'Organization not found for deactivate' ], $actingUser->getId()); - $this->addFlash('error', 'Erreur, l\'organization est introuvable.'); + $this->addFlash('danger', 'Erreur, l\'organization est introuvable.'); throw $this->createNotFoundException(self::NOT_FOUND); } @@ -288,7 +288,7 @@ class OrganizationController extends AbstractController 'org_id' => $id, 'message' => 'Organization not found for activate' ], $actingUser->getId()); - $this->addFlash('error', 'Erreur, l\'organization est introuvable.'); + $this->addFlash('danger', 'Erreur, l\'organization est introuvable.'); throw $this->createNotFoundException(self::NOT_FOUND); } $organization->setIsActive(true); @@ -324,7 +324,7 @@ class OrganizationController extends AbstractController $qb->andWhere('o.email LIKE :email') ->setParameter('email', '%' . $filters['email'] . '%'); } - if (!$this->isGranted('ROLE_SUPER_ADMIN')) { + if (!$this->isGranted('ROLE_ADMIN')) { $actingUser = $this->userService->getUserByIdentifier($this->getUser()->getUserIdentifier()); $uo = $this->entityManager->getRepository(UsersOrganizations::class)->findBy(['users' => $actingUser]); diff --git a/src/Controller/UserController.php b/src/Controller/UserController.php index f07567c..b360a41 100644 --- a/src/Controller/UserController.php +++ b/src/Controller/UserController.php @@ -61,7 +61,7 @@ class UserController extends AbstractController ) { } - +//TODO: Move mailing/notification logic to event listeners/subscribers for better separation of concerns and to avoid bloating the controller with non-controller logic. Keep in mind the potential for circular dependencies and design accordingly (e.g. using interfaces or decoupled events). #[Route(path: '/', name: 'index', methods: ['GET'])] public function index(): Response { @@ -108,7 +108,7 @@ class UserController extends AbstractController 'acting_user_id' => $actingUser->getId(), 'error' => $e->getMessage(), ]); - $this->addFlash('error', 'Une erreur est survenue lors du chargement des informations utilisateur.'); + $this->addFlash('danger', 'Une erreur est survenue lors du chargement des informations utilisateur.'); $referer = $request->headers->get('referer'); return $this->redirect($referer ?? $this->generateUrl('app_index')); } @@ -126,7 +126,7 @@ class UserController extends AbstractController $user = $this->userRepository->find($id); if (!$user) { $this->loggerService->logEntityNotFound('User', ['id' => $id], $actingUser->getId()); - $this->addFlash('error', "L'utilisateur demandé n'existe pas."); + $this->addFlash('danger', "L'utilisateur demandé n'existe pas."); throw $this->createNotFoundException(self::NOT_FOUND); } try { @@ -162,7 +162,7 @@ class UserController extends AbstractController return $this->redirectToRoute('user_show', ['id' => $user->getId(), 'organizationId' => $orgId]); } $this->loggerService->logEntityNotFound('Organization', ['id' => $orgId], $actingUser->getId()); - $this->addFlash('error', "L'organisation n'existe pas."); + $this->addFlash('danger', "L'organisation n'existe pas."); throw $this->createNotFoundException(self::NOT_FOUND); } if ($this->isGranted('ROLE_SUPER_ADMIN')) { @@ -184,10 +184,10 @@ class UserController extends AbstractController ]); } $this->loggerService->logAccessDenied($actingUser->getId()); - $this->addFlash('error', "Accès non autorisé."); + $this->addFlash('danger', "Accès non autorisé."); throw $this->createAccessDeniedException(self::ACCESS_DENIED); } catch (\Exception $e) { - $this->addFlash('error', 'Une erreur est survenue lors de la modification des informations utilisateur.'); + $this->addFlash('danger', 'Une erreur est survenue lors de la modification des informations utilisateur.'); $this->errorLogger->critical($e->getMessage()); } // Default deny access. shouldn't reach here normally. @@ -198,49 +198,44 @@ class UserController extends AbstractController #[Route('/new', name: 'new', methods: ['GET', 'POST'])] public function new(Request $request): Response { - $this->denyAccessUnlessGranted('ROLE_ADMIN'); + $this->denyAccessUnlessGranted('ROLE_USER'); try { $actingUser = $this->userService->getUserByIdentifier($this->getUser()->getUserIdentifier()); - if (!$this->userService->hasAccessTo($actingUser)) { - $this->loggerService->logAccessDenied($actingUser->getId()); - throw $this->createAccessDeniedException(self::ACCESS_DENIED); - } $user = new User(); $form = $this->createForm(UserForm::class, $user); $form->handleRequest($request); - $orgId = $request->get('organizationId'); + $orgId = $request->query->get('organizationId') ?? $request->request->get('organizationId'); if ($orgId) { $org = $this->organizationRepository->find($orgId); if (!$org) { $this->loggerService->logEntityNotFound('Organization', ['id' => $orgId], $actingUser->getId()); - $this->addFlash('error', "L'organisation n'existe pas."); + $this->addFlash('danger', "L'organisation n'existe pas."); throw $this->createNotFoundException(self::NOT_FOUND); } - if ($this->isGranted('ROLE_ADMIN') && !$this->userService->isAdminOfOrganization($org) && !$this->isGranted('ROLE_SUPER_ADMIN')) { + if (!$this->isGranted('ROLE_ADMIN') && !$this->userService->isAdminOfOrganization($org)) { $this->loggerService->logAccessDenied($actingUser->getId()); - $this->addFlash('error', "Accès non autorisé."); + $this->addFlash('danger', "Accès non autorisé."); throw $this->createAccessDeniedException(self::ACCESS_DENIED); } - } elseif ($this->isGranted('ROLE_ADMIN')) { + } else{ $this->loggerService->logAccessDenied($actingUser->getId()); - $this->addFlash('error', "Accès non autorisé."); + $this->addFlash('danger', "Accès non autorisé."); throw $this->createAccessDeniedException(self::ACCESS_DENIED); } if ($form->isSubmitted() && $form->isValid()) { $existingUser = $this->userRepository->findOneBy(['email' => $user->getEmail()]); - // Case : User exists + has organization context + // Case : User exists -> link him to given organization if not already linked, else error message if ($existingUser && $org) { $this->userService->addExistingUserToOrganization( $existingUser, $org, - $actingUser, ); - if ($this->isGranted('ROLE_SUPER_ADMIN')) { + if ($this->isGranted('ROLE_ADMIN')) { $this->loggerService->logSuperAdmin( $existingUser->getId(), $actingUser->getId(), @@ -252,61 +247,17 @@ class UserController extends AbstractController return $this->redirectToRoute('organization_show', ['id' => $orgId]); } - //Code semi-mort : On ne peut plus créer un utilisateur sans organisation - // Case : User exists but NO organization context -> throw error on email field. - -// if ($existingUser) { -// $this->loggerService->logError('Attempt to create user with existing email without organization', [ -// 'target_user_email' => $user->getid(), -// 'acting_user_id' => $actingUser->getId(), -// ]); -// -// $form->get('email')->addError( -// new \Symfony\Component\Form\FormError( -// 'This email is already in use. Add the user to an organization instead.' -// ) -// ); -// -// return $this->render('user/new.html.twig', [ -// 'user' => $user, -// 'form' => $form->createView(), -// 'organizationId' => $orgId, -// ]); -// } + // Case : user doesn't already exist $picture = $form->get('pictureUrl')->getData(); $this->userService->createNewUser($user, $actingUser, $picture); - if ($this->isGranted('ROLE_SUPER_ADMIN')) { - $this->loggerService->logSuperAdmin( - $user->getId(), - $actingUser->getId(), - "Super Admin created new user", - - ); - } - - // Case : Organization provided and user doesn't already exist - if ($orgId) { - $this->userService->linkUserToOrganization( - $user, - $org, - $actingUser, - ); - - if ($this->isGranted('ROLE_SUPER_ADMIN')) { - $this->loggerService->logSuperAdmin( - $user->getId(), - $actingUser->getId(), - "Super Admin linked user to organization during creation", - $org->getId() - ); - } - $this->addFlash('success', 'Nouvel utilisateur créé et ajouté à l\'organisation avec succès. '); - return $this->redirectToRoute('organization_show', ['id' => $orgId]); - } - $this->addFlash('success', 'Nouvel utilisateur créé avec succès. '); - return $this->redirectToRoute('user_index'); + $this->userService->linkUserToOrganization( + $user, + $org, + ); + $this->addFlash('success', 'Nouvel utilisateur créé et ajouté à l\'organisation avec succès. '); + return $this->redirectToRoute('organization_show', ['id' => $orgId]); } return $this->render('user/new.html.twig', [ @@ -319,10 +270,10 @@ class UserController extends AbstractController $this->errorLogger->critical($e->getMessage()); if ($orgId) { - $this->addFlash('error', 'Une erreur est survenue lors de la création de l\'utilisateur pour l\'organisation .'); + $this->addFlash('danger', 'Une erreur est survenue lors de la création de l\'utilisateur pour l\'organisation .'); return $this->redirectToRoute('organization_show', ['id' => $orgId]); } - $this->addFlash('error', 'Une erreur est survenue lors de la création de l\'utilisateur.'); + $this->addFlash('danger', 'Une erreur est survenue lors de la création de l\'utilisateur.'); return $this->redirectToRoute('user_index'); } } @@ -499,7 +450,7 @@ class UserController extends AbstractController if (!$user) { // Security/audit log for missing user $this->loggerService->logEntityNotFound('User', ['id' => $id], $actingUser->getId()); - $this->addFlash('error', "L'utilisateur demandé n'existe pas."); + $this->addFlash('danger', "L'utilisateur demandé n'existe pas."); throw $this->createNotFoundException(self::NOT_FOUND); } @@ -562,7 +513,7 @@ class UserController extends AbstractController if ($e instanceof NotFoundHttpException) { throw $e; // keep 404 semantics } - $this->addFlash('error', 'Erreur lors de la suppression de l\'utilisateur\.'); + $this->addFlash('danger', 'Erreur lors de la suppression de l\'utilisateur\.'); return $this->redirectToRoute('user_index'); } } @@ -577,13 +528,13 @@ class UserController extends AbstractController $uo = $this->entityManager->getRepository(UsersOrganizations::class)->find($id); if (!$uo) { $this->loggerService->logEntityNotFound('UsersOrganization', ['id' => $id], $actingUser->getId()); - $this->addFlash('error', "La liaison utilisateur-organisation n'existe pas."); + $this->addFlash('danger', "La liaison utilisateur-organisation n'existe pas."); throw new NotFoundHttpException("UserOrganization not found"); } $application = $this->entityManager->getRepository(Apps::class)->find($request->get('appId')); if (!$application) { $this->loggerService->logEntityNotFound('Application', ['id' => $request->get('appId')], $actingUser->getId()); - $this->addFlash('error', "L'application demandée n'existe pas."); + $this->addFlash('danger', "L'application demandée n'existe pas."); throw $this->createNotFoundException(self::NOT_FOUND); } @@ -591,7 +542,7 @@ class UserController extends AbstractController $roleUser = $this->entityManager->getRepository(Roles::class)->findOneBy(['name' => 'USER']); if (!$roleUser) { $this->loggerService->logEntityNotFound('Role', ['name' => 'USER'], $actingUser->getId()); - $this->addFlash('error', "Le role de l'utilisateur n'existe pas."); + $this->addFlash('danger', "Le role de l'utilisateur n'existe pas."); throw $this->createNotFoundException('User role not found'); } diff --git a/src/EventSubscriber/UserSubscriber.php b/src/EventSubscriber/UserSubscriber.php index 3e27e14..3e559d9 100644 --- a/src/EventSubscriber/UserSubscriber.php +++ b/src/EventSubscriber/UserSubscriber.php @@ -30,26 +30,21 @@ class UserSubscriber implements EventSubscriberInterface $user = $event->getNewUser(); $actingUser = $event->getActingUser(); - // 1. Generate Token (If logic was moved here, otherwise assume UserService set it) - // If the token generation logic is still in UserService, just send the email here. - // If you moved generating the token here, do it now. - - // 2. Send Email - // Note: You might need to pass the token in the Event if it's not stored in the DB entity - // or generate a new one here if appropriate. + // 1. Send Email if ($user->getPasswordToken()) { $this->emailService->sendPasswordSetupEmail($user, $user->getPasswordToken()); } - // 3. Log the creation - $this->loggerService->logUserCreated($user->getId(), $actingUser->getId()); + // 2. Logic-based Logging (Moved from Service) + if (in_array('ROLE_ADMIN', $actingUser->getRoles(), true)) { + $this->loggerService->logSuperAdmin( + $user->getId(), + $actingUser->getId(), + "Super Admin created new user: " . $user->getUserIdentifier() + ); + } - // 4. Create the Audit Action - $this->actionService->createAction( - "Create new user", - $actingUser, - null, - $user->getUserIdentifier() - ); + // 3. General Audit Trail + $this->actionService->createAction("USER_CREATED", $actingUser, null, $user->getUserIdentifier()); } } \ No newline at end of file diff --git a/src/Service/UserService.php b/src/Service/UserService.php index 0f0dd1c..f367e03 100644 --- a/src/Service/UserService.php +++ b/src/Service/UserService.php @@ -516,14 +516,7 @@ class UserService $user->setIsActive(true); $this->entityManager->persist($user); } - $uo = new UsersOrganizations(); - $uo->setUsers($user); - $uo->setOrganization($organization); - $uo->setStatut("INVITED"); - $uo->setIsActive(false); - $uo->setModifiedAt(new \DateTimeImmutable('now')); - $this->entityManager->persist($uo); - $this->entityManager->flush(); + $uo = $this->linkUserToOrganization($user, $organization); return $uo->getId(); } @@ -571,12 +564,11 @@ class UserService public function addExistingUserToOrganization( User $existingUser, Organizations $org, - User $actingUser, ): int { try { $uoId = $this->handleExistingUser($existingUser, $org); - + $actingUser = $this->getUserByIdentifier($this->security->getUser()->getUserIdentifier()); $this->loggerService->logExistingUserAddedToOrg( $existingUser->getId(), $org->getId(), @@ -610,20 +602,16 @@ class UserService try { $this->formatUserData($user, $picture, true); - // Generate token here if it's part of the user persistence flow - $token = $this->generatePasswordToken($user); $user->setisActive(false); + $this->generatePasswordToken($user); // Set token on the entity + $this->entityManager->persist($user); $this->entityManager->flush(); - + $this->eventDispatcher->dispatch(new UserCreatedEvent($user, $actingUser)); } catch (\Exception $e) { - // Error logging remains here because the event won't fire if exception occurs - $this->loggerService->logError('Error creating new user: ' . $e->getMessage(), [ - 'target_user_email' => $user->getEmail(), - 'acting_user_id' => $actingUser->getId(), - ]); + $this->loggerService->logError('Error creating user: ' . $e->getMessage()); throw $e; } } @@ -634,15 +622,17 @@ class UserService public function linkUserToOrganization( User $user, Organizations $org, - User $actingUser, ): UsersOrganizations { + $actingUser = $this->getUserByIdentifier($this->security->getUser()->getUserIdentifier()); try { + $roleUser = $this->rolesRepository->findOneBy(['name' => 'USER']); $uo = new UsersOrganizations(); $uo->setUsers($user); $uo->setOrganization($org); $uo->setStatut("INVITED"); $uo->setIsActive(false); + $uo->setRole($roleUser); $uo->setModifiedAt(new \DateTimeImmutable('now')); $this->entityManager->persist($uo); $this->entityManager->flush(); @@ -661,6 +651,15 @@ class UserService $org, "Added {$user->getUserIdentifier()} to {$org->getName()}" ); + $auRoles = $actingUser->getRoles(); + if (in_array('ROLE_ADMIN', $auRoles, true)) { + $this->loggerService->logSuperAdmin( + $user->getId(), + $actingUser->getId(), + "Admin linked user to organization during creation", + $org->getId() + ); + } $this->sendNewUserNotifications($user, $org, $actingUser);