From 027a8e0e346b12bad0e754f6e8488b98dac1cbdf Mon Sep 17 00:00:00 2001 From: Oliver Bates Date: Wed, 16 Sep 2026 15:25:48 +0200 Subject: [PATCH 1/5] Added user authentication --- config/packages/security.yaml | 12 +-- .../Admin/AddressBookController.php | 4 + src/Controller/Admin/CalendarController.php | 27 +++++- src/Controller/Admin/UserController.php | 34 +++++++- src/Repository/PrincipalRepository.php | 22 +++++ src/Security/LoginFormAuthenticator.php | 32 +++++-- src/Security/NormalUser.php | 84 +++++++++++++++++++ ...AdminUserProvider.php => UserProvider.php} | 30 ++++++- src/Security/UserVoter.php | 78 +++++++++++++++++ templates/_partials/navigation.html.twig | 14 +++- templates/_partials/share_modal.html.twig | 7 +- templates/addressbooks/index.html.twig | 6 +- templates/base.html.twig | 4 + templates/calendars/index.html.twig | 4 + templates/index.html.twig | 2 +- templates/users/delegates.html.twig | 6 +- templates/users/edit.html.twig | 6 +- templates/users/index.html.twig | 4 +- translations/messages+intl-icu.de.xlf | 8 +- translations/messages+intl-icu.en.xlf | 10 ++- translations/messages+intl-icu.fr.xliff | 8 +- 21 files changed, 363 insertions(+), 39 deletions(-) create mode 100644 src/Security/NormalUser.php rename src/Security/{AdminUserProvider.php => UserProvider.php} (63%) create mode 100644 src/Security/UserVoter.php diff --git a/config/packages/security.yaml b/config/packages/security.yaml index f229488d..e7d207d2 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -2,8 +2,8 @@ security: password_hashers: Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface: 'auto' providers: - admin_user_provider: - id: App\Security\AdminUserProvider + user_provider: + id: App\Security\UserProvider firewalls: dev: pattern: ^/(_(profiler|wdt)|css|images|js)/ @@ -17,7 +17,7 @@ security: lazy: true custom_authenticators: - App\Security\LoginFormAuthenticator - provider: admin_user_provider + provider: user_provider logout: path: app_logout target: dashboard @@ -27,8 +27,8 @@ security: - { path: ^/$, roles: PUBLIC_ACCESS } - { path: ^/dav, roles: PUBLIC_ACCESS } - { path: ^/dashboard, roles: ROLE_ADMIN, allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" } - - { path: ^/users, roles: ROLE_ADMIN, allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" } - - { path: ^/calendars, roles: ROLE_ADMIN, allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" } - - { path: ^/addressbooks, roles: ROLE_ADMIN, allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" } + - { path: ^/users, roles: [ROLE_ADMIN, ROLE_USER], allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" } + - { path: ^/calendars, roles: [ROLE_ADMIN, ROLE_USER], allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" } + - { path: ^/addressbooks, roles: [ROLE_ADMIN, ROLE_USER], allow_if: "'%env(default:default_admin_auth_bypass:ADMIN_AUTH_BYPASS)%' === 'true'" } - { path: ^/api/v1/health$, roles: PUBLIC_ACCESS } - { path: ^/api, roles: IS_AUTHENTICATED } diff --git a/src/Controller/Admin/AddressBookController.php b/src/Controller/Admin/AddressBookController.php index c43c1ebe..68325bbe 100644 --- a/src/Controller/Admin/AddressBookController.php +++ b/src/Controller/Admin/AddressBookController.php @@ -14,11 +14,13 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; use Symfony\Contracts\Translation\TranslatorInterface; +use Symfony\Component\Security\Http\Attribute\IsGranted; #[Route('/addressbooks', name: 'addressbook_')] class AddressBookController extends AbstractController { #[Route('/{userId}', name: 'index')] + #[IsGranted('access', 'userId')] public function addressBooks(ManagerRegistry $doctrine, #[MapEntity(id: 'userId')] User $user, int $userId): Response { $principalUri = $user->getPrincipalUri(); @@ -35,6 +37,7 @@ public function addressBooks(ManagerRegistry $doctrine, #[MapEntity(id: 'userId' #[Route('/{userId}/new', name: 'create')] #[Route('/{userId}/edit/{id}', name: 'edit', requirements: ['id' => "\d+"])] + #[IsGranted('access', 'userId')] public function addressbookCreate(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, ?int $id, TranslatorInterface $trans, BirthdayService $birthdayService): Response { $username = $user->getUsername(); @@ -97,6 +100,7 @@ public function addressbookCreate(ManagerRegistry $doctrine, Request $request, # } #[Route('/{userId}/delete/{id}', name: 'delete', requirements: ['id' => "\d+"], methods: ['POST'])] + #[IsGranted('access', 'userId')] public function addressbookDelete(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, string $id, TranslatorInterface $trans, BirthdayService $birthdayService): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { diff --git a/src/Controller/Admin/CalendarController.php b/src/Controller/Admin/CalendarController.php index 96960fb9..bc6568ae 100644 --- a/src/Controller/Admin/CalendarController.php +++ b/src/Controller/Admin/CalendarController.php @@ -19,11 +19,13 @@ use Symfony\Component\Routing\Annotation\Route; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Contracts\Translation\TranslatorInterface; +use Symfony\Component\Security\Http\Attribute\IsGranted; #[Route('/calendars', name: 'calendar_')] class CalendarController extends AbstractController { #[Route('/{userId}', name: 'index')] + #[IsGranted('access', 'userId')] public function calendars(ManagerRegistry $doctrine, UrlGeneratorInterface $router, #[MapEntity(id: 'userId')] User $user, int $userId): Response { $username = $user->getUsername(); @@ -73,6 +75,7 @@ public function calendars(ManagerRegistry $doctrine, UrlGeneratorInterface $rout #[Route('/{userId}/new', name: 'create')] #[Route('/{userId}/edit/{id}', name: 'edit', requirements: ['id' => "\d+"])] + #[IsGranted('access', 'userId')] public function calendarEdit(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, ?int $id, TranslatorInterface $trans): Response { $principalUri = $user->getPrincipalUri(); @@ -162,6 +165,7 @@ public function calendarEdit(ManagerRegistry $doctrine, Request $request, #[MapE } #[Route('/{userId}/shares/{calendarid}', name: 'shares', requirements: ['calendarid' => "\d+"])] + #[IsGranted('access', 'userId')] public function calendarShares(ManagerRegistry $doctrine, #[MapEntity(id: 'userId')] User $user, int $userId, string $calendarid, TranslatorInterface $trans): Response { $principalUri = $user->getPrincipalUri(); @@ -188,6 +192,7 @@ public function calendarShares(ManagerRegistry $doctrine, #[MapEntity(id: 'userI } #[Route('/{userId}/share/{instanceid}', name: 'share_add', requirements: ['instanceid' => "\d+"], methods: ['POST'])] + #[IsGranted('access', 'userId')] public function calendarShareAdd(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, string $instanceid, TranslatorInterface $trans): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { @@ -202,11 +207,23 @@ public function calendarShareAdd(ManagerRegistry $doctrine, Request $request, #[ throw $this->createNotFoundException('Calendar not found'); } - if (!is_numeric($request->request->get('principalId'))) { - throw new BadRequestHttpException(); - } + if ($this->isGranted('ROLE_ADMIN')) { + // in this case, this is the id of the principal to add + if (!is_numeric($request->request->get('principalId'))) { + throw new BadRequestHttpException(); + } + + $newShareeToAdd = $doctrine->getRepository(Principal::class)->findOneById($request->request->get('principalId')); + } else { + // in this case, this is the username of the user to add, we need to convert it to a principal + $userToAdd = $doctrine->getRepository(User::class)->findOneByUsername($request->request->get('principalId')); + if (!$userToAdd) { + $this->addFlash('warning', 'User does not exist'); + return $this->redirectToRoute('calendar_index', ['userId' => $userId]); + } + $newShareeToAdd = $doctrine->getRepository(Principal::class)->findOneByUri($userToAdd->getPrincipalUri()); + } - $newShareeToAdd = $doctrine->getRepository(Principal::class)->findOneById($request->request->get('principalId')); if (!$newShareeToAdd) { throw $this->createNotFoundException('Member not found'); } @@ -245,6 +262,7 @@ public function calendarShareAdd(ManagerRegistry $doctrine, Request $request, #[ } #[Route('/{userId}/delete/{id}', name: 'delete', requirements: ['id' => "\d+"], methods: ['POST'])] + #[IsGranted('access', 'userId')] public function calendarDelete(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, string $id, TranslatorInterface $trans): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { @@ -300,6 +318,7 @@ public function calendarDelete(ManagerRegistry $doctrine, Request $request, #[Ma } #[Route('/{userId}/revoke/{id}', name: 'revoke', requirements: ['id' => "\d+"], methods: ['POST'])] + #[IsGranted('access', 'userId')] public function calendarRevoke(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, string $id, TranslatorInterface $trans): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { diff --git a/src/Controller/Admin/UserController.php b/src/Controller/Admin/UserController.php index 6619c422..a6221d8b 100644 --- a/src/Controller/Admin/UserController.php +++ b/src/Controller/Admin/UserController.php @@ -19,11 +19,13 @@ use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\Routing\Annotation\Route; use Symfony\Contracts\Translation\TranslatorInterface; +use Symfony\Component\Security\Http\Attribute\IsGranted; #[Route('/users', name: 'user_')] class UserController extends AbstractController { #[Route('/', name: 'index')] + #[IsGranted('ROLE_ADMIN')] public function users(ManagerRegistry $doctrine): Response { $results = $doctrine->getRepository(Principal::class)->findAllMainPrincipalsWithUserIds(); @@ -33,8 +35,25 @@ public function users(ManagerRegistry $doctrine): Response ]); } + #[Route('/{userId}', name: 'user', requirements: ['userId' => "\d+"])] + #[IsGranted('access', 'userId')] + public function user(ManagerRegistry $doctrine, #[MapEntity(id: 'userId')] User $user, int $userId): Response + { + $results = $doctrine->getRepository(Principal::class)->findOneMainPrincipalsWithUserId($userId); + + if (!$results) { + throw BadRequestHttpException('User not found'); + } + + return $this->render('users/index.html.twig', [ + 'results' => $results, + ]); + } + + #[Route('/new', name: 'create')] #[Route('/edit/{userId}', name: 'edit')] + #[IsGranted('access', 'userId')] public function userCreate(ManagerRegistry $doctrine, Utils $utils, Request $request, ?int $userId, TranslatorInterface $trans): Response { if ($userId) { @@ -115,7 +134,10 @@ public function userCreate(ManagerRegistry $doctrine, Utils $utils, Request $req $this->addFlash('success', $trans->trans('user.saved')); - return $this->redirectToRoute('user_index'); + if ($this->isGranted('ROLE_ADMIN')) { + return $this->redirectToRoute('user_index'); + } + return $this->redirectToRoute('user_user', ['userId' => $userId]); } return $this->render('users/edit.html.twig', [ @@ -126,6 +148,7 @@ public function userCreate(ManagerRegistry $doctrine, Utils $utils, Request $req } #[Route('/delete/{userId}', name: 'delete', methods: ['POST'])] + #[IsGranted('access', 'userId')] public function userDelete(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, TranslatorInterface $trans): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { @@ -195,10 +218,14 @@ public function userDelete(ManagerRegistry $doctrine, Request $request, #[MapEnt $entityManager->flush(); $this->addFlash('success', $trans->trans('user.deleted')); - return $this->redirectToRoute('user_index'); + if ($this->isGranted('ROLE_ADMIN')) { + return $this->redirectToRoute('user_index'); + } + return $this->redirectToRoute('app_logout'); } #[Route('/delegates/{userId}', name: 'delegates')] + #[IsGranted('access', 'userId')] public function userDelegates(ManagerRegistry $doctrine, #[MapEntity(id: 'userId')] User $user, int $userId): Response { $principalUri = $user->getPrincipalUri(); @@ -222,6 +249,7 @@ public function userDelegates(ManagerRegistry $doctrine, #[MapEntity(id: 'userId } #[Route('/delegation/{userId}/{toggle}', name: 'delegation_toggle', requirements: ['toggle' => '(on|off)'], methods: ['POST'])] + #[IsGranted('access', 'userId')] public function userToggleDelegation(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, string $toggle): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { @@ -265,6 +293,7 @@ public function userToggleDelegation(ManagerRegistry $doctrine, Request $request } #[Route('/delegates/{userId}/add', name: 'delegate_add', methods: ['POST'])] + #[IsGranted('access', 'userId')] public function userDelegateAdd(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { @@ -309,6 +338,7 @@ public function userDelegateAdd(ManagerRegistry $doctrine, Request $request, #[M } #[Route('/delegates/{userId}/remove/{principalProxyId}/{delegateId}', name: 'delegate_remove', requirements: ['principalProxyId' => "\d+", 'delegateId' => "\d+"], methods: ['POST'])] + #[IsGranted('access', 'userId')] public function userDelegateRemove(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, int $principalProxyId, int $delegateId): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { diff --git a/src/Repository/PrincipalRepository.php b/src/Repository/PrincipalRepository.php index 4fce0d08..84ecd1d8 100644 --- a/src/Repository/PrincipalRepository.php +++ b/src/Repository/PrincipalRepository.php @@ -52,4 +52,26 @@ public function findAllMainPrincipalsWithUserIds(): array ->getQuery() ->getResult(); } + + /** + * @return array + */ + public function findOneMainPrincipalsWithUserId(int $userId): array + { + return $this->createQueryBuilder('p') + ->addSelect('u.id AS userId') + ->leftJoin( + \App\Entity\User::class, + 'u', + \Doctrine\ORM\Query\Expr\Join::WITH, + 'CONCAT(:prefix, u.username) = p.uri' + ) + ->andWhere('p.isMain = :isMain') + ->andWhere('u.id = :userid') + ->setParameter('isMain', true) + ->setParameter('userid', $userId) + ->setParameter('prefix', Principal::PREFIX) + ->getQuery() + ->getResult(); + } } diff --git a/src/Security/LoginFormAuthenticator.php b/src/Security/LoginFormAuthenticator.php index 0d314c61..874b5350 100644 --- a/src/Security/LoginFormAuthenticator.php +++ b/src/Security/LoginFormAuthenticator.php @@ -2,6 +2,8 @@ namespace App\Security; +use App\Entity\User; +use Doctrine\Persistence\ManagerRegistry; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -21,14 +23,16 @@ class LoginFormAuthenticator extends AbstractLoginFormAuthenticator { use TargetPathTrait; + private $doctrine; private $urlGenerator; private $csrfTokenManager; private $adminLogin; private $adminPassword; - public function __construct(UrlGeneratorInterface $urlGenerator, CsrfTokenManagerInterface $csrfTokenManager, string $adminLogin, string $adminPassword) + public function __construct(ManagerRegistry $doctrine, UrlGeneratorInterface $urlGenerator, CsrfTokenManagerInterface $csrfTokenManager, string $adminLogin, string $adminPassword) { + $this->doctrine = $doctrine; $this->urlGenerator = $urlGenerator; $this->csrfTokenManager = $csrfTokenManager; $this->adminLogin = $adminLogin; @@ -58,28 +62,38 @@ public function authenticate(Request $request): Passport $credentials['username'] ); - if ($credentials['username'] !== $this->adminLogin) { - // fail authentication with a custom error + if (($credentials['username'] == $this->adminLogin) && ($credentials['password'] == $this->adminPassword)) { + return new SelfValidatingPassport( + new UserBadge($this->adminLogin), + [new CsrfTokenBadge('authenticate', $credentials['csrf_token'])] + ); + } + + $user = $this->doctrine->getRepository(User::class)->findOneByUsername($credentials['username']); + if (!$user) { throw new CustomUserMessageAuthenticationException('Username could not be found.'); } - if ($credentials['password'] !== $this->adminPassword) { + if (!password_verify($credentials['password'], $user->getPassword())) { // fail authentication with a custom error throw new CustomUserMessageAuthenticationException('Invalid credentials.'); } return new SelfValidatingPassport( - new UserBadge($this->adminLogin), + new UserBadge($user->getUsername()), [new CsrfTokenBadge('authenticate', $credentials['csrf_token'])] ); } public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey): ?Response { - if ($targetPath = $this->getTargetPath($request->getSession(), $providerKey)) { - return new RedirectResponse($targetPath); - } + if (in_array("ROLE_ADMIN", $token->getRoleNames(), true)) { + return new RedirectResponse($this->urlGenerator->generate('dashboard')); + } else if (in_array("ROLE_USER", $token->getRoleNames(), true)) { + return new RedirectResponse($this->urlGenerator->generate('user_user', ['userId' => $token->getUser()->getUserId()])); + } - return new RedirectResponse($this->urlGenerator->generate('dashboard')); + # XXX: this should not be reachable + return new RedirectResponse($this->urlGenerator->generate('/')); } } diff --git a/src/Security/NormalUser.php b/src/Security/NormalUser.php new file mode 100644 index 00000000..4fa7fea6 --- /dev/null +++ b/src/Security/NormalUser.php @@ -0,0 +1,84 @@ +username = $username; + $this->password = $password; + $this->userId = $userId; + } + + /** + * @return (Role|string)[] The user roles + */ + public function getRoles(): array + { + return ['ROLE_USER']; + } + + /** + * Returns the password used to authenticate the user. + */ + public function getPassword(): string + { + return $this->password; + } + + /** + * Returns the salt that was originally used to encode the password. + * + * This can return null if the password was not encoded using a salt. + * + * @return string|null The salt + */ + public function getSalt() + { + return null; + } + + /** + * Returns the username used to authenticate the user. + * + * @return string The username + */ + public function getUsername() + { + return $this->username; + } + + + /** + * Returns the id used of the user. + * + * @return string The id + */ + public function getUserId() + { + return $this->userId; + } + + public function getUserIdentifier(): string + { + return $this->username; + } + + /** + * Removes sensitive data from the user. + * + * This is important if, at any given point, sensitive information like + * the plain-text password is stored on this object. + */ + public function eraseCredentials(): void + { + } +} diff --git a/src/Security/AdminUserProvider.php b/src/Security/UserProvider.php similarity index 63% rename from src/Security/AdminUserProvider.php rename to src/Security/UserProvider.php index deb34df7..c9bfc1a0 100644 --- a/src/Security/AdminUserProvider.php +++ b/src/Security/UserProvider.php @@ -2,13 +2,27 @@ namespace App\Security; +use App\Entity\User; +use Doctrine\Persistence\ManagerRegistry; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; -class AdminUserProvider implements UserProviderInterface +class UserProvider implements UserProviderInterface { + private $doctrine; + + private $adminLogin; + private $adminPassword; + + public function __construct(ManagerRegistry $doctrine, string $adminLogin, string $adminPassword) + { + $this->doctrine = $doctrine; + $this->adminLogin = $adminLogin; + $this->adminPassword = $adminPassword; + } + /** * Symfony calls this method if you use features like switch_user * or remember_me. @@ -27,7 +41,15 @@ public function loadUserByUsername($username) public function loadUserByIdentifier(string $identifier): UserInterface { - return new AdminUser($identifier, bin2hex(random_bytes(64))); + if ($identifier == $this->adminLogin) { + return new AdminUser($identifier, bin2hex(random_bytes(64))); + } + + $user = $this->doctrine->getRepository(User::class)->findOneByUsername($identifier); + if (!$user) { + throw new \Exception('Invalid username'); + } + return new NormalUser($identifier, $user->getPassword(), $user->getId()); } /** @@ -43,7 +65,7 @@ public function loadUserByIdentifier(string $identifier): UserInterface */ public function refreshUser(UserInterface $user): UserInterface { - if (!$user instanceof AdminUser) { + if ((!$user instanceof AdminUser) && (!$user instanceof NormalUser)) { throw new UnsupportedUserException(sprintf('Invalid user class "%s".', get_class($user))); } @@ -55,6 +77,6 @@ public function refreshUser(UserInterface $user): UserInterface */ public function supportsClass($class): bool { - return AdminUser::class === $class; + return (AdminUser::class === $class) || (NormalUser::class === $class); } } diff --git a/src/Security/UserVoter.php b/src/Security/UserVoter.php new file mode 100644 index 00000000..9e2e741a --- /dev/null +++ b/src/Security/UserVoter.php @@ -0,0 +1,78 @@ +doctrine = $doctrine; + } + + protected function supports(string $attribute, mixed $subject): bool + { + // if the voter doesn't support this attribute, return false + if (!in_array($attribute, [self::ACCESS])) { + return false; + } + + return true; + } + + protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token, ?Vote $vote = null): bool + { + $user = $token->getUser(); + + if ($user instanceof AdminUser) { + // admins can alway access everything + return true; + } + + if (!$user instanceof NormalUser) { + // the user must be logged in; if not, deny access + $vote?->addReason('The user is not logged in.'); + return false; + } + + $userId = $subject; + + return match($attribute) { + self::ACCESS => $this->canAccess($user, $userId, $vote), + default => throw new \LogicException('This code should not be reached!') + }; + } + + private function canAccess(NormalUser $logged_user, int $userId, ?Vote $vote): bool + { + $user = $this->doctrine->getRepository(User::class)->findOneById($userId); + if (!$user) { + $vote?->addReason(sprintf( + 'Id %d does not exist', + $userId + )); + + return false; + } + + if ($logged_user->getUsername() === $user->getUsername()) { + return true; + } + + $vote?->addReason(sprintf( + 'The logged in user (username: %s) is not (id: %d)', + $logged_user->getUsername(), $userId + )); + + return false; + } +} diff --git a/templates/_partials/navigation.html.twig b/templates/_partials/navigation.html.twig index 63f4e913..7893dfc5 100644 --- a/templates/_partials/navigation.html.twig +++ b/templates/_partials/navigation.html.twig @@ -1,19 +1,31 @@ \ No newline at end of file + diff --git a/templates/_partials/share_modal.html.twig b/templates/_partials/share_modal.html.twig index a7833888..239e772e 100644 --- a/templates/_partials/share_modal.html.twig +++ b/templates/_partials/share_modal.html.twig @@ -21,11 +21,16 @@
+ {% if admin_logged %} + {% elseif user_logged %} + {# do not show to normal users the whole list of users #} + + {% endif %} {% if principals|length == 0 %} {{ "calendars.delegates.member.none"|trans }} @@ -48,4 +53,4 @@
- \ No newline at end of file + diff --git a/templates/addressbooks/index.html.twig b/templates/addressbooks/index.html.twig index 3d2ddd80..85cf0177 100644 --- a/templates/addressbooks/index.html.twig +++ b/templates/addressbooks/index.html.twig @@ -3,7 +3,11 @@ {% block body %} +{% if admin_logged %} {% include '_partials/back_button.html.twig' with { url: path('user_index'), text: "users.back"|trans } %} +{% elseif user_logged %} +{% include '_partials/back_button.html.twig' with { url: path('user_user', {userId: user_logged_id}), text: "users.back"|trans } %} +{% endif %}

{{ "addressbooks.for"|trans({'who': principal.displayName}) }} + {{ "addressbooks.new"|trans }}

@@ -39,4 +43,4 @@ {% include '_partials/delete_modal.html.twig' with {flavour: 'addressbooks'} %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/templates/base.html.twig b/templates/base.html.twig index 3d6f5c99..f126e16c 100644 --- a/templates/base.html.twig +++ b/templates/base.html.twig @@ -1,3 +1,7 @@ +{% set admin_logged = app.user and is_granted("ROLE_ADMIN") %} +{% set user_logged = app.user and is_granted("ROLE_USER") %} +{% set user_logged_id = app.user and is_granted("ROLE_USER") ? app.user.getUserId() : 0 %} + diff --git a/templates/calendars/index.html.twig b/templates/calendars/index.html.twig index 822dc5ce..d546aa62 100644 --- a/templates/calendars/index.html.twig +++ b/templates/calendars/index.html.twig @@ -3,7 +3,11 @@ {% block body %} +{% if admin_logged %} {% include '_partials/back_button.html.twig' with { url: path('user_index'), text: "users.back"|trans } %} +{% elseif user_logged %} +{% include '_partials/back_button.html.twig' with { url: path('user_user', {userId: user_logged_id}), text: "users.back"|trans } %} +{% endif %}

{{ "calendars.for"|trans({'who': principal.displayName}) }} + {{ "calendars.new"|trans }}

diff --git a/templates/index.html.twig b/templates/index.html.twig index 402b6045..9728ca1a 100644 --- a/templates/index.html.twig +++ b/templates/index.html.twig @@ -42,7 +42,7 @@ {% if webDAVEnabled %}{{ "enabled"|trans }}{% else %}{{ "disabled"|trans }}{% endif %} - {{ "admin.interface"|trans }} + {{ "index.login"|trans }} diff --git a/templates/users/delegates.html.twig b/templates/users/delegates.html.twig index 871bd8a5..b3c0715f 100644 --- a/templates/users/delegates.html.twig +++ b/templates/users/delegates.html.twig @@ -3,7 +3,11 @@ {% block body %} +{% if admin_logged %} {% include '_partials/back_button.html.twig' with { url: path('user_index'), text: "users.back"|trans } %} +{% elseif user_logged %} +{% include '_partials/back_button.html.twig' with { url: path('user_user', {userId: user_logged_id}), text: "users.back"|trans } %} +{% endif %}

{{ "calendars.delegates.for"|trans({'what': principal.displayName}) }} @@ -44,4 +48,4 @@ {% include '_partials/delete_modal.html.twig' with {flavour: 'delegates'} %} {% include '_partials/add_delegate_modal.html.twig' with {principals: allPrincipals} %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/templates/users/edit.html.twig b/templates/users/edit.html.twig index 137705bd..bf6b539e 100644 --- a/templates/users/edit.html.twig +++ b/templates/users/edit.html.twig @@ -3,7 +3,11 @@ {% block body %} +{% if admin_logged %} {% include '_partials/back_button.html.twig' with { url: path('user_index'), text: "users.back"|trans } %} +{% elseif user_logged %} +{% include '_partials/back_button.html.twig' with { url: path('user_user', {userId: user_logged_id}), text: "users.back"|trans } %} +{% endif %} {% if username %}

{{ "users.edit"|trans({'username': username }) }}

@@ -13,4 +17,4 @@ {{ form(form) }} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/templates/users/index.html.twig b/templates/users/index.html.twig index 099fb403..aed3a3d7 100644 --- a/templates/users/index.html.twig +++ b/templates/users/index.html.twig @@ -3,7 +3,9 @@ {% block body %} +{% if admin_logged %}

{{ "title.users_and_resources"|trans }}+ {{ "users.new"|trans }}

+{% endif %}
{% for result in results %} @@ -48,4 +50,4 @@ {% include '_partials/delete_modal.html.twig' with {flavour: 'users'} %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/translations/messages+intl-icu.de.xlf b/translations/messages+intl-icu.de.xlf index 3f9a28c7..5ec2e21f 100644 --- a/translations/messages+intl-icu.de.xlf +++ b/translations/messages+intl-icu.de.xlf @@ -29,8 +29,8 @@ label.error Fehler - - admin.interface + + index.login Administrationsoberfläche @@ -217,6 +217,10 @@ calendars.shared.with Kalender geteilt mit {who} + + calendars.share.with + Kalender mit [Benutzername] teilen + users.username Benutzername diff --git a/translations/messages+intl-icu.en.xlf b/translations/messages+intl-icu.en.xlf index 4c6e1cd4..6b0547ce 100644 --- a/translations/messages+intl-icu.en.xlf +++ b/translations/messages+intl-icu.en.xlf @@ -29,9 +29,9 @@ label.error Error - - admin.interface - Administration interface + + index.login + Login close @@ -217,6 +217,10 @@ calendars.shared.with Calendars shared with {who} + + calendars.share.with + Share calendar with [username] + users.username Username diff --git a/translations/messages+intl-icu.fr.xliff b/translations/messages+intl-icu.fr.xliff index fad1aede..d08de1ef 100644 --- a/translations/messages+intl-icu.fr.xliff +++ b/translations/messages+intl-icu.fr.xliff @@ -29,8 +29,8 @@ label.error Erreur - - admin.interface + + index.login Interface d'administration @@ -217,6 +217,10 @@ calendars.shared.with Calendriers partagés avec {who} + + calendars.share.with + Partagez le calendrier avec [nom d'utilisateur] + users.username Nom d'utilisateur From 4be8f96b152fc5256b2ca5ba1ac849aa1d3975ec Mon Sep 17 00:00:00 2001 From: Oliver Bates Date: Wed, 16 Sep 2026 15:28:33 +0200 Subject: [PATCH 2/5] Renamed Admin Controllers to User --- src/Controller/{Admin => User}/AddressBookController.php | 2 +- src/Controller/{Admin => User}/CalendarController.php | 2 +- src/Controller/{Admin => User}/DashboardController.php | 2 +- src/Controller/{Admin => User}/UserController.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename src/Controller/{Admin => User}/AddressBookController.php (99%) rename src/Controller/{Admin => User}/CalendarController.php (99%) rename src/Controller/{Admin => User}/DashboardController.php (98%) rename src/Controller/{Admin => User}/UserController.php (99%) diff --git a/src/Controller/Admin/AddressBookController.php b/src/Controller/User/AddressBookController.php similarity index 99% rename from src/Controller/Admin/AddressBookController.php rename to src/Controller/User/AddressBookController.php index 68325bbe..f65b1a5e 100644 --- a/src/Controller/Admin/AddressBookController.php +++ b/src/Controller/User/AddressBookController.php @@ -1,6 +1,6 @@ Date: Wed, 16 Sep 2026 16:43:36 +0200 Subject: [PATCH 3/5] Forgot to commit services.yaml --- config/services.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config/services.yaml b/config/services.yaml index 25662b12..254e837f 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -70,6 +70,11 @@ services: $adminLogin: "%env(ADMIN_LOGIN)%" $adminPassword: "%env(ADMIN_PASSWORD)%" + App\Security\UserProvider: + arguments: + $adminLogin: "%env(ADMIN_LOGIN)%" + $adminPassword: "%env(ADMIN_PASSWORD)%" + App\Logging\Monolog\PasswordFilterProcessor: tags: - { name: monolog.processor } From e00348c6cfe6fd396fb42161b40777ccb92e1b48 Mon Sep 17 00:00:00 2001 From: Oliver Bates Date: Thu, 17 Sep 2026 14:34:19 +0200 Subject: [PATCH 4/5] Users have to write the correct delegate, not choose from a list + correct translations --- src/Controller/User/UserController.php | 21 ++++++++++++++----- .../_partials/add_delegate_modal.html.twig | 7 ++++++- translations/messages+intl-icu.de.xlf | 10 ++++++--- translations/messages+intl-icu.en.xlf | 10 ++++++--- ...-icu.fr.xliff => messages+intl-icu.fr.xlf} | 8 +++++-- 5 files changed, 42 insertions(+), 14 deletions(-) rename translations/{messages+intl-icu.fr.xliff => messages+intl-icu.fr.xlf} (98%) diff --git a/src/Controller/User/UserController.php b/src/Controller/User/UserController.php index 0d417abe..7caa5756 100644 --- a/src/Controller/User/UserController.php +++ b/src/Controller/User/UserController.php @@ -300,13 +300,24 @@ public function userDelegateAdd(ManagerRegistry $doctrine, Request $request, #[M throw $this->createAccessDeniedException('Invalid CSRF token.'); } - if (!is_numeric($request->request->get('principalId'))) { - throw new BadRequestHttpException(); - } - $principalUri = $user->getPrincipalUri(); - $newMemberToAdd = $doctrine->getRepository(Principal::class)->findOneById($request->request->get('principalId')); + if ($this->isGranted('ROLE_ADMIN')) { + // in this case, this is the id of the principal to add + if (!is_numeric($request->request->get('principalId'))) { + throw new BadRequestHttpException(); + } + + $newMemberToAdd = $doctrine->getRepository(Principal::class)->findOneById($request->request->get('principalId')); + } else { + // in this case, this is the username of the member to add, we need to convert it to a principal + $memberToAdd = $doctrine->getRepository(User::class)->findOneByUsername($request->request->get('principalId')); + if (!$memberToAdd) { + $this->addFlash('warning', 'User does not exist'); + return $this->redirectToRoute('user_delegates', ['userId' => $userId]); + } + $newMemberToAdd = $doctrine->getRepository(Principal::class)->findOneByUri($memberToAdd->getPrincipalUri()); + } if (!$newMemberToAdd) { throw $this->createNotFoundException('Member not found'); diff --git a/templates/_partials/add_delegate_modal.html.twig b/templates/_partials/add_delegate_modal.html.twig index 6eaba06e..4e3cdc2c 100644 --- a/templates/_partials/add_delegate_modal.html.twig +++ b/templates/_partials/add_delegate_modal.html.twig @@ -9,11 +9,16 @@
+ {% if admin_logged %} + {% elseif user_logged %} + {# do not show to normal users the whole list of users #} + + {% endif %} {{ "delegates.member.help"|trans }}
@@ -30,4 +35,4 @@
- \ No newline at end of file + diff --git a/translations/messages+intl-icu.de.xlf b/translations/messages+intl-icu.de.xlf index 5ec2e21f..df8302fb 100644 --- a/translations/messages+intl-icu.de.xlf +++ b/translations/messages+intl-icu.de.xlf @@ -29,7 +29,7 @@ label.error Fehler - + index.login Administrationsoberfläche @@ -217,7 +217,7 @@ calendars.shared.with Kalender geteilt mit {who} - + calendars.share.with Kalender mit [Benutzername] teilen @@ -285,6 +285,10 @@ calendars.delegates.member.add Diesen Kalender mit anderen Benutzern teilen: + + calendars.delegates.member.add.username + Teile alle Kalender mit [Benutzername] + calendars.delegates.member.help Das Hinzufügen eines Benutzers, der bereits einen gemeinsamen Zugriff auf diesen Kalender hat, wirkt sich nur auf dessen Zugriffsrecht aus. @@ -589,7 +593,7 @@ calendar.share_access.3 lesen / schreiben - + calendar.public öffentlich diff --git a/translations/messages+intl-icu.en.xlf b/translations/messages+intl-icu.en.xlf index 6b0547ce..46148590 100644 --- a/translations/messages+intl-icu.en.xlf +++ b/translations/messages+intl-icu.en.xlf @@ -29,7 +29,7 @@ label.error Error - + index.login Login @@ -217,7 +217,7 @@ calendars.shared.with Calendars shared with {who} - + calendars.share.with Share calendar with [username] @@ -285,6 +285,10 @@ calendars.delegates.member.add Share this calendar with another user: + + calendars.delegates.member.add.username + Share all calendars with [username] + calendars.delegates.member.help Adding a user who already has a shared access to this calendar will only affect its access right @@ -589,7 +593,7 @@ calendar.share_access.3 read / write - + calendar.public public diff --git a/translations/messages+intl-icu.fr.xliff b/translations/messages+intl-icu.fr.xlf similarity index 98% rename from translations/messages+intl-icu.fr.xliff rename to translations/messages+intl-icu.fr.xlf index d08de1ef..8e8d908a 100644 --- a/translations/messages+intl-icu.fr.xliff +++ b/translations/messages+intl-icu.fr.xlf @@ -29,7 +29,7 @@ label.error Erreur - + index.login Interface d'administration @@ -217,7 +217,7 @@ calendars.shared.with Calendriers partagés avec {who} - + calendars.share.with Partagez le calendrier avec [nom d'utilisateur] @@ -285,6 +285,10 @@ calendars.delegates.member.add Partager ce calendrier avec un autre utilisateur : + + calendars.delegates.member.add.username + Partagez tous les calendriers avec [nom d'utilisateur] + calendars.delegates.member.help L'ajout d'un utilisateur ayant déjà un accès partagé à ce calendrier n'affectera que ses droits d'accès From 512adbb8e582fe9d8123994bffc1942f02ec9f7c Mon Sep 17 00:00:00 2001 From: Oliver Bates Date: Thu, 17 Sep 2026 14:37:09 +0200 Subject: [PATCH 5/5] Linter --- src/Controller/User/AddressBookController.php | 8 +- src/Controller/User/CalendarController.php | 47 +++++------ src/Controller/User/DashboardController.php | 2 +- src/Controller/User/UserController.php | 82 ++++++++++--------- src/Security/LoginFormAuthenticator.php | 16 ++-- src/Security/NormalUser.php | 1 - src/Security/UserProvider.php | 16 ++-- src/Security/UserVoter.php | 37 +++++---- 8 files changed, 106 insertions(+), 103 deletions(-) diff --git a/src/Controller/User/AddressBookController.php b/src/Controller/User/AddressBookController.php index a9393051..ab4ba70b 100644 --- a/src/Controller/User/AddressBookController.php +++ b/src/Controller/User/AddressBookController.php @@ -13,14 +13,14 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; -use Symfony\Contracts\Translation\TranslatorInterface; use Symfony\Component\Security\Http\Attribute\IsGranted; +use Symfony\Contracts\Translation\TranslatorInterface; #[Route('/addressbooks', name: 'addressbook_')] class AddressBookController extends AbstractController { #[Route('/{userId}', name: 'index')] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function addressBooks(ManagerRegistry $doctrine, #[MapEntity(id: 'userId')] User $user, int $userId): Response { $principalUri = $user->getPrincipalUri(); @@ -42,7 +42,7 @@ public function addressBooks(ManagerRegistry $doctrine, #[MapEntity(id: 'userId' #[Route('/{userId}/new', name: 'create')] #[Route('/{userId}/edit/{id}', name: 'edit', requirements: ['id' => "\d+"])] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function addressbookCreate(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, ?int $id, TranslatorInterface $trans, BirthdayService $birthdayService): Response { $username = $user->getUsername(); @@ -105,7 +105,7 @@ public function addressbookCreate(ManagerRegistry $doctrine, Request $request, # } #[Route('/{userId}/delete/{id}', name: 'delete', requirements: ['id' => "\d+"], methods: ['POST'])] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function addressbookDelete(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, string $id, TranslatorInterface $trans, BirthdayService $birthdayService): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { diff --git a/src/Controller/User/CalendarController.php b/src/Controller/User/CalendarController.php index adbe21d8..73e2b858 100644 --- a/src/Controller/User/CalendarController.php +++ b/src/Controller/User/CalendarController.php @@ -18,14 +18,14 @@ use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\Routing\Annotation\Route; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; -use Symfony\Contracts\Translation\TranslatorInterface; use Symfony\Component\Security\Http\Attribute\IsGranted; +use Symfony\Contracts\Translation\TranslatorInterface; #[Route('/calendars', name: 'calendar_')] class CalendarController extends AbstractController { #[Route('/{userId}', name: 'index')] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function calendars(ManagerRegistry $doctrine, UrlGeneratorInterface $router, #[MapEntity(id: 'userId')] User $user, int $userId): Response { $username = $user->getUsername(); @@ -76,7 +76,7 @@ public function calendars(ManagerRegistry $doctrine, UrlGeneratorInterface $rout #[Route('/{userId}/new', name: 'create')] #[Route('/{userId}/edit/{id}', name: 'edit', requirements: ['id' => "\d+"])] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function calendarEdit(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, ?int $id, TranslatorInterface $trans): Response { $principalUri = $user->getPrincipalUri(); @@ -166,7 +166,7 @@ public function calendarEdit(ManagerRegistry $doctrine, Request $request, #[MapE } #[Route('/{userId}/shares/{calendarid}', name: 'shares', requirements: ['calendarid' => "\d+"])] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function calendarShares(ManagerRegistry $doctrine, #[MapEntity(id: 'userId')] User $user, int $userId, string $calendarid, TranslatorInterface $trans): Response { $principalUri = $user->getPrincipalUri(); @@ -193,7 +193,7 @@ public function calendarShares(ManagerRegistry $doctrine, #[MapEntity(id: 'userI } #[Route('/{userId}/share/{instanceid}', name: 'share_add', requirements: ['instanceid' => "\d+"], methods: ['POST'])] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function calendarShareAdd(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, string $instanceid, TranslatorInterface $trans): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { @@ -208,22 +208,23 @@ public function calendarShareAdd(ManagerRegistry $doctrine, Request $request, #[ throw $this->createNotFoundException('Calendar not found'); } - if ($this->isGranted('ROLE_ADMIN')) { - // in this case, this is the id of the principal to add - if (!is_numeric($request->request->get('principalId'))) { - throw new BadRequestHttpException(); - } - - $newShareeToAdd = $doctrine->getRepository(Principal::class)->findOneById($request->request->get('principalId')); - } else { - // in this case, this is the username of the user to add, we need to convert it to a principal - $userToAdd = $doctrine->getRepository(User::class)->findOneByUsername($request->request->get('principalId')); - if (!$userToAdd) { - $this->addFlash('warning', 'User does not exist'); - return $this->redirectToRoute('calendar_index', ['userId' => $userId]); - } - $newShareeToAdd = $doctrine->getRepository(Principal::class)->findOneByUri($userToAdd->getPrincipalUri()); - } + if ($this->isGranted('ROLE_ADMIN')) { + // in this case, this is the id of the principal to add + if (!is_numeric($request->request->get('principalId'))) { + throw new BadRequestHttpException(); + } + + $newShareeToAdd = $doctrine->getRepository(Principal::class)->findOneById($request->request->get('principalId')); + } else { + // in this case, this is the username of the user to add, we need to convert it to a principal + $userToAdd = $doctrine->getRepository(User::class)->findOneByUsername($request->request->get('principalId')); + if (!$userToAdd) { + $this->addFlash('warning', 'User does not exist'); + + return $this->redirectToRoute('calendar_index', ['userId' => $userId]); + } + $newShareeToAdd = $doctrine->getRepository(Principal::class)->findOneByUri($userToAdd->getPrincipalUri()); + } if (!$newShareeToAdd) { throw $this->createNotFoundException('Member not found'); @@ -263,7 +264,7 @@ public function calendarShareAdd(ManagerRegistry $doctrine, Request $request, #[ } #[Route('/{userId}/delete/{id}', name: 'delete', requirements: ['id' => "\d+"], methods: ['POST'])] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function calendarDelete(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, string $id, TranslatorInterface $trans): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { @@ -319,7 +320,7 @@ public function calendarDelete(ManagerRegistry $doctrine, Request $request, #[Ma } #[Route('/{userId}/revoke/{id}', name: 'revoke', requirements: ['id' => "\d+"], methods: ['POST'])] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function calendarRevoke(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, string $id, TranslatorInterface $trans): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { diff --git a/src/Controller/User/DashboardController.php b/src/Controller/User/DashboardController.php index 60ca8ef7..f0fc0fb6 100644 --- a/src/Controller/User/DashboardController.php +++ b/src/Controller/User/DashboardController.php @@ -16,7 +16,7 @@ class DashboardController extends AbstractController { #[Route('/dashboard', name: 'dashboard')] - #[IsGranted('ROLE_ADMIN')] + #[IsGranted('ROLE_ADMIN')] public function dashboard(ManagerRegistry $doctrine): Response { $usersCount = $doctrine->getRepository(User::class)->count([]); diff --git a/src/Controller/User/UserController.php b/src/Controller/User/UserController.php index 7caa5756..7dae605c 100644 --- a/src/Controller/User/UserController.php +++ b/src/Controller/User/UserController.php @@ -18,14 +18,14 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\Routing\Annotation\Route; -use Symfony\Contracts\Translation\TranslatorInterface; use Symfony\Component\Security\Http\Attribute\IsGranted; +use Symfony\Contracts\Translation\TranslatorInterface; #[Route('/users', name: 'user_')] class UserController extends AbstractController { #[Route('/', name: 'index')] - #[IsGranted('ROLE_ADMIN')] + #[IsGranted('ROLE_ADMIN')] public function users(ManagerRegistry $doctrine): Response { $results = $doctrine->getRepository(Principal::class)->findAllMainPrincipalsWithUserIds(); @@ -36,24 +36,23 @@ public function users(ManagerRegistry $doctrine): Response } #[Route('/{userId}', name: 'user', requirements: ['userId' => "\d+"])] - #[IsGranted('access', 'userId')] - public function user(ManagerRegistry $doctrine, #[MapEntity(id: 'userId')] User $user, int $userId): Response - { + #[IsGranted('access', 'userId')] + public function user(ManagerRegistry $doctrine, #[MapEntity(id: 'userId')] User $user, int $userId): Response + { $results = $doctrine->getRepository(Principal::class)->findOneMainPrincipalsWithUserId($userId); - if (!$results) { - throw BadRequestHttpException('User not found'); - } + if (!$results) { + throw BadRequestHttpException('User not found'); + } return $this->render('users/index.html.twig', [ 'results' => $results, ]); - } - + } #[Route('/new', name: 'create')] #[Route('/edit/{userId}', name: 'edit')] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function userCreate(ManagerRegistry $doctrine, Utils $utils, Request $request, ?int $userId, TranslatorInterface $trans): Response { if ($userId) { @@ -134,10 +133,11 @@ public function userCreate(ManagerRegistry $doctrine, Utils $utils, Request $req $this->addFlash('success', $trans->trans('user.saved')); - if ($this->isGranted('ROLE_ADMIN')) { - return $this->redirectToRoute('user_index'); - } - return $this->redirectToRoute('user_user', ['userId' => $userId]); + if ($this->isGranted('ROLE_ADMIN')) { + return $this->redirectToRoute('user_index'); + } + + return $this->redirectToRoute('user_user', ['userId' => $userId]); } return $this->render('users/edit.html.twig', [ @@ -148,7 +148,7 @@ public function userCreate(ManagerRegistry $doctrine, Utils $utils, Request $req } #[Route('/delete/{userId}', name: 'delete', methods: ['POST'])] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function userDelete(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, TranslatorInterface $trans): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { @@ -218,14 +218,15 @@ public function userDelete(ManagerRegistry $doctrine, Request $request, #[MapEnt $entityManager->flush(); $this->addFlash('success', $trans->trans('user.deleted')); - if ($this->isGranted('ROLE_ADMIN')) { - return $this->redirectToRoute('user_index'); - } - return $this->redirectToRoute('app_logout'); + if ($this->isGranted('ROLE_ADMIN')) { + return $this->redirectToRoute('user_index'); + } + + return $this->redirectToRoute('app_logout'); } #[Route('/delegates/{userId}', name: 'delegates')] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function userDelegates(ManagerRegistry $doctrine, #[MapEntity(id: 'userId')] User $user, int $userId): Response { $principalUri = $user->getPrincipalUri(); @@ -249,7 +250,7 @@ public function userDelegates(ManagerRegistry $doctrine, #[MapEntity(id: 'userId } #[Route('/delegation/{userId}/{toggle}', name: 'delegation_toggle', requirements: ['toggle' => '(on|off)'], methods: ['POST'])] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function userToggleDelegation(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, string $toggle): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { @@ -293,7 +294,7 @@ public function userToggleDelegation(ManagerRegistry $doctrine, Request $request } #[Route('/delegates/{userId}/add', name: 'delegate_add', methods: ['POST'])] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function userDelegateAdd(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { @@ -302,22 +303,23 @@ public function userDelegateAdd(ManagerRegistry $doctrine, Request $request, #[M $principalUri = $user->getPrincipalUri(); - if ($this->isGranted('ROLE_ADMIN')) { - // in this case, this is the id of the principal to add - if (!is_numeric($request->request->get('principalId'))) { - throw new BadRequestHttpException(); - } - - $newMemberToAdd = $doctrine->getRepository(Principal::class)->findOneById($request->request->get('principalId')); - } else { - // in this case, this is the username of the member to add, we need to convert it to a principal - $memberToAdd = $doctrine->getRepository(User::class)->findOneByUsername($request->request->get('principalId')); - if (!$memberToAdd) { - $this->addFlash('warning', 'User does not exist'); - return $this->redirectToRoute('user_delegates', ['userId' => $userId]); - } - $newMemberToAdd = $doctrine->getRepository(Principal::class)->findOneByUri($memberToAdd->getPrincipalUri()); - } + if ($this->isGranted('ROLE_ADMIN')) { + // in this case, this is the id of the principal to add + if (!is_numeric($request->request->get('principalId'))) { + throw new BadRequestHttpException(); + } + + $newMemberToAdd = $doctrine->getRepository(Principal::class)->findOneById($request->request->get('principalId')); + } else { + // in this case, this is the username of the member to add, we need to convert it to a principal + $memberToAdd = $doctrine->getRepository(User::class)->findOneByUsername($request->request->get('principalId')); + if (!$memberToAdd) { + $this->addFlash('warning', 'User does not exist'); + + return $this->redirectToRoute('user_delegates', ['userId' => $userId]); + } + $newMemberToAdd = $doctrine->getRepository(Principal::class)->findOneByUri($memberToAdd->getPrincipalUri()); + } if (!$newMemberToAdd) { throw $this->createNotFoundException('Member not found'); @@ -349,7 +351,7 @@ public function userDelegateAdd(ManagerRegistry $doctrine, Request $request, #[M } #[Route('/delegates/{userId}/remove/{principalProxyId}/{delegateId}', name: 'delegate_remove', requirements: ['principalProxyId' => "\d+", 'delegateId' => "\d+"], methods: ['POST'])] - #[IsGranted('access', 'userId')] + #[IsGranted('access', 'userId')] public function userDelegateRemove(ManagerRegistry $doctrine, Request $request, #[MapEntity(id: 'userId')] User $user, int $userId, int $principalProxyId, int $delegateId): Response { if (!$this->isCsrfTokenValid('admin_action', $request->getPayload()->getString('_token'))) { diff --git a/src/Security/LoginFormAuthenticator.php b/src/Security/LoginFormAuthenticator.php index c75fd656..711bdcdb 100644 --- a/src/Security/LoginFormAuthenticator.php +++ b/src/Security/LoginFormAuthenticator.php @@ -31,11 +31,11 @@ class LoginFormAuthenticator extends AbstractLoginFormAuthenticator public function __construct(ManagerRegistry $doctrine, UrlGeneratorInterface $urlGenerator, CsrfTokenManagerInterface $csrfTokenManager, string $adminLogin, string $adminPassword) { - $this->doctrine = $doctrine; + $this->doctrine = $doctrine; $this->urlGenerator = $urlGenerator; $this->csrfTokenManager = $csrfTokenManager; $this->adminLogin = $adminLogin; - $this->adminPassword = password_hash($adminPassword, PASSWORD_DEFAULT); + $this->adminPassword = password_hash($adminPassword, PASSWORD_DEFAULT); } protected function getLoginUrl(Request $request): string @@ -90,13 +90,13 @@ function (string $presentedPassword) use ($username, $username_to_test, $passwor public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey): ?Response { - if (in_array("ROLE_ADMIN", $token->getRoleNames(), true)) { - return new RedirectResponse($this->urlGenerator->generate('dashboard')); - } else if (in_array("ROLE_USER", $token->getRoleNames(), true)) { - return new RedirectResponse($this->urlGenerator->generate('user_user', ['userId' => $token->getUser()->getUserId()])); - } + if (in_array('ROLE_ADMIN', $token->getRoleNames(), true)) { + return new RedirectResponse($this->urlGenerator->generate('dashboard')); + } elseif (in_array('ROLE_USER', $token->getRoleNames(), true)) { + return new RedirectResponse($this->urlGenerator->generate('user_user', ['userId' => $token->getUser()->getUserId()])); + } - # XXX: this should not be reachable + // XXX: this should not be reachable return new RedirectResponse($this->urlGenerator->generate('/')); } } diff --git a/src/Security/NormalUser.php b/src/Security/NormalUser.php index 4fa7fea6..60f172fa 100644 --- a/src/Security/NormalUser.php +++ b/src/Security/NormalUser.php @@ -56,7 +56,6 @@ public function getUsername() return $this->username; } - /** * Returns the id used of the user. * diff --git a/src/Security/UserProvider.php b/src/Security/UserProvider.php index a777d013..580217cd 100644 --- a/src/Security/UserProvider.php +++ b/src/Security/UserProvider.php @@ -17,8 +17,8 @@ class UserProvider implements UserProviderInterface private $adminPassword; public function __construct(ManagerRegistry $doctrine, string $adminLogin, string $adminPassword) - { - $this->doctrine = $doctrine; + { + $this->doctrine = $doctrine; $this->adminLogin = $adminLogin; $this->adminPassword = $adminPassword; } @@ -41,16 +41,16 @@ public function loadUserByUsername($username) public function loadUserByIdentifier(string $identifier): UserInterface { - if ($identifier == $this->adminLogin) { - return new AdminUser($identifier, bin2hex(random_bytes(64))); - } + if ($identifier == $this->adminLogin) { + return new AdminUser($identifier, bin2hex(random_bytes(64))); + } - $user = $this->doctrine->getRepository(User::class)->findOneByUsername($identifier); - if (!$user) { + $user = $this->doctrine->getRepository(User::class)->findOneByUsername($identifier); + if (!$user) { // instead of throwing an exception, return a fake user: this will // fail during authentication since the user does not exist return new NormalUser($identifier, '', 0); - } + } return new NormalUser($identifier, $user->getPassword(), $user->getId()); } diff --git a/src/Security/UserVoter.php b/src/Security/UserVoter.php index 9e2e741a..c13a9028 100644 --- a/src/Security/UserVoter.php +++ b/src/Security/UserVoter.php @@ -10,13 +10,13 @@ class UserVoter extends Voter { - const ACCESS = 'access'; + public const ACCESS = 'access'; private $doctrine; public function __construct(ManagerRegistry $doctrine) - { - $this->doctrine = $doctrine; + { + $this->doctrine = $doctrine; } protected function supports(string $attribute, mixed $subject): bool @@ -25,7 +25,7 @@ protected function supports(string $attribute, mixed $subject): bool if (!in_array($attribute, [self::ACCESS])) { return false; } - + return true; } @@ -34,35 +34,36 @@ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInter $user = $token->getUser(); if ($user instanceof AdminUser) { - // admins can alway access everything - return true; - } + // admins can alway access everything + return true; + } if (!$user instanceof NormalUser) { // the user must be logged in; if not, deny access $vote?->addReason('The user is not logged in.'); + return false; } $userId = $subject; - return match($attribute) { + return match ($attribute) { self::ACCESS => $this->canAccess($user, $userId, $vote), - default => throw new \LogicException('This code should not be reached!') + default => throw new \LogicException('This code should not be reached!'), }; } private function canAccess(NormalUser $logged_user, int $userId, ?Vote $vote): bool { - $user = $this->doctrine->getRepository(User::class)->findOneById($userId); - if (!$user) { - $vote?->addReason(sprintf( - 'Id %d does not exist', - $userId - )); - - return false; - } + $user = $this->doctrine->getRepository(User::class)->findOneById($userId); + if (!$user) { + $vote?->addReason(sprintf( + 'Id %d does not exist', + $userId + )); + + return false; + } if ($logged_user->getUsername() === $user->getUsername()) { return true;