From 3c58e153dc1954cd617b6ad9cc96ab3454a2a823 Mon Sep 17 00:00:00 2001 From: Frank Date: Sat, 11 Jul 2026 23:09:52 +0200 Subject: [PATCH] Soft-delete users instead of hard-deleting, add last login tracking Fixes the 500 on admin user deletion: deleting a user with an email_log row (i.e. basically anyone who received any email) hit an unhandled ForeignKeyConstraintViolationException, since email_log, reset_password_request and player all have non-nullable FKs to user with no cascade at the DB level. Instead of cascading the delete (which would be fine for email_log and reset_password_request but risky for player - removing a player row could corrupt other real players' session state), admin delete now sets a deletedAt timestamp instead of removing the row: - UserChecker blocks login for deleted users (checkPreAuth). - EmailLoggerListener rejects the message before send for deleted recipients (checked at actual send time, not at queue time). - Added a last_login_at column + a LoginSuccessEvent listener to populate it, and surfaced both last login and status in the admin users list. Added `app:users:purge-deleted`, a command intended to run on a schedule that permanently removes users who were soft-deleted more than 3 months ago and never played a game (join to Player via a NOT EXISTS subquery). For that eventual hard-delete to actually succeed, email_log and reset_password_request now cascade-delete at the DB level (migration drops+recreates both FK constraints with ON DELETE CASCADE) - player intentionally still isn't cascaded, so a user with game history can never be purged this way even by mistake. --- migrations/Version20260711210000.php | 38 +++++++++++++++++ src/Command/PurgeDeletedUsersCommand.php | 41 +++++++++++++++++++ .../Controller/GameAdminUserController.php | 9 ++-- src/Tech/Entity/EmailLog.php | 2 +- src/Tech/Entity/ResetPasswordRequest.php | 2 +- src/Tech/Entity/User.php | 35 ++++++++++++++++ .../EventListener/EmailLoggerListener.php | 5 +++ src/Tech/EventListener/LastLoginListener.php | 28 +++++++++++++ src/Tech/Repository/UserRepository.php | 16 ++++++++ src/Tech/Service/UserChecker.php | 4 ++ templates/game/admin/users/index.html.twig | 16 +++++++- 11 files changed, 188 insertions(+), 8 deletions(-) create mode 100644 migrations/Version20260711210000.php create mode 100644 src/Command/PurgeDeletedUsersCommand.php create mode 100644 src/Tech/EventListener/LastLoginListener.php diff --git a/migrations/Version20260711210000.php b/migrations/Version20260711210000.php new file mode 100644 index 0000000..960cfc0 --- /dev/null +++ b/migrations/Version20260711210000.php @@ -0,0 +1,38 @@ +addSql('ALTER TABLE `user` ADD deleted_at DATETIME DEFAULT NULL, ADD last_login_at DATETIME DEFAULT NULL'); + + $this->addSql('ALTER TABLE email_log DROP FOREIGN KEY FK_6FB4883A76ED395'); + $this->addSql('ALTER TABLE email_log ADD CONSTRAINT FK_6FB4883A76ED395 FOREIGN KEY (user_id) REFERENCES `user` (id) ON DELETE CASCADE'); + + $this->addSql('ALTER TABLE reset_password_request DROP FOREIGN KEY FK_7CE748AA76ED395'); + $this->addSql('ALTER TABLE reset_password_request ADD CONSTRAINT FK_7CE748AA76ED395 FOREIGN KEY (user_id) REFERENCES `user` (id) ON DELETE CASCADE'); + } + + public function down(Schema $schema): void + { + $this->addSql('ALTER TABLE email_log DROP FOREIGN KEY FK_6FB4883A76ED395'); + $this->addSql('ALTER TABLE email_log ADD CONSTRAINT FK_6FB4883A76ED395 FOREIGN KEY (user_id) REFERENCES `user` (id)'); + + $this->addSql('ALTER TABLE reset_password_request DROP FOREIGN KEY FK_7CE748AA76ED395'); + $this->addSql('ALTER TABLE reset_password_request ADD CONSTRAINT FK_7CE748AA76ED395 FOREIGN KEY (user_id) REFERENCES `user` (id)'); + + $this->addSql('ALTER TABLE `user` DROP deleted_at, DROP last_login_at'); + } +} diff --git a/src/Command/PurgeDeletedUsersCommand.php b/src/Command/PurgeDeletedUsersCommand.php new file mode 100644 index 0000000..4b4f937 --- /dev/null +++ b/src/Command/PurgeDeletedUsersCommand.php @@ -0,0 +1,41 @@ +userRepository->findPurgeableDeletedUsers($deletedBefore); + + foreach ($users as $user) { + $this->entityManager->remove($user); + } + + $this->entityManager->flush(); + + $output->writeln(sprintf('Purged %d deleted user(s).', count($users))); + + return Command::SUCCESS; + } +} diff --git a/src/Game/Controller/GameAdminUserController.php b/src/Game/Controller/GameAdminUserController.php index fab833e..3f4b501 100644 --- a/src/Game/Controller/GameAdminUserController.php +++ b/src/Game/Controller/GameAdminUserController.php @@ -66,11 +66,12 @@ final class GameAdminUserController extends AbstractController return $this->redirectToRoute('game_admin_users'); } - $username = $user->getUsername(); - $em->remove($user); - $em->flush(); + if (!$user->isDeleted()) { + $user->setDeletedAt(new \DateTimeImmutable()); + $em->flush(); + } - $this->addFlash('success', sprintf('User "%s" deleted.', $username)); + $this->addFlash('success', sprintf('User "%s" deleted.', $user->getUsername())); return $this->redirectToRoute('game_admin_users'); } diff --git a/src/Tech/Entity/EmailLog.php b/src/Tech/Entity/EmailLog.php index 5a295e4..f3dd169 100644 --- a/src/Tech/Entity/EmailLog.php +++ b/src/Tech/Entity/EmailLog.php @@ -16,7 +16,7 @@ class EmailLog private ?int $id = null; #[ORM\ManyToOne(targetEntity: User::class)] - #[ORM\JoinColumn(nullable: false)] + #[ORM\JoinColumn(nullable: false, onDelete: 'CASCADE')] private ?User $user = null; #[ORM\Column(length: 255)] diff --git a/src/Tech/Entity/ResetPasswordRequest.php b/src/Tech/Entity/ResetPasswordRequest.php index 111ab70..db4989d 100644 --- a/src/Tech/Entity/ResetPasswordRequest.php +++ b/src/Tech/Entity/ResetPasswordRequest.php @@ -19,7 +19,7 @@ class ResetPasswordRequest implements ResetPasswordRequestInterface private ?int $id = null; #[ORM\ManyToOne] - #[ORM\JoinColumn(nullable: false)] + #[ORM\JoinColumn(nullable: false, onDelete: 'CASCADE')] private ?User $user = null; public function __construct(User $user, \DateTimeInterface $expiresAt, string $selector, string $hashedToken) diff --git a/src/Tech/Entity/User.php b/src/Tech/Entity/User.php index 7f12364..ebbe084 100644 --- a/src/Tech/Entity/User.php +++ b/src/Tech/Entity/User.php @@ -42,6 +42,12 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface #[ORM\Column(type: 'boolean')] private bool $marketingOptIn = false; + #[ORM\Column(type: 'datetime_immutable', nullable: true)] + private ?\DateTimeImmutable $deletedAt = null; + + #[ORM\Column(type: 'datetime_immutable', nullable: true)] + private ?\DateTimeImmutable $lastLoginAt = null; + public function getId(): ?int { return $this->id; @@ -152,4 +158,33 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface return $this; } + + public function getDeletedAt(): ?\DateTimeImmutable + { + return $this->deletedAt; + } + + public function setDeletedAt(?\DateTimeImmutable $deletedAt): static + { + $this->deletedAt = $deletedAt; + + return $this; + } + + public function isDeleted(): bool + { + return $this->deletedAt !== null; + } + + public function getLastLoginAt(): ?\DateTimeImmutable + { + return $this->lastLoginAt; + } + + public function setLastLoginAt(?\DateTimeImmutable $lastLoginAt): static + { + $this->lastLoginAt = $lastLoginAt; + + return $this; + } } diff --git a/src/Tech/EventListener/EmailLoggerListener.php b/src/Tech/EventListener/EmailLoggerListener.php index 196dd3e..f694a26 100644 --- a/src/Tech/EventListener/EmailLoggerListener.php +++ b/src/Tech/EventListener/EmailLoggerListener.php @@ -42,6 +42,11 @@ class EmailLoggerListener continue; } + if ($user->isDeleted()) { + $event->reject(); + return; + } + $emailLog = new EmailLog(); $emailLog->setUser($user); $emailLog->setSentAt(new \DateTimeImmutable()); diff --git a/src/Tech/EventListener/LastLoginListener.php b/src/Tech/EventListener/LastLoginListener.php new file mode 100644 index 0000000..4f8de4b --- /dev/null +++ b/src/Tech/EventListener/LastLoginListener.php @@ -0,0 +1,28 @@ +getUser(); + if (!$user instanceof User) { + return; + } + + $user->setLastLoginAt(new \DateTimeImmutable()); + $this->entityManager->flush(); + } +} diff --git a/src/Tech/Repository/UserRepository.php b/src/Tech/Repository/UserRepository.php index 7742d1e..d04591b 100644 --- a/src/Tech/Repository/UserRepository.php +++ b/src/Tech/Repository/UserRepository.php @@ -44,4 +44,20 @@ class UserRepository extends ServiceEntityRepository implements PasswordUpgrader ->getQuery() ->getResult(); } + + /** + * Soft-deleted users, deleted before the given date, who never played a game. + * + * @return User[] + */ + public function findPurgeableDeletedUsers(\DateTimeImmutable $deletedBefore): array + { + return $this->createQueryBuilder('u') + ->andWhere('u.deletedAt IS NOT NULL') + ->andWhere('u.deletedAt <= :deletedBefore') + ->andWhere('NOT EXISTS (SELECT 1 FROM App\Game\Entity\Player p WHERE p.user = u)') + ->setParameter('deletedBefore', $deletedBefore) + ->getQuery() + ->getResult(); + } } diff --git a/src/Tech/Service/UserChecker.php b/src/Tech/Service/UserChecker.php index 66d6558..77f700a 100644 --- a/src/Tech/Service/UserChecker.php +++ b/src/Tech/Service/UserChecker.php @@ -16,6 +16,10 @@ class UserChecker implements UserCheckerInterface return; } + if ($user->isDeleted()) { + throw new CustomUserMessageAuthenticationException('This account no longer exists.'); + } + if (!$user->isVerified()) { throw new CustomUserMessageAuthenticationException('Your email address is not verified.', ['%resend_link%' => '/verify/resend']); } diff --git a/templates/game/admin/users/index.html.twig b/templates/game/admin/users/index.html.twig index 115bdfc..e84b9d9 100644 --- a/templates/game/admin/users/index.html.twig +++ b/templates/game/admin/users/index.html.twig @@ -18,6 +18,8 @@ Roles Verified Marketing + Last Login + Status Actions @@ -57,6 +59,16 @@ ✗ No {% endif %} + + {{ user.lastLoginAt ? user.lastLoginAt|date('Y-m-d H:i') : 'Never' }} + + + {% if user.deleted %} + Deleted + {% else %} + Active + {% endif %} + Edit - {% if user != app.user %} + {% if user != app.user and not user.deleted %}