diff --git a/docs/implementation-examples.md b/docs/implementation-examples.md index 7728b4d0..3f00d5df 100644 --- a/docs/implementation-examples.md +++ b/docs/implementation-examples.md @@ -16,7 +16,7 @@ change one action. #### Recap * `UserTheme.php` create theme enum. * `UserValidator.php` add validation line in `validateUserValues()`. -* `UserAuthorizationChecker.php` add to the `$grantedUpdateKeys` in `isGrantedToUpdate()`. +* `UserPermissionVerifier.php` add to the `$grantedUpdateKeys` in `isGrantedToUpdate()`. * `UserUpdater.php` add `theme` to the authorized update keys in `updateUser()`. * `UserData.php` add instance variable and populate in constructor. * `UserFinderRepository.php` add to the class attribute `$fields`. diff --git a/docs/testing/testing-cheatsheet.md b/docs/testing/testing-cheatsheet.md index 4864f953..c8c4cf47 100644 --- a/docs/testing/testing-cheatsheet.md +++ b/docs/testing/testing-cheatsheet.md @@ -398,7 +398,7 @@ public function testNoteListAction( 'noteUpdatedAt' => (new \DateTime($noteData['updated_at']))->format('d. F Y • H:i'), 'userId' => $noteData['user_id'], 'userFullName' => $userLinkedToNoteRow['first_name'] . ' ' . $userLinkedToNoteRow['surname'], - // Has to match privilege from NoteAuthorizationGetter.php (rules are in NoteAuthorizationChecker.php) + // Has to match privilege from notePrivilegeDeterminer.php (rules are in notePermissionVerifier.php) 'privilege' => $expectedResult['privilege']->value, ]; // Assert response data diff --git a/public/assets/client/note/client-read-note-event-listener-setup.js b/public/assets/client/note/client-read-note-event-listener-setup.js index 6afd21eb..8adb89d4 100644 --- a/public/assets/client/note/client-read-note-event-listener-setup.js +++ b/public/assets/client/note/client-read-note-event-listener-setup.js @@ -194,7 +194,8 @@ export function addHideNoteBtnEventListener(btn) { toggleEyeIcon(); // Submit update to server submitUpdate( - {hidden: newHiddenValue}, `notes/${noteId}`, `notes/${noteId}` + {hidden: newHiddenValue}, `notes/${noteId}`, + `notes/${noteId}`, btn.closest('.note-container').id ).then(r => {}).catch(r => { // Revert eye to how it was before on failure toggleEyeIcon(); diff --git a/public/assets/client/note/client-read-template-note.html.js b/public/assets/client/note/client-read-template-note.html.js index 586d40f9..df9c170b 100644 --- a/public/assets/client/note/client-read-template-note.html.js +++ b/public/assets/client/note/client-read-template-note.html.js @@ -12,27 +12,27 @@ export function getNoteHtml(note) { ${createdAt} ${/*Show active eye icon if hidden*/ hidden === 1 || hidden === '1' ? `hide` : /* Else the non-active one if allowed*/ userHasPrivilegeTo(privilege, 'U') ? ` + >` : /* Else the non-active one if allowed*/ privilege.includes('U') ? ` hide` : ''} - ${/*Show delete button */ userHasPrivilegeTo(privilege, 'D') ? `delete` : ''} ${escapeHtml(userFullName)}
- ${userHasPrivilegeTo(privilege, 'R') ? '' : `
`} + ${privilege.includes('R') ? '' : `
`}
diff --git a/public/assets/client/read/client-read-main.js b/public/assets/client/read/client-read-main.js index af5c2de8..0bdc9b29 100644 --- a/public/assets/client/read/client-read-main.js +++ b/public/assets/client/read/client-read-main.js @@ -81,23 +81,26 @@ document.querySelector('#undelete-client-btn')?.addEventListener('click', () => // Toggle personal info edit icons -let personalInfoEditIconsToggle = document.querySelector('#toggle-personal-info-edit-icons'); let personalInfoContainer = document.querySelector('#client-personal-info-flex-container'); -personalInfoEditIconsToggle.addEventListener('click', () => { - let personalInfosEditIcons = document.querySelectorAll('#client-personal-info-flex-container div .contenteditable-edit-icon'); - for (let editIcon of personalInfosEditIcons) { - editIcon.classList.toggle('always-displayed-icon'); - } -}) +let personalInfoEditIconsToggle = document.querySelector('#toggle-personal-info-edit-icons'); +// PersonalInfoEditIconsToggle is not present if the user doesn't have update permission +if (personalInfoEditIconsToggle) { + personalInfoEditIconsToggle.addEventListener('click', () => { + let personalInfosEditIcons = document.querySelectorAll('#client-personal-info-flex-container div .contenteditable-edit-icon'); + for (let editIcon of personalInfosEditIcons) { + editIcon.classList.toggle('always-displayed-icon'); + } + }) // Display toggle btn if screen is touch device https://stackoverflow.com/a/13470899/9013718 -if ('ontouchstart' in window || navigator.msMaxTouchPoints) { - personalInfoEditIconsToggle.style.display = 'inline-block'; - // Increase right padding to not overlap edit icons - personalInfoContainer.style.paddingRight = '20px'; -} else { - personalInfoEditIconsToggle.style.display = 'none'; - personalInfoContainer.style.paddingRight = null; + if ('ontouchstart' in window || navigator.msMaxTouchPoints) { + personalInfoEditIconsToggle.style.display = 'inline-block'; + // Increase right padding to not overlap edit icons + personalInfoContainer.style.paddingRight = '20px'; + } else { + personalInfoEditIconsToggle.style.display = 'none'; + personalInfoContainer.style.paddingRight = null; + } } /** diff --git a/public/assets/general/ajax/submit-update-data.js b/public/assets/general/ajax/submit-update-data.js index 73cbffa8..f7983524 100644 --- a/public/assets/general/ajax/submit-update-data.js +++ b/public/assets/general/ajax/submit-update-data.js @@ -11,10 +11,10 @@ import {handleFail, removeValidationErrorMessages} from "./ajax-util/fail-handle * @param {string} route after base path e.g. clients/1 * @param {boolean|string} redirectToRouteIfUnauthenticated true or redirect route url after base path. * If true, the redirect url is the same as the given route - * + * @param domFieldId field id to display the validation error message for the correct field * @return Promise with as content server response as JSON */ -export function submitUpdate(formFieldsAndValues, route, redirectToRouteIfUnauthenticated = false) { +export function submitUpdate(formFieldsAndValues, route, redirectToRouteIfUnauthenticated = false, domFieldId = null) { return new Promise(function (resolve, reject) { // Make ajax call let xHttp = new XMLHttpRequest(); @@ -23,7 +23,7 @@ export function submitUpdate(formFieldsAndValues, route, redirectToRouteIfUnauth // Fail if (xHttp.status !== 201 && xHttp.status !== 200) { // Default fail handler - handleFail(xHttp); + handleFail(xHttp, domFieldId); // reject() only needed if promise is caught with .catch() reject(JSON.parse(xHttp.responseText)); } diff --git a/public/assets/general/validation/form-validation.js b/public/assets/general/validation/form-validation.js index 033cb7c2..906c744b 100644 --- a/public/assets/general/validation/form-validation.js +++ b/public/assets/general/validation/form-validation.js @@ -7,6 +7,7 @@ * */ export function displayValidationErrorMessage(fieldName, errorMessage, domFieldId = null) { let field; + console.log('displayValidationErrorMessage', fieldName, errorMessage, domFieldId); if (domFieldId !== null) { field = document.querySelector('#' + domFieldId); } else { diff --git a/src/Application/Action/Authentication/Ajax/AccountUnlockProcessAction.php b/src/Application/Action/Authentication/Ajax/AccountUnlockProcessAction.php index cc767a1b..8c434cf0 100644 --- a/src/Application/Action/Authentication/Ajax/AccountUnlockProcessAction.php +++ b/src/Application/Action/Authentication/Ajax/AccountUnlockProcessAction.php @@ -31,7 +31,7 @@ public function __invoke(ServerRequest $request, Response $response): Response // There may be other query params e.g. redirect if (isset($queryParams['id'], $queryParams['token'])) { try { - $userId = $this->accountUnlockTokenVerifier->getUserIdIfUnlockTokenIsValid( + $userId = $this->accountUnlockTokenVerifier->verifyUnlockTokenAndGetUserId( (int)$queryParams['id'], $queryParams['token'] ); diff --git a/src/Application/Action/Authentication/Ajax/LoginSubmitAction.php b/src/Application/Action/Authentication/Ajax/LoginSubmitAction.php index d5da56af..48dc35af 100644 --- a/src/Application/Action/Authentication/Ajax/LoginSubmitAction.php +++ b/src/Application/Action/Authentication/Ajax/LoginSubmitAction.php @@ -37,13 +37,9 @@ public function __invoke(ServerRequest $request, Response $response): Response try { // Throws InvalidCredentialsException if not allowed - $userId = $this->loginVerifier->getUserIdIfAllowedToLogin( - $submitValues, - $submitValues['g-recaptcha-response'] ?? null, - $queryParams - ); + $userId = $this->loginVerifier->verifyLoginAndGetUserId($submitValues, $queryParams); - // Clear all session data and regenerate session ID + // Regenerate session ID $this->sessionManager->regenerateId(); // Add user to session $this->session->set('user_id', $userId); @@ -107,7 +103,7 @@ public function __invoke(ServerRequest $request, Response $response): Response ['email' => $submitValues['email']], ); } catch (UnableToLoginStatusNotActiveException $unableToLoginException) { - // When user doesn't have status active + // When user status is not "active" $this->templateRenderer->addPhpViewAttribute('formError', true); // Add form error message $this->templateRenderer->addPhpViewAttribute('formErrorMessage', $unableToLoginException->getMessage()); diff --git a/src/Application/Action/Authentication/Ajax/RegisterVerifyProcessAction.php b/src/Application/Action/Authentication/Ajax/RegisterVerifyProcessAction.php index e1cf0dbb..717736d7 100644 --- a/src/Application/Action/Authentication/Ajax/RegisterVerifyProcessAction.php +++ b/src/Application/Action/Authentication/Ajax/RegisterVerifyProcessAction.php @@ -33,7 +33,7 @@ public function __invoke(ServerRequest $request, Response $response): Response // There may be other query params e.g. redirect if (isset($queryParams['id'], $queryParams['token'])) { try { - $userId = $this->registerTokenVerifier->getUserIdIfRegisterTokenIsValid( + $userId = $this->registerTokenVerifier->verifyRegisterTokenAndGetUserId( (int)$queryParams['id'], $queryParams['token'] ); diff --git a/src/Application/Action/Client/Page/ClientListPageAction.php b/src/Application/Action/Client/Page/ClientListPageAction.php index 0c24f47e..41fc9b4d 100644 --- a/src/Application/Action/Client/Page/ClientListPageAction.php +++ b/src/Application/Action/Client/Page/ClientListPageAction.php @@ -4,7 +4,7 @@ use App\Application\Responder\TemplateRenderer; use App\Domain\Authorization\Privilege; -use App\Domain\Client\Service\Authorization\ClientAuthorizationChecker; +use App\Domain\Client\Service\Authorization\ClientPermissionVerifier; use App\Domain\Client\Service\ClientListFilter\ClientListFilterChipProvider; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; @@ -14,7 +14,7 @@ final class ClientListPageAction public function __construct( private readonly TemplateRenderer $templateRenderer, private readonly ClientListFilterChipProvider $clientListFilterChipGetter, - private readonly ClientAuthorizationChecker $clientAuthorizationChecker + private readonly ClientPermissionVerifier $clientPermissionVerifier ) { } @@ -41,7 +41,7 @@ public function __invoke( $this->templateRenderer->addPhpViewAttribute('clientListFilters', $clientListFilters); $this->templateRenderer->addPhpViewAttribute( 'clientCreatePrivilege', - $this->clientAuthorizationChecker->isGrantedToCreate() ? Privilege::CREATE : Privilege::NONE + $this->clientPermissionVerifier->isGrantedToCreate() ? Privilege::CREATE : Privilege::NONE ); return $this->templateRenderer->render($response, 'client/clients-list.html.php'); diff --git a/src/Application/Action/User/Page/UserListPageAction.php b/src/Application/Action/User/Page/UserListPageAction.php index 2f912274..7d50f18c 100644 --- a/src/Application/Action/User/Page/UserListPageAction.php +++ b/src/Application/Action/User/Page/UserListPageAction.php @@ -3,7 +3,7 @@ namespace App\Application\Action\User\Page; use App\Application\Responder\TemplateRenderer; -use App\Domain\User\Service\Authorization\UserAuthorizationChecker; +use App\Domain\User\Service\Authorization\UserPermissionVerifier; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Slim\Exception\HttpForbiddenException; @@ -12,7 +12,7 @@ final class UserListPageAction { public function __construct( private readonly TemplateRenderer $templateRenderer, - private readonly UserAuthorizationChecker $userAuthorizationChecker, + private readonly UserPermissionVerifier $userPermissionVerifier, ) { } @@ -30,7 +30,7 @@ public function __invoke( ResponseInterface $response, array $args ): ResponseInterface { - if ($this->userAuthorizationChecker->isGrantedToRead()) { + if ($this->userPermissionVerifier->isGrantedToRead()) { return $this->templateRenderer->render($response, 'user/user-list.html.php'); } diff --git a/src/Application/Middleware/PhpViewExtensionMiddleware.php b/src/Application/Middleware/PhpViewExtensionMiddleware.php index 6c3dd018..063a8a43 100644 --- a/src/Application/Middleware/PhpViewExtensionMiddleware.php +++ b/src/Application/Middleware/PhpViewExtensionMiddleware.php @@ -3,7 +3,7 @@ namespace App\Application\Middleware; use App\Common\JsImportVersionAdder; -use App\Domain\User\Service\Authorization\UserAuthorizationChecker; +use App\Domain\User\Service\Authorization\UserPermissionVerifier; use App\Domain\Utility\Settings; use Cake\Database\Exception\DatabaseException; use Odan\Session\SessionInterface; @@ -28,7 +28,7 @@ public function __construct( private readonly SessionInterface $session, private readonly JsImportVersionAdder $jsImportVersionAdder, Settings $settings, - private readonly UserAuthorizationChecker $userAuthorizationChecker, + private readonly UserPermissionVerifier $userPermissionVerifier, private readonly RouteParserInterface $routeParser ) { $this->publicSettings = $settings->get('public'); @@ -60,7 +60,7 @@ public function process( try { $this->phpRenderer->addAttribute( 'userListAuthorization', - $this->userAuthorizationChecker->isGrantedToRead($loggedInUserId + 1, false) + $this->userPermissionVerifier->isGrantedToRead($loggedInUserId + 1, false) ); } catch (DatabaseException $databaseException) { // Mysql connection not working. Caught here to prevent error page from crashing diff --git a/src/Application/Middleware/UserAuthenticationMiddleware.php b/src/Application/Middleware/UserAuthenticationMiddleware.php index 777adfae..1d6cd2ab 100644 --- a/src/Application/Middleware/UserAuthenticationMiddleware.php +++ b/src/Application/Middleware/UserAuthenticationMiddleware.php @@ -29,7 +29,7 @@ public function __construct( } /** - * User authentication middleware. Check if user is logged in and if not + * User authentication middleware. Check if the user is logged in and if not * redirect to login page with redirect back query params. * * @param ServerRequestInterface $request @@ -37,10 +37,8 @@ public function __construct( * * @return ResponseInterface */ - public function process( - ServerRequestInterface $request, - RequestHandlerInterface $handler - ): ResponseInterface { + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { // Check if user is logged in if (($loggedInUserId = $this->session->get('user_id')) !== null) { // Check that the user status is active @@ -55,7 +53,7 @@ public function process( $response = $this->responseFactory->createResponse(); - // Inform user that he/she has to login first + // Inform the user that he/she has to log in before accessing the page $this->session->getFlash()->add('info', 'Please login to access this page.'); $queryParams = []; @@ -70,7 +68,7 @@ public function process( $queryParams['redirect'] = $routeName; } - // If it's a JSON request return 401 with the login url and its possible query params + // If it's a JSON request, return 401 with the login url and its possible query params if ($request->getHeaderLine('Content-Type') === 'application/json') { return $this->jsonResponder->respondWithJson( $response, @@ -78,7 +76,7 @@ public function process( 401 ); } - // If no redirect header is set, and it's not a JSON request, redirect to same url as the request after login + // If no redirect header is set, and it's not a JSON request, redirect to the same url as the request after login $queryParams = ['redirect' => $request->getUri()->getPath()]; return $this->redirectHandler->redirectToRouteName($response, 'login-page', [], $queryParams); diff --git a/src/Domain/Authentication/Repository/UserRoleFinderRepository.php b/src/Domain/Authentication/Repository/UserRoleFinderRepository.php index a3296fc5..423508d3 100644 --- a/src/Domain/Authentication/Repository/UserRoleFinderRepository.php +++ b/src/Domain/Authentication/Repository/UserRoleFinderRepository.php @@ -54,6 +54,25 @@ public function getUserRoleDataFromUser(int $userId): UserRoleData return new UserRoleData(); } + /** + * Get role hierarchy from a user that has status active. + * + * @param int $userId + * + * @return int user role hierarchy (lower hierarchy means higher privileged role) + */ + public function getRoleHierarchyByUserId(int $userId): int + { + $query = $this->queryFactory->selectQuery() + ->select(['user_role.hierarchy']) + ->from('user') + ->leftJoin('user_role', ['user.user_role_id = user_role.id']) + ->where(['user.deleted_at IS' => null, 'user.id' => $userId]); + $roleResultRow = $query->execute()->fetch('assoc'); + // If no role found, return highest hierarchy which means lowest privileged role + return (int)($roleResultRow['hierarchy'] ?? 1000); + } + /** * Get user role hierarchies mapped by name or id if param is true. * diff --git a/src/Domain/Authentication/Service/AccountUnlockTokenVerifier.php b/src/Domain/Authentication/Service/AccountUnlockTokenVerifier.php index f7a9502a..65593a12 100644 --- a/src/Domain/Authentication/Service/AccountUnlockTokenVerifier.php +++ b/src/Domain/Authentication/Service/AccountUnlockTokenVerifier.php @@ -30,7 +30,7 @@ public function __construct( * * @return int */ - public function getUserIdIfUnlockTokenIsValid(int $verificationId, string $token): int + public function verifyUnlockTokenAndGetUserId(int $verificationId, string $token): int { $verification = $this->verificationTokenFinderRepository->findUserVerification($verificationId); // Verify given token with token in database diff --git a/src/Domain/Authentication/Service/AuthenticationLogger.php b/src/Domain/Authentication/Service/AuthenticationLogger.php new file mode 100644 index 00000000..a494ab00 --- /dev/null +++ b/src/Domain/Authentication/Service/AuthenticationLogger.php @@ -0,0 +1,34 @@ +authenticationLoggerRepository->logLoginRequest( + $email, + $this->userNetworkSessionData->ipAddress, + $success, + $userId + ); + } +} diff --git a/src/Domain/Authentication/Service/LoginNonActiveUserHandler.php b/src/Domain/Authentication/Service/LoginNonActiveUserHandler.php index 7f20ef21..394b6b53 100644 --- a/src/Domain/Authentication/Service/LoginNonActiveUserHandler.php +++ b/src/Domain/Authentication/Service/LoginNonActiveUserHandler.php @@ -26,6 +26,7 @@ public function __construct( private readonly LocaleHelper $localeHelper, private readonly SecurityEmailChecker $securityEmailChecker, private readonly LoggerInterface $logger, + private readonly AuthenticationLogger $authenticationLogger, readonly Settings $settings, ) { $this->mainContactEmail = $this->settings->get( @@ -62,6 +63,9 @@ public function handleLoginAttemptFromNonActiveUser( __('Unable to login at the moment, please check your email inbox for a more detailed message.') ); + // Log failed login attempt + $this->authenticationLogger->logLoginRequest($dbUser->email, false, $dbUser->id); + try { $userId = $dbUser->id; $email = $dbUser->email; diff --git a/src/Domain/Authentication/Service/LoginVerifier.php b/src/Domain/Authentication/Service/LoginVerifier.php index 0687af35..575bf130 100644 --- a/src/Domain/Authentication/Service/LoginVerifier.php +++ b/src/Domain/Authentication/Service/LoginVerifier.php @@ -2,9 +2,7 @@ namespace App\Domain\Authentication\Service; -use App\Application\Data\UserNetworkSessionData; use App\Domain\Authentication\Exception\InvalidCredentialsException; -use App\Domain\Security\Repository\AuthenticationLoggerRepository; use App\Domain\Security\Service\SecurityLoginChecker; use App\Domain\User\Enum\UserActivity; use App\Domain\User\Enum\UserStatus; @@ -19,51 +17,42 @@ public function __construct( private readonly UserValidator $userValidator, private readonly SecurityLoginChecker $loginSecurityChecker, private readonly UserFinderRepository $userFinderRepository, - private readonly AuthenticationLoggerRepository $authenticationLoggerRepository, private readonly LoginNonActiveUserHandler $loginNonActiveUserHandler, private readonly UserActivityLogger $userActivityLogger, - private readonly UserNetworkSessionData $ipAddressData, + private readonly AuthenticationLogger $authenticationLogger, ) { } /** - * Checks if user is allowed to login. - * If yes, the user object is returned with id - * If no, an InvalidCredentialsException is thrown. + * Verifies the user's login credentials and returns the user's ID if the login is successful. * - * @param array $userLoginValues - * @param string|null $captcha user captcha response if filled out - * @param array $queryParams + * @param array $userLoginValues An associative array containing the user's login credentials. + * Expected keys are 'email' and 'password' and optionally 'g-recaptcha-response'. + * @param array $queryParams An associative array containing any additional query parameters. * - * @throws TransportExceptionInterface + * @throws TransportExceptionInterface If an error occurs while sending an email to a non-active user. + * @throws InvalidCredentialsException If the user does not exist or the password is incorrect. * - * @return int id + * @return int The ID of the user if the login is successful. */ - public function getUserIdIfAllowedToLogin( - array $userLoginValues, - ?string $captcha = null, - array $queryParams = [] - ): int { - // Validate entries coming from client + public function verifyLoginAndGetUserId(array $userLoginValues, array $queryParams = []): int + { + // Validate submitted values $this->userValidator->validateUserLogin($userLoginValues); + $captcha = $userLoginValues['g-recaptcha-response'] ?? null; - // Perform login security check + // Perform login security check before verifying credentials $this->loginSecurityChecker->performLoginSecurityCheck($userLoginValues['email'], $captcha); $dbUser = $this->userFinderRepository->findUserByEmail($userLoginValues['email']); - // Check if user exists - // Verify if password matches and enter login request + + // Check if user exists and verify if the password is correct if (isset($dbUser->email, $dbUser->passwordHash) && password_verify($userLoginValues['password'], $dbUser->passwordHash)) { // If password correct and status active, log user in by if ($dbUser->status === UserStatus::Active) { - // Insert login success request - $this->authenticationLoggerRepository->logLoginRequest( - $dbUser->email, - $this->ipAddressData->ipAddress, - true, - $dbUser->id - ); + // Log successful login request + $this->authenticationLogger->logLoginRequest($dbUser->email, true, $dbUser->id); $this->userActivityLogger->logUserActivity( UserActivity::READ, @@ -73,26 +62,22 @@ public function getUserIdIfAllowedToLogin( $dbUser->id ); - // Return id (not sure if it's better to regenerate session here in service or in action) + // Return id return (int)$dbUser->id; } - // If password is correct but the status not verified, send email to user + // If the password is correct but the status not verified, send email to user // captcha needed if email security check requires captcha $this->loginNonActiveUserHandler->handleLoginAttemptFromNonActiveUser($dbUser, $queryParams, $captcha); } - // Password not correct or user not existing - insert login request for ip - $this->authenticationLoggerRepository->logLoginRequest( - $userLoginValues['email'], - $this->ipAddressData->ipAddress, - false, - $dbUser->id - ); + // Password is not correct or user not existing - insert login request + $this->authenticationLogger->logLoginRequest($userLoginValues['email'], false, $dbUser->id); - // Perform second login request check to display the correct error message to the user if throttle is in place + // Perform second login request check after additional verification to display the correct error + // message to the user if throttle is in place $this->loginSecurityChecker->performLoginSecurityCheck($userLoginValues['email'], $captcha); - // Throw InvalidCred exception if user doesn't exist or wrong password + // Throw exception if the user doesn't exist or wrong password // Vague exception on purpose for security throw new InvalidCredentialsException($userLoginValues['email']); } diff --git a/src/Domain/Authentication/Service/PasswordChanger.php b/src/Domain/Authentication/Service/PasswordChanger.php index eb03fcd1..dda13b4d 100644 --- a/src/Domain/Authentication/Service/PasswordChanger.php +++ b/src/Domain/Authentication/Service/PasswordChanger.php @@ -5,7 +5,7 @@ use App\Domain\Authentication\Exception\ForbiddenException; use App\Domain\User\Enum\UserActivity; use App\Domain\User\Repository\UserUpdaterRepository; -use App\Domain\User\Service\Authorization\UserAuthorizationChecker; +use App\Domain\User\Service\Authorization\UserPermissionVerifier; use App\Domain\User\Service\UserValidator; use App\Domain\UserActivity\Service\UserActivityLogger; use Psr\Log\LoggerInterface; @@ -13,7 +13,7 @@ class PasswordChanger { public function __construct( - private readonly UserAuthorizationChecker $userAuthorizationChecker, + private readonly UserPermissionVerifier $userPermissionVerifier, private readonly UserUpdaterRepository $userUpdaterRepository, private readonly UserValidator $userValidator, private readonly UserActivityLogger $userActivityLogger, @@ -36,16 +36,16 @@ public function changeUserPassword(array $userValues, int $userId): bool // If old password is provided and user is allowed to update password if (isset($userValues['old_password']) - && $this->userAuthorizationChecker->isGrantedToUpdate(['password_hash' => 'value'], $userId) + && $this->userPermissionVerifier->isGrantedToUpdate(['password_hash' => 'value'], $userId) ) { // Verify old password; throws validation exception if not correct old password $this->userValidator->checkIfOldPasswordIsCorrect(['old_password' => $userValues['old_password']], $userId); } // Else if old password is not set, check if granted to update without it. if (!isset($userValues['old_password']) // Check if allowed to update password without the old password - && (!$this->userAuthorizationChecker->isGrantedToUpdate(['password_without_verification' => 'value'], $userId) + && (!$this->userPermissionVerifier->isGrantedToUpdate(['password_without_verification' => 'value'], $userId) // Check if allowed to update password_hash of that user - || !$this->userAuthorizationChecker->isGrantedToUpdate(['password_hash' => 'value'], $userId)) + || !$this->userPermissionVerifier->isGrantedToUpdate(['password_hash' => 'value'], $userId)) ) { throw new ForbiddenException('Not granted to update password without verification.'); } @@ -64,7 +64,7 @@ public function changeUserPassword(array $userValues, int $userId): bool */ private function updateUserPassword(string $newPassword, int $userId): bool { - if ($this->userAuthorizationChecker->isGrantedToUpdate(['password_hash' => 'value'], $userId)) { + if ($this->userPermissionVerifier->isGrantedToUpdate(['password_hash' => 'value'], $userId)) { $passwordHash = password_hash($newPassword, PASSWORD_DEFAULT); $updated = $this->userUpdaterRepository->changeUserPassword($passwordHash, $userId); if ($updated) { diff --git a/src/Domain/Authentication/Service/PasswordResetterWithToken.php b/src/Domain/Authentication/Service/PasswordResetterWithToken.php index 403823ae..07aad8ca 100644 --- a/src/Domain/Authentication/Service/PasswordResetterWithToken.php +++ b/src/Domain/Authentication/Service/PasswordResetterWithToken.php @@ -31,7 +31,7 @@ public function resetPasswordWithToken(array $passwordResetValues): bool // Validate passwords BEFORE token verification as it would be set to usedAt even if passwords are not valid $this->userValidator->validatePasswordReset($passwordResetValues); // If passwords are valid strings, verify token and set token to used - $userId = $this->verificationTokenVerifier->getUserIdIfTokenIsValid( + $userId = $this->verificationTokenVerifier->verifyTokenAndGetUserId( $passwordResetValues['id'], $passwordResetValues['token'] ); diff --git a/src/Domain/Authentication/Service/RegisterTokenVerifier.php b/src/Domain/Authentication/Service/RegisterTokenVerifier.php index 25f891a7..3d106fc0 100644 --- a/src/Domain/Authentication/Service/RegisterTokenVerifier.php +++ b/src/Domain/Authentication/Service/RegisterTokenVerifier.php @@ -30,7 +30,7 @@ public function __construct( * * @return int */ - public function getUserIdIfRegisterTokenIsValid(int $verificationId, string $token): int + public function verifyRegisterTokenAndGetUserId(int $verificationId, string $token): int { $verification = $this->verificationTokenFinderRepository->findUserVerification($verificationId); if ($verification->userId) { diff --git a/src/Domain/Authentication/Service/VerificationTokenVerifier.php b/src/Domain/Authentication/Service/VerificationTokenVerifier.php index 558dadcf..6eebb5ad 100644 --- a/src/Domain/Authentication/Service/VerificationTokenVerifier.php +++ b/src/Domain/Authentication/Service/VerificationTokenVerifier.php @@ -27,7 +27,7 @@ public function __construct( * * @see AccountUnlockTokenVerifier, RegisterTokenVerifier */ - public function getUserIdIfTokenIsValid(int $verificationId, string $token): int + public function verifyTokenAndGetUserId(int $verificationId, string $token): int { $verification = $this->verificationTokenFinderRepository->findUserVerification($verificationId); diff --git a/src/Domain/Authorization/AuthorizationChecker.php b/src/Domain/Authorization/AuthorizationChecker.php index 84547283..214ddb69 100644 --- a/src/Domain/Authorization/AuthorizationChecker.php +++ b/src/Domain/Authorization/AuthorizationChecker.php @@ -27,13 +27,13 @@ public function __construct( */ public function isAuthorizedByRole(UserRole $minimalRequiredRole): bool { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( $this->userNetworkSessionData->userId ); // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] // * Lower hierarchy number means higher privileged role $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); - return $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[$minimalRequiredRole->value]; + return $authenticatedUserRoleHierarchy <= $userRoleHierarchies[$minimalRequiredRole->value]; } } diff --git a/src/Domain/Authorization/Privilege.php b/src/Domain/Authorization/Privilege.php index ee900fe4..54e04586 100644 --- a/src/Domain/Authorization/Privilege.php +++ b/src/Domain/Authorization/Privilege.php @@ -4,31 +4,39 @@ enum Privilege: string { - // ? Instead of the function hasPrivilege() privilege loaded via Ajax by the client checks if the letter is contained - // in the name. For instance if update privilege is required, the client will check if privilege contains "U". + // The case values are the first letter of the privilege name. + // These are returned on a JSON Ajax request and used by the frontend to determine if the user has + // a certain privilege because JS doesn't have access to the hasPrivilege() function. + // For instance, if update privilege is required, the client will check if privilege value contains "U". + // No rights case NONE = 'NONE'; - // Allowed to read all entries + // Allowed to read case READ = 'R'; - // Allowed to read and create all entries + // Allowed to read and create case CREATE = 'CR'; - // Allowed only to create (needed when user cannot see hidden note but may create one) + // Allowed only to create (needed when the user is not allowed to see hidden notes but may create some) case ONLY_CREATE = 'C'; - // Allowed to read, create and update all entries + // Allowed to read, create and update case UPDATE = 'CRU'; - // Allowed to read, create, update and delete all entries + // Allowed to read, create, update and delete case DELETE = 'CRUD'; // Allowed to do everything on each note // case ALL = 'ALL'; /** - * Check if granted to perform action with given needed rights. - * Not sure though if it's smart to implement a hierarchical system - * for CRUD operations or if a collection of privileges would be better. + * Checks if the current privilege allows for the required privilege. * - * @param Privilege $requiredPrivilege + * This method uses a match expression to check if the current privilege ($this) allows for the required privilege. + * The match expression checks the required privilege against each possible privilege case. + * For each case, it checks if the current privilege is in an array of privileges that allow for the required privilege. + * If the current privilege is in the array, the method returns true, indicating that the required privilege is allowed. + * If the current privilege is not in the array, the method continues to the next case. + * If no cases match the required privilege, the method returns false, + * indicating that the required privilege is not allowed. * - * @return bool + * @param Privilege $requiredPrivilege The required privilege. + * @return bool True if the current privilege allows for the required privilege, false otherwise. */ public function hasPrivilege(Privilege $requiredPrivilege): bool { @@ -36,7 +44,7 @@ public function hasPrivilege(Privilege $requiredPrivilege): bool // Privilege READ is true if $this is either READ, CREATE, UPDATE or DELETE self::READ => in_array($this, [self::READ, self::CREATE, self::UPDATE, self::DELETE], true), self::CREATE => in_array($this, [self::CREATE, self::ONLY_CREATE, self::UPDATE, self::DELETE], true), - self::ONLY_CREATE => in_array($this, [self::CREATE, self::ONLY_CREATE], true), // should not be used + self::ONLY_CREATE => in_array($this, [self::CREATE, self::ONLY_CREATE], true), self::UPDATE => in_array($this, [self::UPDATE, self::DELETE], true), self::DELETE => $this === self::DELETE, self::NONE => true, diff --git a/src/Domain/Client/Data/ClientReadResult.php b/src/Domain/Client/Data/ClientReadResult.php index 5375c4af..01f28406 100644 --- a/src/Domain/Client/Data/ClientReadResult.php +++ b/src/Domain/Client/Data/ClientReadResult.php @@ -19,8 +19,8 @@ class ClientReadResult extends ClientData // and a new class ClientReadResultAggregateData could be created extending this one as it contains more attributes public ?NoteData $mainNoteData = null; // Main note data - // Client main data privilege (first-, second name, phone, email, location) - public ?Privilege $mainDataPrivilege = null; + // Client personal info privilege (first-, second name, phone, email, location) + public ?Privilege $generalPrivilege = null; public ?Privilege $clientStatusPrivilege = null; public ?Privilege $assignedUserPrivilege = null; public ?Privilege $noteCreatePrivilege = null; @@ -48,7 +48,7 @@ public function jsonSerialize(): array 'notesAmount' => $this->notesAmount, 'mainNoteData' => $this->mainNoteData, - 'mainDataPrivilege' => $this->mainDataPrivilege?->value, + 'personalInfoPrivilege' => $this->generalPrivilege?->value, 'clientStatusPrivilege' => $this->clientStatusPrivilege?->value, 'assignedUserPrivilege' => $this->assignedUserPrivilege?->value, 'noteCreatePrivilege' => $this->noteCreatePrivilege?->value, diff --git a/src/Domain/Client/Service/Authorization/ClientAuthorizationChecker.php b/src/Domain/Client/Service/Authorization/ClientAuthorizationChecker.php deleted file mode 100644 index 911ebe5d..00000000 --- a/src/Domain/Client/Service/Authorization/ClientAuthorizationChecker.php +++ /dev/null @@ -1,306 +0,0 @@ -loggedInUserId = $this->userNetworkSessionData->userId; - } - - /** - * Check if authenticated user is allowed to create client. - * - * @param ClientData|null $client null if check before actual client creation - * request otherwise it has to be provided - * - * @return bool - */ - public function isGrantedToCreate(?ClientData $client = null): bool - { - if ($this->loggedInUserId) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( - $this->loggedInUserId - ); - // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] - // * Lower hierarchy number means higher privileged role - $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); - - // Newcomer is not allowed to do anything - // If hierarchy number is greater or equals newcomer it means that user is not allowed - if ($authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::ADVISOR->value]) { - // Advisor may create clients but can only assign them to himself or leave it unassigned. - // If $client is null (not provided), advisor is authorized (being used to check if create btn should - // be displayed in template) - if ($client === null || $this->isGrantedToAssignUserToClient($client->userId)) { - // If authenticated user is at least advisor and client user id is himself, or it's a - // managing_advisor (logic in isGrantedToAssignUserToClient) -> granted to create - return true; - } - } - } - - $this->logger->notice( - 'User ' . $this->loggedInUserId . ' tried to create client but isn\'t allowed.' - ); - - return false; - } - - /** - * Check if authenticated user is allowed to assign user to client - * (client id not needed as the same rules applies for new clients and all existing clients) - * In own function to be used to filter dropdown options for frontend. - * - * @param int|null $assignedUserId - * @param UserRoleData|null $authenticatedUserRoleData optional so that it can be called outside this class - * @param array|null $userRoleHierarchies optional so that it can be called outside this class - * - * @return bool|void - */ - public function isGrantedToAssignUserToClient( - ?int $assignedUserId, - ?UserRoleData $authenticatedUserRoleData = null, - ?array $userRoleHierarchies = null - ) { - if ($this->loggedInUserId) { - // $authenticatedUserRoleData and $userRoleHierarchies passed as arguments if called inside this class - if ($authenticatedUserRoleData === null) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( - $this->loggedInUserId - ); - } - if ($userRoleHierarchies === null) { - // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] - // * Lower hierarchy number means higher privileged role - $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); - } - - // If hierarchy number is greater or equals advisor it means that user may create client - if ($authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::ADVISOR->value]) { - // Advisor may create clients but can only assign them to himself or leave it unassigned - if ($assignedUserId === $this->loggedInUserId || $assignedUserId === null - // managing advisor can link user to someone else - || $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) { - // If authenticated user is at least advisor and client user id is authenticated user himself, - // null (unassigned) or authenticated user is managing_advisor -> granted to assign - return true; - } - } - } - } - - /** - * Logic to check if logged-in user is granted to update client. - * - * @param array $clientDataToUpdate validated array with as key the column to - * update and value the new value (or fictive "value"). - * There may be one or multiple entries, depending on what the user wants to update. - * @param int|null $ownerId user_id linked to client - * @param bool $log log if forbidden (expected false when function is called for privilege setting) - * - * @return bool - */ - public function isGrantedToUpdate(array $clientDataToUpdate, ?int $ownerId, bool $log = true): bool - { - $grantedUpdateKeys = []; - if ($this->loggedInUserId) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( - $this->loggedInUserId - ); - // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] - // * Lower hierarchy number means higher privileged role - $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); - - // Roles: newcomer < advisor < managing_advisor < administrator - // If logged-in hierarchy value is smaller or equal advisor -> granted - if ($authenticatedUserRoleData->hierarchy <= $userRoleHierarchies['advisor']) { - // Things that advisor is allowed to change for all client records even when not owner - // "main_data" is the same as the group of columns that follows it - if (array_key_exists('main_data', $clientDataToUpdate)) { - $grantedUpdateKeys[] = 'main_data'; - } - // Same as main data but in separate columns to be returned as granted keys - if (array_key_exists('first_name', $clientDataToUpdate)) { - $grantedUpdateKeys[] = 'first_name'; - } - if (array_key_exists('last_name', $clientDataToUpdate)) { - $grantedUpdateKeys[] = 'last_name'; - } - if (array_key_exists('birthdate', $clientDataToUpdate)) { - $grantedUpdateKeys[] = 'birthdate'; - } - if (array_key_exists('location', $clientDataToUpdate)) { - $grantedUpdateKeys[] = 'location'; - } - if (array_key_exists('phone', $clientDataToUpdate)) { - $grantedUpdateKeys[] = 'phone'; - } - if (array_key_exists('email', $clientDataToUpdate)) { - $grantedUpdateKeys[] = 'email'; - } - if (array_key_exists('sex', $clientDataToUpdate)) { - $grantedUpdateKeys[] = 'sex'; - } - if (array_key_exists('vigilance_level', $clientDataToUpdate)) { - $grantedUpdateKeys[] = 'vigilance_level'; - } - /** Update main note authorization is in @see NoteAuthorizationChecker::isGrantedToUpdate () */ - - // Everything that owner and managing_advisor is permitted to do - // advisor may only edit client_status_id if he's owner | managing_advisor and higher is allowed - if ($this->loggedInUserId === $ownerId - || $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies['managing_advisor']) { - // Check if client_status_id is among data to be changed if yes add it to $grantedUpdateKeys array - if (array_key_exists('client_status_id', $clientDataToUpdate)) { - $grantedUpdateKeys[] = 'client_status_id'; - } - } - // Things that only managing_advisor and higher privileged are allowed to do - if ($authenticatedUserRoleData->hierarchy <= $userRoleHierarchies['managing_advisor'] - // isGrantedToAssignUserToClient can NOT be used as it expects a real user_id which is not provided - // in the case where user_id value is the string "value" from (ClientAuthGetter) to check if - // the authenticated user is allowed to change assigned user (html enabled/disabled) + && array_key_exists('user_id', $clientDataToUpdate)) { + $grantedUpdateKeys[] = 'user_id'; + } + + // If there is a request to undelete coming from the client, + // the same authorization rules than deletion are valid + if (array_key_exists('deleted_at', $clientDataToUpdate) && $this->isGrantedToDelete($ownerId, $log)) { + $grantedUpdateKeys[] = 'deleted_at'; + } + } + // If data that the user wanted to update and the grantedUpdateKeys are equal by having the same keys -> granted + foreach ($clientDataToUpdate as $key => $value) { + // If at least one array key doesn't exist in $grantedUpdateKeys it means that user is not permitted + if (!in_array($key, $grantedUpdateKeys, true)) { + if ($log === true) { + $this->logger->notice( + 'User ' . $this->loggedInUserId . ' tried to update client but isn\'t allowed to change' . + $key . ' to "' . $value . '".' + ); + } + + return false; + } + } + + return true; + } + + /** + * Check if the authenticated user is allowed to delete client. + * + * @param int|null $ownerId + * @param bool $log log if forbidden (expected false when function is called for privilege setting) + * + * @return bool + */ + public function isGrantedToDelete(?int $ownerId, bool $log = true): bool + { + if (!$this->loggedInUserId) { + $this->logger->error('loggedInUserId not set while isGrantedToDelete authorization check'); + return false; + } + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( + $this->loggedInUserId + ); + // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] + // * Lower hierarchy number means higher privileged role + $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); + + // Only managing_advisor and higher are allowed to delete client so user has to at least have this role + if ($authenticatedUserRoleHierarchy <= $userRoleHierarchies['managing_advisor']) { + return true; + } + + if ($log === true) { + $this->logger->notice( + 'User ' . $this->loggedInUserId . ' tried to delete client but isn\'t allowed.' + ); + } + + return false; + } + + /** + * Instead of a isGrantedToListClient(), this function checks + * with isGrantedToReadClient and removes clients that + * authenticated user may not see. + * + * @param ClientReadResult[]|null $clients + * + * @return ClientReadResult[] + */ + public function removeNonAuthorizedClientsFromList(?array $clients): array + { + $authorizedClients = []; + foreach ($clients ?? [] as $client) { + // Check if the authenticated user is allowed to read each client and if yes, add it to the return array + if ($this->isGrantedToRead($client->userId)) { + $authorizedClients[] = $client; + } + } + + return $authorizedClients; + } + + /** + * Check if the authenticated user is allowed to read client. + * + * @param int|null $ownerId + * @param string|\DateTimeImmutable|null $deletedAt + * @param bool $log log if forbidden (expected false when function is called for privilege setting) + * + * @return bool + */ + public function isGrantedToRead( + ?int $ownerId, + string|\DateTimeImmutable|null $deletedAt = null, + bool $log = true + ): bool { + if ($this->loggedInUserId) { + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( + $this->loggedInUserId + ); + // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] + // * Lower hierarchy number means higher privileged role + $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); + + // Newcomer are allowed to see all clients regardless of owner if not deleted + if ($authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::NEWCOMER->value] + && $deletedAt === null + ) { + return true; + } + // Managing advisors can see all clients including deleted ones + if ($authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) { + return true; + } + } + if ($log === true) { + $this->logger->notice( + 'User ' . $this->loggedInUserId . ' tried to read client but isn\'t allowed.' + ); + } + + return false; + } +} diff --git a/src/Domain/Client/Service/Authorization/ClientAuthorizationGetter.php b/src/Domain/Client/Service/Authorization/ClientPrivilegeDeterminer.php similarity index 66% rename from src/Domain/Client/Service/Authorization/ClientAuthorizationGetter.php rename to src/Domain/Client/Service/Authorization/ClientPrivilegeDeterminer.php index c571d7b2..987e96a5 100644 --- a/src/Domain/Client/Service/Authorization/ClientAuthorizationGetter.php +++ b/src/Domain/Client/Service/Authorization/ClientPrivilegeDeterminer.php @@ -8,10 +8,10 @@ * The client should know when to display edit and delete icons * Admins can edit all notes, users only their own. */ -class ClientAuthorizationGetter +class ClientPrivilegeDeterminer { public function __construct( - private readonly ClientAuthorizationChecker $clientAuthorizationChecker, + private readonly ClientPermissionVerifier $clientPermissionVerifier, ) { } @@ -23,15 +23,15 @@ public function __construct( * * @return Privilege */ - public function getMutationPrivilegeForClientColumn(?int $clientOwnerId, ?string $column = null): Privilege + public function determineMutationPrivilege(?int $clientOwnerId, ?string $column = null): Privilege { // Check first against the highest privilege, if allowed, directly return otherwise continue down the chain - if ($this->clientAuthorizationChecker->isGrantedToDelete($clientOwnerId, false)) { + if ($this->clientPermissionVerifier->isGrantedToDelete($clientOwnerId, false)) { return Privilege::DELETE; } // Value does not matter as keys are relevant if ($column !== null - && $this->clientAuthorizationChecker->isGrantedToUpdate([$column => 'value'], $clientOwnerId, false) + && $this->clientPermissionVerifier->isGrantedToUpdate([$column => 'value'], $clientOwnerId, false) ) { return Privilege::UPDATE; } diff --git a/src/Domain/Client/Service/ClientCreator.php b/src/Domain/Client/Service/ClientCreator.php index 2497f94c..92ef1f1a 100644 --- a/src/Domain/Client/Service/ClientCreator.php +++ b/src/Domain/Client/Service/ClientCreator.php @@ -7,7 +7,7 @@ use App\Domain\Client\Data\ClientData; use App\Domain\Client\Repository\ClientCreatorRepository; use App\Domain\Client\Repository\ClientDeleterRepository; -use App\Domain\Client\Service\Authorization\ClientAuthorizationChecker; +use App\Domain\Client\Service\Authorization\ClientPermissionVerifier; use App\Domain\Note\Service\NoteCreator; use App\Domain\User\Enum\UserActivity; use App\Domain\UserActivity\Service\UserActivityLogger; @@ -18,7 +18,7 @@ class ClientCreator public function __construct( private readonly ClientValidator $clientValidator, private readonly ClientCreatorRepository $clientCreatorRepository, - private readonly ClientAuthorizationChecker $clientAuthorizationChecker, + private readonly ClientPermissionVerifier $clientPermissionVerifier, private readonly NoteCreator $noteCreator, private readonly ClientDeleterRepository $clientDeleterRepository, private readonly UserActivityLogger $userActivityLogger, @@ -41,7 +41,7 @@ public function createClient(array $clientValues): int $client = new ClientData($clientValues); - if ($this->clientAuthorizationChecker->isGrantedToCreate($client)) { + if ($this->clientPermissionVerifier->isGrantedToCreate($client)) { // Insert client $clientId = $this->clientCreatorRepository->insertClient($client->toArrayForDatabase()); // Insert user activity diff --git a/src/Domain/Client/Service/ClientDeleter.php b/src/Domain/Client/Service/ClientDeleter.php index 0c0eae0f..c4c9f3c0 100644 --- a/src/Domain/Client/Service/ClientDeleter.php +++ b/src/Domain/Client/Service/ClientDeleter.php @@ -4,7 +4,7 @@ use App\Domain\Authentication\Exception\ForbiddenException; use App\Domain\Client\Repository\ClientDeleterRepository; -use App\Domain\Client\Service\Authorization\ClientAuthorizationChecker; +use App\Domain\Client\Service\Authorization\ClientPermissionVerifier; use App\Domain\Note\Repository\NoteDeleterRepository; use App\Domain\User\Enum\UserActivity; use App\Domain\UserActivity\Service\UserActivityLogger; @@ -14,7 +14,7 @@ class ClientDeleter public function __construct( private readonly ClientDeleterRepository $clientDeleterRepository, private readonly ClientFinder $clientFinder, - private readonly ClientAuthorizationChecker $clientAuthorizationChecker, + private readonly ClientPermissionVerifier $clientPermissionVerifier, private readonly UserActivityLogger $userActivityLogger, private readonly NoteDeleterRepository $noteDeleterRepository, ) { @@ -34,7 +34,7 @@ public function deleteClient(int $clientId): bool // Find post in db to get its ownership $clientFromDb = $this->clientFinder->findClient($clientId); - if ($this->clientAuthorizationChecker->isGrantedToDelete($clientFromDb->userId)) { + if ($this->clientPermissionVerifier->isGrantedToDelete($clientFromDb->userId)) { $this->noteDeleterRepository->deleteNotesFromClient($clientId); $deleted = $this->clientDeleterRepository->deleteClient($clientId); if ($deleted) { diff --git a/src/Domain/Client/Service/ClientFinder.php b/src/Domain/Client/Service/ClientFinder.php index 3fc43818..51b59ef6 100644 --- a/src/Domain/Client/Service/ClientFinder.php +++ b/src/Domain/Client/Service/ClientFinder.php @@ -9,10 +9,10 @@ use App\Domain\Client\Data\ClientReadResult; use App\Domain\Client\Repository\ClientFinderRepository; use App\Domain\Client\Repository\ClientStatus\ClientStatusFinderRepository; -use App\Domain\Client\Service\Authorization\ClientAuthorizationChecker; -use App\Domain\Client\Service\Authorization\ClientAuthorizationGetter; -use App\Domain\Note\Service\Authorization\NoteAuthorizationChecker; -use App\Domain\Note\Service\Authorization\NoteAuthorizationGetter; +use App\Domain\Client\Service\Authorization\ClientPermissionVerifier; +use App\Domain\Client\Service\Authorization\ClientPrivilegeDeterminer; +use App\Domain\Note\Service\Authorization\NotePermissionVerifier; +use App\Domain\Note\Service\Authorization\NotePrivilegeDeterminer; use App\Domain\Note\Service\NoteFinder; use App\Domain\User\Repository\UserFinderRepository; use App\Domain\User\Service\UserNameAbbreviator; @@ -25,10 +25,10 @@ public function __construct( private readonly UserNameAbbreviator $userNameAbbreviator, private readonly ClientStatusFinderRepository $clientStatusFinderRepository, private readonly NoteFinder $noteFinder, - private readonly ClientAuthorizationChecker $clientAuthorizationChecker, - private readonly ClientAuthorizationGetter $clientAuthorizationGetter, - private readonly NoteAuthorizationGetter $noteAuthorizationGetter, - private readonly NoteAuthorizationChecker $noteAuthorizationChecker, + private readonly ClientPermissionVerifier $clientPermissionVerifier, + private readonly ClientPrivilegeDeterminer $clientPrivilegeDeterminer, + private readonly NotePrivilegeDeterminer $notePrivilegeDeterminer, + private readonly NotePermissionVerifier $notePermissionVerifier, ) { } @@ -68,13 +68,13 @@ private function findClientsWhereWithResultAggregate(array $whereArray = ['clien $clientResultsWithAggregates = $this->clientFinderRepository->findClientsWithResultAggregate($whereArray); // Add assigned user and client status privilege to each clientResultAggregate foreach ($clientResultsWithAggregates as $key => $client) { - if ($this->clientAuthorizationChecker->isGrantedToRead($client->userId, $client->deletedAt)) { - $client->assignedUserPrivilege = $this->clientAuthorizationGetter->getMutationPrivilegeForClientColumn( + if ($this->clientPermissionVerifier->isGrantedToRead($client->userId, $client->deletedAt)) { + $client->assignedUserPrivilege = $this->clientPrivilegeDeterminer->determineMutationPrivilege( $client->userId, 'user_id' ); // Set client status privilege - $client->clientStatusPrivilege = $this->clientAuthorizationGetter->getMutationPrivilegeForClientColumn( + $client->clientStatusPrivilege = $this->clientPrivilegeDeterminer->determineMutationPrivilege( $client->userId, 'client_status_id', ); @@ -109,36 +109,36 @@ public function findClientReadAggregate(int $clientId): ClientReadResult { $clientResultAggregate = $this->clientFinderRepository->findClientAggregateByIdIncludingDeleted($clientId); if ($clientResultAggregate->id - && $this->clientAuthorizationChecker->isGrantedToRead( + && $this->clientPermissionVerifier->isGrantedToRead( $clientResultAggregate->userId, $clientResultAggregate->deletedAt ) ) { // Set client mutation privilege - $clientResultAggregate->mainDataPrivilege = $this->clientAuthorizationGetter->getMutationPrivilegeForClientColumn( + $clientResultAggregate->generalPrivilege = $this->clientPrivilegeDeterminer->determineMutationPrivilege( $clientResultAggregate->userId, - 'main_data' + 'personal_info' ); // Set main note privilege if ($clientResultAggregate->mainNoteData !== null) { - $clientResultAggregate->mainNoteData->privilege = $this->noteAuthorizationGetter->getMainNotePrivilege( + $clientResultAggregate->mainNoteData->privilege = $this->notePrivilegeDeterminer->getMainNotePrivilege( $clientResultAggregate->mainNoteData->userId, $clientResultAggregate->userId ); } // Set assigned user privilege - $clientResultAggregate->assignedUserPrivilege = $this->clientAuthorizationGetter->getMutationPrivilegeForClientColumn( + $clientResultAggregate->assignedUserPrivilege = $this->clientPrivilegeDeterminer->determineMutationPrivilege( $clientResultAggregate->userId, 'user_id', ); // Set client status privilege - $clientResultAggregate->clientStatusPrivilege = $this->clientAuthorizationGetter->getMutationPrivilegeForClientColumn( + $clientResultAggregate->clientStatusPrivilege = $this->clientPrivilegeDeterminer->determineMutationPrivilege( $clientResultAggregate->userId, 'client_status_id', ); // Set create note privilege - $clientResultAggregate->noteCreatePrivilege = $this->noteAuthorizationChecker->isGrantedToCreate( + $clientResultAggregate->noteCreatePrivilege = $this->notePermissionVerifier->isGrantedToCreate( 0, $clientResultAggregate->userId, false diff --git a/src/Domain/Client/Service/ClientFinderWithFilter.php b/src/Domain/Client/Service/ClientFinderWithFilter.php index 40d76446..9b0d683b 100644 --- a/src/Domain/Client/Service/ClientFinderWithFilter.php +++ b/src/Domain/Client/Service/ClientFinderWithFilter.php @@ -4,7 +4,7 @@ use App\Domain\Client\Data\ClientListResultCollection; use App\Domain\Client\Exception\InvalidClientFilterException; -use App\Domain\Client\Service\Authorization\ClientAuthorizationChecker; +use App\Domain\Client\Service\Authorization\ClientPermissionVerifier; use App\Domain\FilterSetting\FilterModule; use App\Domain\FilterSetting\FilterSettingSaver; @@ -13,7 +13,7 @@ class ClientFinderWithFilter public function __construct( private readonly ClientFinder $clientFinder, private readonly ClientFilterWhereConditionBuilder $clientFilterWhereConditionBuilder, - private readonly ClientAuthorizationChecker $clientAuthorizationChecker, + private readonly ClientPermissionVerifier $clientPermissionVerifier, private readonly FilterSettingSaver $filterSettingSaver, ) { } @@ -101,7 +101,7 @@ public function findClientsWithFilter(array $params): ClientListResultCollection ); $clientListResultCollection = $this->clientFinder->findClientListWithAggregates($queryBuilderWhereArray); // Remove clients that user is not allowed to see instead of throwing a ForbiddenException - $clientListResultCollection->clients = $this->clientAuthorizationChecker->removeNonAuthorizedClientsFromList( + $clientListResultCollection->clients = $this->clientPermissionVerifier->removeNonAuthorizedClientsFromList( $clientListResultCollection->clients ); diff --git a/src/Domain/Client/Service/ClientUpdater.php b/src/Domain/Client/Service/ClientUpdater.php index eb096e6e..6f945dfb 100644 --- a/src/Domain/Client/Service/ClientUpdater.php +++ b/src/Domain/Client/Service/ClientUpdater.php @@ -4,7 +4,7 @@ use App\Domain\Authentication\Exception\ForbiddenException; use App\Domain\Client\Repository\ClientUpdaterRepository; -use App\Domain\Client\Service\Authorization\ClientAuthorizationChecker; +use App\Domain\Client\Service\Authorization\ClientPermissionVerifier; use App\Domain\Exception\InvalidOperationException; use App\Domain\User\Enum\UserActivity; use App\Domain\UserActivity\Service\UserActivityLogger; @@ -15,7 +15,7 @@ public function __construct( private readonly ClientUpdaterRepository $clientUpdaterRepository, private readonly ClientValidator $clientValidator, private readonly ClientFinder $clientFinder, - private readonly ClientAuthorizationChecker $clientAuthorizationChecker, + private readonly ClientPermissionVerifier $clientPermissionVerifier, private readonly UserActivityLogger $userActivityLogger, ) { } @@ -36,7 +36,7 @@ public function updateClient(int $clientId, array $clientValues): array // Find note in db to compare its ownership $clientFromDb = $this->clientFinder->findClient($clientId); - if ($this->clientAuthorizationChecker->isGrantedToUpdate($clientValues, $clientFromDb->userId)) { + if ($this->clientPermissionVerifier->isGrantedToUpdate($clientValues, $clientFromDb->userId)) { $updateData = []; $responseData = null; // Check to be sure that only columns that may be updated are in the final $updateData array diff --git a/src/Domain/Client/Service/ClientUtilFinder.php b/src/Domain/Client/Service/ClientUtilFinder.php index a8c53eed..44a3fc11 100644 --- a/src/Domain/Client/Service/ClientUtilFinder.php +++ b/src/Domain/Client/Service/ClientUtilFinder.php @@ -4,7 +4,7 @@ use App\Domain\Client\Data\ClientDropdownValuesData; use App\Domain\Client\Repository\ClientStatus\ClientStatusFinderRepository; -use App\Domain\Client\Service\Authorization\ClientAuthorizationChecker; +use App\Domain\Client\Service\Authorization\ClientPermissionVerifier; use App\Domain\User\Repository\UserFinderRepository; use App\Domain\User\Service\UserNameAbbreviator; @@ -14,7 +14,7 @@ public function __construct( private readonly UserFinderRepository $userFinderRepository, private readonly UserNameAbbreviator $userNameAbbreviator, private readonly ClientStatusFinderRepository $clientStatusFinderRepository, - private readonly ClientAuthorizationChecker $clientAuthorizationChecker, + private readonly ClientPermissionVerifier $clientPermissionVerifier, ) { } @@ -36,7 +36,7 @@ public function findClientDropdownValues(?int $alreadyAssignedUserId = null): Cl // for it to be visible in GUI even if select is greyed out ($alreadyAssignedUserId !== null && $userData->id === $alreadyAssignedUserId) // Check if authenticated user is allowed to assign the currently iterating user to a client - || $this->clientAuthorizationChecker->isGrantedToAssignUserToClient($userData->id) + || $this->clientPermissionVerifier->isGrantedToAssignUserToClient($userData->id) ) { $grantedAssignableUsers[] = $userData; } diff --git a/src/Domain/Dashboard/Panel/UserFilterChipProvider.php b/src/Domain/Dashboard/Panel/UserFilterChipProvider.php index 313e8e45..82927d31 100644 --- a/src/Domain/Dashboard/Panel/UserFilterChipProvider.php +++ b/src/Domain/Dashboard/Panel/UserFilterChipProvider.php @@ -6,7 +6,7 @@ use App\Domain\FilterSetting\FilterModule; use App\Domain\FilterSetting\FilterSettingFinder; use App\Domain\User\Repository\UserFinderRepository; -use App\Domain\User\Service\Authorization\UserAuthorizationChecker; +use App\Domain\User\Service\Authorization\UserPermissionVerifier; use App\Domain\User\Service\UserNameAbbreviator; use Odan\Session\SessionInterface; @@ -17,7 +17,7 @@ public function __construct( private readonly SessionInterface $session, private readonly UserNameAbbreviator $userNameAbbreviator, private readonly UserFinderRepository $userFinderRepository, - private readonly UserAuthorizationChecker $userAuthorizationChecker, + private readonly UserPermissionVerifier $userPermissionVerifier, ) { } @@ -101,7 +101,7 @@ private function getUserActivityFilters(): array 'paramName' => 'user', 'paramValue' => $userId, 'category' => null, - 'authorized' => $this->userAuthorizationChecker->isGrantedToReadUserActivity((int)$userId, false), + 'authorized' => $this->userPermissionVerifier->isGrantedToReadUserActivity((int)$userId, false), ]); } } diff --git a/src/Domain/Note/Service/Authorization/NoteAuthorizationChecker.php b/src/Domain/Note/Service/Authorization/NotePermissionVerifier.php similarity index 83% rename from src/Domain/Note/Service/Authorization/NoteAuthorizationChecker.php rename to src/Domain/Note/Service/Authorization/NotePermissionVerifier.php index e2a33461..7e397fb2 100644 --- a/src/Domain/Note/Service/Authorization/NoteAuthorizationChecker.php +++ b/src/Domain/Note/Service/Authorization/NotePermissionVerifier.php @@ -7,7 +7,7 @@ use Odan\Session\SessionInterface; use Psr\Log\LoggerInterface; -class NoteAuthorizationChecker +class NotePermissionVerifier { public function __construct( private readonly SessionInterface $session, @@ -17,7 +17,7 @@ public function __construct( } /** - * Check if authenticated user is allowed to read note. + * Check if the authenticated user is allowed to read note. * * @param int $isMain * @param int|null $noteOwnerId optional owner id when main note @@ -35,17 +35,17 @@ public function isGrantedToRead( bool $log = true ): bool { if (($loggedInUserId = (int)$this->session->get('user_id')) !== 0) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( $loggedInUserId ); // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] // * Lower hierarchy number means higher privileged role $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); // newcomers may see all notes and main notes - if (($authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::NEWCOMER->value]) + if (($authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::NEWCOMER->value]) && ( // When hidden is not null or 0, user has to be advisor to read note in_array($isHidden, [null, 0, false], true) - || $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::ADVISOR->value] + || $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::ADVISOR->value] // If authenticated user is client owner or note owner -> granted to read hidden notes || $loggedInUserId === $clientOwnerId || $loggedInUserId === $noteOwnerId ) @@ -74,16 +74,16 @@ public function isGrantedToRead( public function isGrantedToCreate(int $isMain = 0, ?int $clientOwnerId = null, bool $log = true): bool { if (($loggedInUserId = (int)$this->session->get('user_id')) !== 0) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( $loggedInUserId ); // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] // * Lower hierarchy number means higher privileged role $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); if (($isMain === 0 // newcomers may see create notes for any client - && $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::NEWCOMER->value]) + && $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::NEWCOMER->value]) || ($isMain === 1 // only advisors and higher may create main notes - && $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::ADVISOR->value])) { + && $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::ADVISOR->value])) { return true; } } @@ -114,7 +114,7 @@ public function isGrantedToUpdate( bool $log = true ): bool { if (($loggedInUserId = (int)$this->session->get('user_id')) !== 0) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( $loggedInUserId ); // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] @@ -123,10 +123,10 @@ public function isGrantedToUpdate( // If owner or logged-in hierarchy value is smaller or equal managing_advisor -> granted to update if (($isMain === 0 && ($loggedInUserId === $noteOwnerId - || $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value])) + || $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value])) // If it's a main note, advisors and higher may edit it and $clientOwnerId could be relevant here || ($isMain === 1 // Should be identical to client update basic info authorization - && $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::ADVISOR->value]) + && $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::ADVISOR->value]) ) { return true; } @@ -158,7 +158,7 @@ public function isGrantedToDelete( bool $log = true ): bool { if (($loggedInUserId = (int)$this->session->get('user_id')) !== 0) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( $loggedInUserId ); // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] @@ -167,7 +167,7 @@ public function isGrantedToDelete( // If owner or logged-in hierarchy value is smaller or equal managing_advisor -> granted to update if ($loggedInUserId === $noteOwnerId - || $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) { + || $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) { return true; } } diff --git a/src/Domain/Note/Service/Authorization/NoteAuthorizationGetter.php b/src/Domain/Note/Service/Authorization/NotePrivilegeDeterminer.php similarity index 70% rename from src/Domain/Note/Service/Authorization/NoteAuthorizationGetter.php rename to src/Domain/Note/Service/Authorization/NotePrivilegeDeterminer.php index 1c7c9d01..804e9838 100644 --- a/src/Domain/Note/Service/Authorization/NoteAuthorizationGetter.php +++ b/src/Domain/Note/Service/Authorization/NotePrivilegeDeterminer.php @@ -8,10 +8,10 @@ * The client should know when to display edit and delete icons * Admins can edit all notes, users only their own. */ -class NoteAuthorizationGetter +class NotePrivilegeDeterminer { public function __construct( - private readonly NoteAuthorizationChecker $noteAuthorizationChecker, + private readonly NotePermissionVerifier $notePermissionVerifier, ) { } @@ -27,13 +27,13 @@ public function getMainNotePrivilege(?int $noteOwnerId, ?int $clientOwnerId): Pr { // Delete not possible with main note // Check first against the highest privilege, if allowed, directly return otherwise continue down the chain - if ($this->noteAuthorizationChecker->isGrantedToUpdate(1, $noteOwnerId, $clientOwnerId, false)) { + if ($this->notePermissionVerifier->isGrantedToUpdate(1, $noteOwnerId, $clientOwnerId, false)) { return Privilege::UPDATE; } - if ($this->noteAuthorizationChecker->isGrantedToCreate(1, $clientOwnerId, false)) { + if ($this->notePermissionVerifier->isGrantedToCreate(1, $clientOwnerId, false)) { return Privilege::CREATE; } - if ($this->noteAuthorizationChecker->isGrantedToRead(1, $noteOwnerId, $clientOwnerId, 0, false)) { + if ($this->notePermissionVerifier->isGrantedToRead(1, $noteOwnerId, $clientOwnerId, 0, false)) { return Privilege::READ; } @@ -52,15 +52,15 @@ public function getMainNotePrivilege(?int $noteOwnerId, ?int $clientOwnerId): Pr public function getNotePrivilege(int $noteOwnerId, ?int $clientOwnerId = null, ?int $hidden = null): Privilege { // Check first against the highest privilege, if allowed, directly return otherwise continue down the chain - if ($this->noteAuthorizationChecker->isGrantedToDelete($noteOwnerId, $clientOwnerId, false)) { + if ($this->notePermissionVerifier->isGrantedToDelete($noteOwnerId, $clientOwnerId, false)) { return Privilege::DELETE; } - if ($this->noteAuthorizationChecker->isGrantedToUpdate(0, $noteOwnerId, $clientOwnerId, false)) { + if ($this->notePermissionVerifier->isGrantedToUpdate(0, $noteOwnerId, $clientOwnerId, false)) { return Privilege::UPDATE; } // Create must NOT be included here as it's irrelevant on specific notes and has an impact on "READ" privilege as // read is lower than create in the hierarchy. - if ($this->noteAuthorizationChecker->isGrantedToRead(0, $noteOwnerId, $clientOwnerId, $hidden, false)) { + if ($this->notePermissionVerifier->isGrantedToRead(0, $noteOwnerId, $clientOwnerId, $hidden, false)) { return Privilege::READ; } diff --git a/src/Domain/Note/Service/NoteCreator.php b/src/Domain/Note/Service/NoteCreator.php index 7f11b81b..a3ff2ecb 100644 --- a/src/Domain/Note/Service/NoteCreator.php +++ b/src/Domain/Note/Service/NoteCreator.php @@ -5,7 +5,7 @@ use App\Application\Data\UserNetworkSessionData; use App\Domain\Authentication\Exception\ForbiddenException; use App\Domain\Note\Repository\NoteCreatorRepository; -use App\Domain\Note\Service\Authorization\NoteAuthorizationChecker; +use App\Domain\Note\Service\Authorization\NotePermissionVerifier; use App\Domain\User\Enum\UserActivity; use App\Domain\User\Service\UserFinder; use App\Domain\UserActivity\Service\UserActivityLogger; @@ -16,7 +16,7 @@ class NoteCreator public function __construct( private readonly NoteValidator $noteValidator, private readonly NoteCreatorRepository $noteCreatorRepository, - private readonly NoteAuthorizationChecker $noteAuthorizationChecker, + private readonly NotePermissionVerifier $notePermissionVerifier, private readonly UserActivityLogger $userActivityLogger, private readonly UserNetworkSessionData $userNetworkSessionData, private readonly UserFinder $userFinder, @@ -37,7 +37,7 @@ public function createNote(array $noteValues): array // Exception thrown if validation fails $this->noteValidator->validateNoteValues($noteValues, true); - if ($this->noteAuthorizationChecker->isGrantedToCreate((int)$noteValues['is_main'])) { + if ($this->notePermissionVerifier->isGrantedToCreate((int)$noteValues['is_main'])) { $noteId = $this->noteCreatorRepository->insertNote($noteValues); if (!empty($noteId)) { $this->userActivityLogger->logUserActivity( diff --git a/src/Domain/Note/Service/NoteDeleter.php b/src/Domain/Note/Service/NoteDeleter.php index de183295..b0068bd5 100644 --- a/src/Domain/Note/Service/NoteDeleter.php +++ b/src/Domain/Note/Service/NoteDeleter.php @@ -5,7 +5,7 @@ use App\Domain\Authentication\Exception\ForbiddenException; use App\Domain\Exception\InvalidOperationException; use App\Domain\Note\Repository\NoteDeleterRepository; -use App\Domain\Note\Service\Authorization\NoteAuthorizationChecker; +use App\Domain\Note\Service\Authorization\NotePermissionVerifier; use App\Domain\User\Enum\UserActivity; use App\Domain\UserActivity\Service\UserActivityLogger; @@ -14,7 +14,7 @@ class NoteDeleter public function __construct( private readonly NoteDeleterRepository $noteDeleterRepository, private readonly NoteFinder $noteFinder, - private readonly NoteAuthorizationChecker $noteAuthorizationChecker, + private readonly NotePermissionVerifier $notePermissionVerifier, private readonly UserActivityLogger $userActivityLogger, ) { } @@ -37,7 +37,7 @@ public function deleteNote(int $noteId): bool throw new InvalidOperationException('The main note cannot be deleted.'); } - if ($this->noteAuthorizationChecker->isGrantedToDelete($noteFromDb->userId)) { + if ($this->notePermissionVerifier->isGrantedToDelete($noteFromDb->userId)) { $deleted = $this->noteDeleterRepository->deleteNote($noteId); if ($deleted) { $this->userActivityLogger->logUserActivity( diff --git a/src/Domain/Note/Service/NoteFinder.php b/src/Domain/Note/Service/NoteFinder.php index da293055..9faefa69 100644 --- a/src/Domain/Note/Service/NoteFinder.php +++ b/src/Domain/Note/Service/NoteFinder.php @@ -7,13 +7,13 @@ use App\Domain\Note\Data\NoteData; use App\Domain\Note\Data\NoteResultData; use App\Domain\Note\Repository\NoteFinderRepository; -use App\Domain\Note\Service\Authorization\NoteAuthorizationGetter; +use App\Domain\Note\Service\Authorization\NotePrivilegeDeterminer; class NoteFinder { public function __construct( private readonly NoteFinderRepository $noteFinderRepository, - private readonly NoteAuthorizationGetter $noteAuthorizationGetter, + private readonly NotePrivilegeDeterminer $notePrivilegeDeterminer, private readonly ClientFinderRepository $clientFinderRepository, ) { } @@ -75,7 +75,7 @@ private function setNotePrivilegeAndRemoveMessageOfHidden( foreach ($notes as $noteResultData) { // Privilege only create possible if user may not see the note but may create one - $noteResultData->privilege = $this->noteAuthorizationGetter->getNotePrivilege( + $noteResultData->privilege = $this->notePrivilegeDeterminer->getNotePrivilege( (int)$noteResultData->userId, $clientOwnerId, $noteResultData->hidden, diff --git a/src/Domain/Note/Service/NoteUpdater.php b/src/Domain/Note/Service/NoteUpdater.php index 89cda892..c427ff11 100644 --- a/src/Domain/Note/Service/NoteUpdater.php +++ b/src/Domain/Note/Service/NoteUpdater.php @@ -5,7 +5,7 @@ use App\Domain\Authentication\Exception\ForbiddenException; use App\Domain\Note\Data\NoteData; use App\Domain\Note\Repository\NoteUpdaterRepository; -use App\Domain\Note\Service\Authorization\NoteAuthorizationChecker; +use App\Domain\Note\Service\Authorization\NotePermissionVerifier; use App\Domain\User\Enum\UserActivity; use App\Domain\UserActivity\Service\UserActivityLogger; @@ -15,7 +15,7 @@ public function __construct( private readonly NoteValidator $noteValidator, private readonly NoteUpdaterRepository $noteUpdaterRepository, private readonly NoteFinder $noteFinder, - private readonly NoteAuthorizationChecker $noteAuthorizationChecker, + private readonly NotePermissionVerifier $notePermissionVerifier, private readonly UserActivityLogger $userActivityLogger, ) { } @@ -40,7 +40,7 @@ public function updateNote(int $noteId, ?array $noteValues): bool $note = new NoteData($noteValues); - if ($this->noteAuthorizationChecker->isGrantedToUpdate($noteFromDb->isMain ?? 0, $noteFromDb->userId)) { + if ($this->notePermissionVerifier->isGrantedToUpdate($noteFromDb->isMain ?? 0, $noteFromDb->userId)) { $updateData = []; // Change message if (null !== $note->message) { @@ -51,11 +51,13 @@ public function updateNote(int $noteId, ?array $noteValues): bool if (null !== $note->hidden) { $updateData['hidden'] = $note->hidden; } - - $updated = $this->noteUpdaterRepository->updateNote($updateData, $noteId); - if ($updated) { - $this->userActivityLogger->logUserActivity(UserActivity::UPDATED, 'note', $noteId, $updateData); + $updated = false; + // $updateData empty string if request body is empty + if ($updateData !== []) { + $updated = $this->noteUpdaterRepository->updateNote($updateData, $noteId); } + $this->userActivityLogger->logUserActivity(UserActivity::UPDATED, 'note', $noteId, $updateData); + return $updated; } diff --git a/src/Domain/Note/Service/NoteValidator.php b/src/Domain/Note/Service/NoteValidator.php index 5d65b00f..b937b934 100644 --- a/src/Domain/Note/Service/NoteValidator.php +++ b/src/Domain/Note/Service/NoteValidator.php @@ -24,11 +24,20 @@ public function __construct( public function validateNoteValues(array $noteValues, bool $isCreateMode = true): void { $validator = new Validator(); - $validator = $validator->requirePresence('message', true, __('Field is required')) + $validator = $validator->requirePresence('message', $isCreateMode, __('Field is required')) ->maxLength('message', 1000, __('Maximum length is 1000', 1000)) // is_main and client_id keys only required on creation ->requirePresence('is_main', $isCreateMode, __('Field is required')) - ->requirePresence('client_id', $isCreateMode, __('Field is required')); + // When update or create request is called and all fields except is_main, something is wrong + ->add('is_main', 'notEmptyOtherFields', [ + 'rule' => function ($value, $context) { + return !(count($context['data']) === 1 && array_keys($context['data'])[0] === 'is_main'); + }, + 'message' => __('Request body is empty'), + ]) + ->requirePresence('client_id', $isCreateMode, __('Field is required')) + ; + if ((int)$noteValues['is_main'] === 1) { // If main note, the min length can be 0 as we can't delete it diff --git a/src/Domain/User/Data/UserResultData.php b/src/Domain/User/Data/UserResultData.php index f69abfa2..970d4b46 100644 --- a/src/Domain/User/Data/UserResultData.php +++ b/src/Domain/User/Data/UserResultData.php @@ -7,12 +7,12 @@ class UserResultData extends UserData { public ?Privilege $generalPrivilege = null; - // If authenticated user is allowed to change status + // If the authenticated user is allowed to change status public ?Privilege $statusPrivilege = null; - // If authenticated user is allowed to change the password without having to type in the old password + // If the authenticated user is allowed to change the password without having to type in the old password public ?Privilege $passwordWithoutVerificationPrivilege = null; - // If authenticated user is allowed to change role + // If the authenticated user is allowed to change role public ?Privilege $userRolePrivilege = null; // Authorization limits which entries are in the user role dropdown diff --git a/src/Domain/User/Service/Authorization/AuthorizedUserRoleFilterer.php b/src/Domain/User/Service/Authorization/AuthorizedUserRoleFilterer.php new file mode 100644 index 00000000..a0fc9227 --- /dev/null +++ b/src/Domain/User/Service/Authorization/AuthorizedUserRoleFilterer.php @@ -0,0 +1,47 @@ +userRoleFinderRepository->findAllUserRolesForDropdown(); + // Available user roles for dropdown and privilege + $grantedCreateUserRoles = []; + foreach ($allUserRoles as $roleId => $roleName) { + // If the role is already attributed to user the value is added so that it's displayed in the dropdown + if (($attributedUserRoleId !== null && $roleId === $attributedUserRoleId) + // Check if user role is granted + || $this->userPermissionVerifier->userRoleIsGranted($roleId, $attributedUserRoleId) === true + ) { + $grantedCreateUserRoles[$roleId] = $roleName; + } + } + + return $grantedCreateUserRoles; + } +} \ No newline at end of file diff --git a/src/Domain/User/Service/Authorization/UserAuthorizationChecker.php b/src/Domain/User/Service/Authorization/UserAuthorizationChecker.php deleted file mode 100644 index 2227d126..00000000 --- a/src/Domain/User/Service/Authorization/UserAuthorizationChecker.php +++ /dev/null @@ -1,364 +0,0 @@ -loggedInUserId = $this->userNetworkSessionData->userId ?? null; - } - - /** - * Check if authenticated user is allowed to create - * Important to have user role in the object. - * - * @param array $userValues - * - * @return bool - */ - public function isGrantedToCreate(array $userValues): bool - { - if ($this->loggedInUserId) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( - $this->loggedInUserId - ); - // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] - // * Lower hierarchy number means higher privileged role - $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); - - // Newcomer and advisor are not allowed to do anything from other users - only user edit his own profile - // Managing advisor may change users - if ($authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) { - // Managing advisors can do everything with users except setting a role higher than advisor - if ($userValues['user_role_id'] !== null - && $this->userRoleIsGranted( - $userValues['user_role_id'], - null, - $authenticatedUserRoleData, - $userRoleHierarchies - ) === true - ) { - return true; - } - - if ($userValues['user_role_id'] === null) { - return true; - } - } - } - // There is no need to check if user want to create his own user as he can't be logged in if the user doesn't exist - - $this->logger->notice( - 'User ' . $this->loggedInUserId . ' tried to create user but isn\'t allowed.' - ); - - return false; - } - - /** - * Check if authenticated user is allowed to assign given user role. - * - * @param int $userRoleId - * @param int|null $userRoleIdOfUserToMutate whenever possible user role of user to be changed has to be provided - * @param UserRoleData|null $authenticatedUserRoleData optional so that it can be called outside this class - * @param array|null $userRoleHierarchies optional so that it can be called outside this class - * - * @return bool - */ - public function userRoleIsGranted( - string|int $userRoleId, - null|string|int $userRoleIdOfUserToMutate, - ?UserRoleData $authenticatedUserRoleData = null, - ?array $userRoleHierarchies = null, - ): bool { - if ($this->loggedInUserId) { - // $authenticatedUserRoleData and $userRoleHierarchies passed as arguments if called inside this class - if ($authenticatedUserRoleData === null) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( - $this->loggedInUserId - ); - } - if ($userRoleHierarchies === null) { - // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] - // * Lower hierarchy number means higher privileged role - $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); - } - - $userRoleHierarchiesById = $this->userRoleFinderRepository->getUserRolesHierarchies(true); - - // Role higher (lower hierarchy number) than managing advisor may assign any role - if ($authenticatedUserRoleData->hierarchy < $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) { - return true; - } - - if ( // Managing advisor can only attribute roles with lower or equal privilege than advisor - $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value] - && $userRoleHierarchiesById[$userRoleId] >= $userRoleHierarchies[UserRole::ADVISOR->value] - // And managing advisor may only change advisors or newcomers - && ($userRoleIdOfUserToMutate === null - || $userRoleHierarchiesById[$userRoleIdOfUserToMutate] >= - $userRoleHierarchies[UserRole::ADVISOR->value]) - ) { - return true; - } - } - - return false; - } - - /** - * Logic to check if logged-in user is granted to update user. - * - * @param array $userDataToUpdate validated array with as key the column to - * update and value the new value. There may be one or multiple entries, - * depending on what the user wants to update - * @param string|int $userIdToUpdate - * @param bool $log log if forbidden (expected false when function is called for privilege setting) - * - * @return bool - */ - public function isGrantedToUpdate( - array $userDataToUpdate, - string|int $userIdToUpdate, - bool $log = true - ): bool { - $grantedUpdateKeys = []; - // Unset key id from data to update as is present in the array without the intention of being changed - unset($userDataToUpdate['id']); - if ($this->loggedInUserId) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( - $this->loggedInUserId - ); - $userToUpdateRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( - (int)$userIdToUpdate - ); - // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] - // * Lower hierarchy number means higher privileged role - $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); - - // Roles: newcomer < advisor < managing_advisor < administrator - // If logged-in hierarchy value is smaller or equal managing advisor - if ((($authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value] - // and that the user to change is has no role higher than advisor - && $userToUpdateRoleData->hierarchy >= $userRoleHierarchies[UserRole::ADVISOR->value]) - // or it's an admin which is allowed to change users with role - || $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::ADMIN->value]) // or user edits his own profile - || $this->loggedInUserId === (int)$userIdToUpdate - ) { - // Managing advisor cannot change other managing advisors or admins but admins can change themselves and everyone else - - // Things that managing advisor and owner user are allowed to change - if (array_key_exists('general_data', $userDataToUpdate)) { - // General data is the "main" data like first name, last name and email - $grantedUpdateKeys[] = 'general_data'; - } - if (array_key_exists('first_name', $userDataToUpdate)) { - $grantedUpdateKeys[] = 'first_name'; - } - if (array_key_exists('surname', $userDataToUpdate)) { - $grantedUpdateKeys[] = 'surname'; - } - if (array_key_exists('email', $userDataToUpdate)) { - $grantedUpdateKeys[] = 'email'; - } - if (array_key_exists('password_hash', $userDataToUpdate)) { - $grantedUpdateKeys[] = 'password_hash'; - } - if (array_key_exists('theme', $userDataToUpdate)) { - $grantedUpdateKeys[] = 'theme'; - } - if (array_key_exists('language', $userDataToUpdate)) { - $grantedUpdateKeys[] = 'language'; - } - // If new basic data fiel is added, add it to provider userUpdateAuthorizationCases() $basicDataChanges - // and invalid value to provider invalidUserUpdateCases() - - // Things that only managing_advisor and higher privileged are allowed to do with users (and not if own profile) - // If user is managing advisor we know by the parent if-statement that the user to change has not higher - // role than advisor - if ($authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) { - if (array_key_exists('status', $userDataToUpdate)) { - $grantedUpdateKeys[] = 'status'; - } - // Check that authenticated user is granted to attribute role - if (array_key_exists('user_role_id', $userDataToUpdate) && $this->userRoleIsGranted( - $userDataToUpdate['user_role_id'], - $userToUpdateRoleData->id, - $authenticatedUserRoleData, - $userRoleHierarchies - ) === true) { - $grantedUpdateKeys[] = 'user_role_id'; - } - - // There is a special case with passwords where the user can change his own password, but he needs to - // provide the old password. If password_without_verification is given as $userDataToUpdate it means - // that the authenticated user can change the password without the old password. - if (array_key_exists('password_without_verification', $userDataToUpdate) - // If user want to change his own password, the old password is required regardless of role - // so that nobody can change his password if the computer is left unattended and logged-in - // https://security.stackexchange.com/a/24292 - to change other passwords it would be best if - // the authenticated managing_advisor / admin password is asked instead of the old user password - // but this is too much for this project. - && $this->loggedInUserId !== (int)$userIdToUpdate) { - $grantedUpdateKeys[] = 'password_without_verification'; - } - } - // Owner user (profile edit) is not allowed to change its user role or status - } - } - - // If data that the user wanted to update and the grantedUpdateKeys are equal by having the same keys -> granted - foreach ($userDataToUpdate as $key => $value) { - // If at least one array key doesn't exist in $grantedUpdateKeys it means that user is not permitted - if (!in_array($key, $grantedUpdateKeys, true)) { - if ($log === true) { - $this->logger->notice( - 'User ' . $this->loggedInUserId . ' tried to update user but isn\'t allowed to change' . - $key . ' to "' . $value . '".' - ); - } - - return false; - } - } - - return true; - } - - /** - * Check if authenticated user is allowed to delete user. - * - * @param int $userIdToDelete - * @param bool $log log if forbidden (expected false when function is called for privilege setting) - * - * @return bool - */ - public function isGrantedToDelete( - int $userIdToDelete, - bool $log = true - ): bool { - if ($this->loggedInUserId) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( - $this->loggedInUserId - ); - $userToDeleteRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser($userIdToDelete); - - // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] - // * Lower hierarchy number means higher privileged role - $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); - - // Only managing_advisor and higher are allowed to delete user and only if the user is advisor or lower or their own - if (($authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value] - && ($userToDeleteRoleData->hierarchy >= $userRoleHierarchies[UserRole::ADVISOR->value] - || $userIdToDelete === $this->loggedInUserId)) - // or authenticated user is admin - || $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::ADMIN->value]) { - return true; - } - } - - if ($log === true) { - $this->logger->notice( - 'User ' . $this->loggedInUserId . ' tried to delete user but isn\'t allowed.' - ); - } - - return false; - } - - /** - * Check if authenticated user is allowed to read user. - * - * @param int|null $userIdToRead null when check for all users - * @param bool $log log if forbidden (expected false when function is called for privilege setting) - * - * @return bool - */ - public function isGrantedToRead( - ?int $userIdToRead = null, - bool $log = true - ): bool { - if ($this->loggedInUserId) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( - $this->loggedInUserId - ); - // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] - // * Lower hierarchy number means higher privileged role - $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); - - // Only managing advisor and higher privilege are allowed to see other users - if ($authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value] - // or user wants to view his own profile - || $this->loggedInUserId === $userIdToRead) { - return true; - } - } - - if ($log === true) { - $this->logger->notice( - 'User ' . $this->loggedInUserId . ' tried to read user but isn\'t allowed.' - ); - } - - return false; - } - - /** - * Check if authenticated user is allowed to read user activity. - * - * @param int $userIdToRead - * @param bool $log log if forbidden - * - * @return bool - */ - public function isGrantedToReadUserActivity( - int $userIdToRead, - bool $log = true - ): bool { - if ($this->loggedInUserId) { - $authenticatedUserRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser( - $this->loggedInUserId - ); - // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] - // * Lower hierarchy number means higher privileged role - $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); - - $userToReadRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser($userIdToRead); - - // Only managing advisor are allowed to see users activity but only if target user role is not higher than also managing advisor - if (($authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value] - && $userToReadRoleData->hierarchy >= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) - // or authenticated user is admin - || $authenticatedUserRoleData->hierarchy <= $userRoleHierarchies[UserRole::ADMIN->value] - // or user wants to view his own activity - || $this->loggedInUserId === $userIdToRead) { - return true; - } - } - - if ($log === true) { - $this->logger->notice( - "User $this->loggedInUserId tried to read activity of user $userIdToRead but isn't allowed." - ); - } - - return false; - } -} diff --git a/src/Domain/User/Service/Authorization/UserAuthorizationGetter.php b/src/Domain/User/Service/Authorization/UserAuthorizationGetter.php deleted file mode 100644 index 2094d82c..00000000 --- a/src/Domain/User/Service/Authorization/UserAuthorizationGetter.php +++ /dev/null @@ -1,108 +0,0 @@ - 1) { - return Privilege::UPDATE; - } - - return Privilege::READ; - } - - /** - * Returns all roles that authenticated user is allowed to choose when - * creating a new user. - * - * Note: this is not performant at all as for each user all user roles changes - * have to be tested and isGrantedToUpdate makes 4 sql requests each time meaning - * that for 10 users and 4 roles and 4 requests in the function there will be - * more than 120 sql requests so if optimisations have to be made, here is a good place - * to start. It is like this for simplicity as there will not be a lot of users - * anyway and the user list action is quite rare and limited to some users. - * - * @param int|null $attributedUserRoleId - * - * @return array - */ - public function getAuthorizedUserRoles(?int $attributedUserRoleId = null): array - { - $allUserRoles = $this->userRoleFinderRepository->findAllUserRolesForDropdown(); - // Available user roles for dropdown and privilege - $grantedCreateUserRoles = []; - foreach ($allUserRoles as $roleId => $roleName) { - // If the role is already attributed to user the value is added so that it's displayed in the dropdown - if (($attributedUserRoleId !== null && $roleId === $attributedUserRoleId) - // Check if user role is granted - || $this->userAuthorizationChecker->userRoleIsGranted($roleId, $attributedUserRoleId) === true - ) { - $grantedCreateUserRoles[$roleId] = $roleName; - } - } - - return $grantedCreateUserRoles; - } - - /** - * Checks if authenticated user is allowed to update or read given column - * or delete user. - * - * @param int $userId - * @param string|null $column - * - * @return Privilege - */ - public function getMutationPrivilegeForUserColumn(int $userId, ?string $column = null): Privilege - { - // Usually I'd check first against the highest privilege and if allowed, directly return otherwise continue down the chain - // But some authorizations are limited per column, so when a column is provided, the update privilege is checked first - - // Check if given value may be updated by authenticated user (value does not matter as keys are relevant) - $updatePrivilege = Privilege::NONE; - if ($column !== null - && $this->userAuthorizationChecker->isGrantedToUpdate([$column => 'value'], $userId, false) - ) { - $updatePrivilege = Privilege::UPDATE; - } - // If update privilege is set or there was no column, check for delete - if (($updatePrivilege === Privilege::UPDATE || $column === null) - && $this->userAuthorizationChecker->isGrantedToDelete($userId, false) - ) { - return Privilege::DELETE; - } - // If delete privilege wasn't returned and the authenticated is allowed to update, return update privilege - if ($updatePrivilege === Privilege::UPDATE) { - return $updatePrivilege; - } - - if ($this->userAuthorizationChecker->isGrantedToRead($userId, false)) { - return Privilege::READ; - } - - return Privilege::NONE; - } -} diff --git a/src/Domain/User/Service/Authorization/UserPermissionVerifier.php b/src/Domain/User/Service/Authorization/UserPermissionVerifier.php new file mode 100644 index 00000000..d4dcf69e --- /dev/null +++ b/src/Domain/User/Service/Authorization/UserPermissionVerifier.php @@ -0,0 +1,375 @@ +loggedInUserId = $this->userNetworkSessionData->userId ?? null; + } + + /** + * Check if the authenticated user is allowed to create + * Important to have user role in the object. + * + * @param array $userValues + * + * @return bool + */ + public function isGrantedToCreate(array $userValues): bool + { + if (!$this->loggedInUserId) { + $this->logger->error( + 'loggedInUserId not set while authorization check isGrantedToCreate: ' + . json_encode($userValues, JSON_PARTIAL_OUTPUT_ON_ERROR) + ); + return false; + } + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( + $this->loggedInUserId + ); + // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] + // * Lower hierarchy number means higher privileged role + $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); + + // Newcomer and advisor are not allowed to do anything from other users - only user edit his own profile + // Managing advisor may change users + if ($authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) { + // Managing advisors can do everything with users except setting a role higher than advisor + if ($userValues['user_role_id'] !== null + && $this->userRoleIsGranted( + $userValues['user_role_id'], + null, + $authenticatedUserRoleHierarchy, + $userRoleHierarchies + ) === true + ) { + return true; + } + + if ($userValues['user_role_id'] === null) { + return true; + } + } + // There is no need to check if user wants to create his own user as he can't be logged in if the user doesn't exist + + $this->logger->notice( + 'User ' . $this->loggedInUserId . ' tried to create user but isn\'t allowed.' + ); + + return false; + } + + /** + * Check if the authenticated user is allowed to assign user role. + * + * @param string|int $userRoleId + * @param string|int|null $userRoleIdOfUserToMutate whenever possible user role of user to be changed has to be provided + * @param int|null $authenticatedUserRoleHierarchy optional so that it can be called outside this class + * @param array|null $userRoleHierarchies optional so that it can be called outside this class + * + * @return bool + */ + public function userRoleIsGranted( + string|int $userRoleId, + null|string|int $userRoleIdOfUserToMutate, + ?int $authenticatedUserRoleHierarchy = null, + ?array $userRoleHierarchies = null, + ): bool { + if (!$this->loggedInUserId) { + $this->logger->error( + 'loggedInUserId not set while authorization check that user role is granted $userRoleIdOfUserToMutate: ' + . $userRoleIdOfUserToMutate + ); + return false; + } + // $authenticatedUserRoleData and $userRoleHierarchies passed as arguments if called inside this class + if ($authenticatedUserRoleHierarchy === null) { + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( + $this->loggedInUserId + ); + } + if ($userRoleHierarchies === null) { + // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] + // * Lower hierarchy number means higher privileged role + $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); + } + + $userRoleHierarchiesById = $this->userRoleFinderRepository->getUserRolesHierarchies(true); + + // Role higher (lower hierarchy number) than managing advisor may assign any role + if ($authenticatedUserRoleHierarchy < $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) { + return true; + } + + if ( // Managing advisor can only attribute roles with lower or equal privilege than advisor + $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value] + && $userRoleHierarchiesById[$userRoleId] >= $userRoleHierarchies[UserRole::ADVISOR->value] + // And managing advisor may only change advisors or newcomers + && ($userRoleIdOfUserToMutate === null + || $userRoleHierarchiesById[$userRoleIdOfUserToMutate] >= + $userRoleHierarchies[UserRole::ADVISOR->value]) + ) { + return true; + } + + return false; + } + + /** + * Logic to check if logged-in user is granted to update user. + * This function has a high cyclomatic complexity due to the many if-statements, + * but for now I find it more readable than splitting it up into multiple functions. + * + * @param array $userDataToUpdate validated array with as key the column to + * update and value the new value. There may be one or multiple entries, + * depending on what the user wants to update + * @param string|int $userIdToUpdate + * @param bool $log log if forbidden (expected false when function is called for privilege setting) + * + * @return bool + */ + public function isGrantedToUpdate(array $userDataToUpdate, string|int $userIdToUpdate, bool $log = true): bool + { + // Unset key id from data to update as is present in the array without the intention of being changed + unset($userDataToUpdate['id']); + if (!$this->loggedInUserId) { + $this->logger->error( + 'loggedInUserId not while user update authorization check' . + json_encode($userDataToUpdate, JSON_PARTIAL_OUTPUT_ON_ERROR) + ); + return false; + } + $grantedUpdateKeys = []; + + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( + $this->loggedInUserId + ); + $userToUpdateRoleData = $this->userRoleFinderRepository->getUserRoleDataFromUser((int)$userIdToUpdate); + // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] + // * Lower hierarchy number means higher privileged role + $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); + + // Only managing advisor or higher privileged can change users + if ((($authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value] + // but only if user to change is advisor or lower + && $userToUpdateRoleData->hierarchy >= $userRoleHierarchies[UserRole::ADVISOR->value]) + // if user role is higher privileged than managing advisor (admin) -> authorized + || $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::ADMIN->value]) + // or if the user edits his own profile + || $this->loggedInUserId === (int)$userIdToUpdate + ) { + // Things that managing advisor and owner user are allowed to change + // Personal info is the "main" data like first name, last name and email + $grantedUpdateKeys[] = 'personal_info'; + $grantedUpdateKeys[] = 'first_name'; + $grantedUpdateKeys[] = 'surname'; + $grantedUpdateKeys[] = 'email'; + $grantedUpdateKeys[] = 'password_hash'; + $grantedUpdateKeys[] = 'theme'; + $grantedUpdateKeys[] = 'language'; + // If a new basic data field is added, it has to be added to provider userUpdateAuthorizationCases() + // $basicDataChanges variable and invalid value to provider invalidUserUpdateCases() + + // Things that only managing_advisor and higher privileged are allowed to change + // If the user is managing advisor we know by the parent if-statement + // that the user to change has not higher role than advisor + if ($authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) { + $grantedUpdateKeys[] = 'status'; + + // Check that authenticated user is granted to attribute role if that's requested + if (array_key_exists('user_role_id', $userDataToUpdate) + && $this->userRoleIsGranted( + $userDataToUpdate['user_role_id'], + $userToUpdateRoleData->id, + $authenticatedUserRoleHierarchy, + $userRoleHierarchies + ) === true) { + $grantedUpdateKeys[] = 'user_role_id'; + } + + // There is a special case with passwords where the user can change his own password, but he needs to + // provide the old password. If password_without_verification is added to $grantedUpdateKeys it means + // that the authenticated user can change the password without the old password. + // But if the user wants to change his own password, the old password is required regardless of role + // so that nobody can change his password if the computer is left unattended and logged-in + // https://security.stackexchange.com/a/24292 - to change other passwords it would be best if + // the authenticated managing_advisor / admin password is asked instead of the old user password + // but this is too much for this project. + if ($this->loggedInUserId !== (int)$userIdToUpdate) { + $grantedUpdateKeys[] = 'password_without_verification'; + } + } + // Owner user (profile edit) is not allowed to change its user role or status + } + + + // Check that the data that the user wanted to update is in $grantedUpdateKeys array + foreach ($userDataToUpdate as $key => $value) { + // If at least one array key doesn't exist in $grantedUpdateKeys it means that user is not permitted + if (!in_array($key, $grantedUpdateKeys, true)) { + if ($log === true) { + $this->logger->notice( + 'User ' . $this->loggedInUserId . ' tried to update user but isn\'t allowed to change' . + $key . ' to "' . $value . '".' + ); + } + + return false; + } + } + // All keys in $userDataToUpdate are in $grantedUpdateKeys + return true; + } + + /** + * Check if authenticated user is allowed to delete user. + * + * @param int $userIdToDelete + * @param bool $log log if forbidden (expected false when function is called for privilege setting) + * + * @return bool + */ + public function isGrantedToDelete( + int $userIdToDelete, + bool $log = true + ): bool { + if (!$this->loggedInUserId) { + $this->logger->error( + 'loggedInUserId not set while authorization check isGrantedToDelete $userIdToDelete: ' + . $userIdToDelete + ); + return false; + } + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( + $this->loggedInUserId + ); + $userToDeleteRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId($userIdToDelete); + + // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] + // * Lower hierarchy number means higher privileged role + $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); + + // Only managing_advisor and higher are allowed to delete user and only if the user is advisor or lower or their own + if (($authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value] + && ($userToDeleteRoleHierarchy >= $userRoleHierarchies[UserRole::ADVISOR->value] + || $userIdToDelete === $this->loggedInUserId)) + // or authenticated user is admin + || $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::ADMIN->value]) { + return true; + } + + + if ($log === true) { + $this->logger->notice( + 'User ' . $this->loggedInUserId . ' tried to delete user but isn\'t allowed.' + ); + } + + return false; + } + + /** + * Check if authenticated user is allowed to read user. + * + * @param int|null $userIdToRead null when check for all users + * @param bool $log log if forbidden (expected false when function is called for privilege setting) + * + * @return bool + */ + public function isGrantedToRead(?int $userIdToRead = null, bool $log = true): bool + { + if (!$this->loggedInUserId) { + $this->logger->error( + 'loggedInUserId not set while authorization check isGrantedToRead $userIdToRead: ' + . $userIdToRead + ); + return false; + } + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( + $this->loggedInUserId + ); + // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] + // * Lower hierarchy number means higher privileged role + $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); + + // Only managing advisor and higher privilege are allowed to see other users + // If the user role hierarchy of the authenticated user is lower or equal + // than from the managing advisor -> authorized + if ($authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value] + // or user wants to view his own profile + || $this->loggedInUserId === $userIdToRead) { + return true; + } + + if ($log === true) { + $this->logger->notice('User ' . $this->loggedInUserId . ' tried to read user but isn\'t allowed.'); + } + + return false; + } + + /** + * Check if the authenticated user is allowed to read user activity. + * + * @param int $userIdToRead + * @param bool $log log if forbidden + * + * @return bool + */ + public function isGrantedToReadUserActivity( + int $userIdToRead, + bool $log = true + ): bool { + if (!$this->loggedInUserId) { + $this->logger->error( + 'loggedInUserId not set while authorization check isGrantedToReadUserActivity $userIdToRead: ' + . $userIdToRead + ); + return false; + } + + $authenticatedUserRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId( + $this->loggedInUserId + ); + // Returns array with role name as key and hierarchy as value [role_name => hierarchy_int] + // * Lower hierarchy number means higher privileged role + $userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies(); + + $userToReadRoleHierarchy = $this->userRoleFinderRepository->getRoleHierarchyByUserId($userIdToRead); + + // Only managing advisors are allowed to see user activity, but only if target user role is not higher than also managing advisor + if (($authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value] + && $userToReadRoleHierarchy >= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]) + // or authenticated user is admin + || $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::ADMIN->value] + // or user wants to view his own activity + || $this->loggedInUserId === $userIdToRead) { + return true; + } + + if ($log === true) { + $this->logger->notice( + "User $this->loggedInUserId tried to read activity of user $userIdToRead but isn't allowed." + ); + } + + return false; + } +} diff --git a/src/Domain/User/Service/Authorization/UserPrivilegeDeterminer.php b/src/Domain/User/Service/Authorization/UserPrivilegeDeterminer.php new file mode 100644 index 00000000..78639093 --- /dev/null +++ b/src/Domain/User/Service/Authorization/UserPrivilegeDeterminer.php @@ -0,0 +1,73 @@ + 1) { + return Privilege::UPDATE; + } + + return Privilege::READ; + } + + /** + * Checks if authenticated user is allowed to update or read given column + * or delete user. + * + * @param int $userId + * @param string|null $column + * + * @return Privilege + */ + public function determineMutationPrivilege(int $userId, ?string $column = null): Privilege + { + // Usually I'd check first against the highest privilege and if allowed, directly return otherwise continue + // down the chain. But some authorizations are limited per column, so when a $column is provided, + // the update privilege is checked first + + // Check if given value may be updated by authenticated user (value does not matter as keys are relevant) + $updatePrivilege = Privilege::NONE; + if ($column !== null + && $this->userPermissionVerifier->isGrantedToUpdate([$column => 'value'], $userId, false) + ) { + $updatePrivilege = Privilege::UPDATE; + } + // If update privilege is set or there was no column, check for "delete" + if (($updatePrivilege === Privilege::UPDATE || $column === null) + && $this->userPermissionVerifier->isGrantedToDelete($userId, false) + ) { + return Privilege::DELETE; + } + // If delete privilege wasn't returned, and the authenticated is allowed to update, return update privilege + if ($updatePrivilege === Privilege::UPDATE) { + return $updatePrivilege; + } + + if ($this->userPermissionVerifier->isGrantedToRead($userId, false)) { + return Privilege::READ; + } + + return Privilege::NONE; + } +} diff --git a/src/Domain/User/Service/UserCreator.php b/src/Domain/User/Service/UserCreator.php index 70a672a7..fe09e801 100644 --- a/src/Domain/User/Service/UserCreator.php +++ b/src/Domain/User/Service/UserCreator.php @@ -12,7 +12,7 @@ use App\Domain\User\Enum\UserRole; use App\Domain\User\Enum\UserStatus; use App\Domain\User\Repository\UserCreatorRepository; -use App\Domain\User\Service\Authorization\UserAuthorizationChecker; +use App\Domain\User\Service\Authorization\UserPermissionVerifier; use App\Domain\UserActivity\Service\UserActivityLogger; use Symfony\Component\Mailer\Exception\TransportExceptionInterface; @@ -21,7 +21,7 @@ final class UserCreator public function __construct( private readonly UserValidator $userValidator, private readonly SecurityEmailChecker $emailSecurityChecker, - private readonly UserAuthorizationChecker $userAuthorizationChecker, + private readonly UserPermissionVerifier $userPermissionVerifier, private readonly UserCreatorRepository $userCreatorRepository, private readonly VerificationTokenCreator $verificationTokenCreator, private readonly RegistrationMailSender $registrationMailer, @@ -44,7 +44,7 @@ public function __construct( public function createUser(array $userValues, ?string $captcha = null, array $queryParams = []): bool|int { // Before validation, check if authenticated user is authorized to create user with the given data - if ($this->userAuthorizationChecker->isGrantedToCreate($userValues)) { + if ($this->userPermissionVerifier->isGrantedToCreate($userValues)) { // * Validation has to be done AFTER authorization check // to not reveal potential sensitive infos such as from the validation messages $this->userValidator->validateUserValues($userValues); diff --git a/src/Domain/User/Service/UserDeleter.php b/src/Domain/User/Service/UserDeleter.php index ce9994b5..c4990b5d 100644 --- a/src/Domain/User/Service/UserDeleter.php +++ b/src/Domain/User/Service/UserDeleter.php @@ -5,7 +5,7 @@ use App\Domain\Authentication\Exception\ForbiddenException; use App\Domain\User\Enum\UserActivity; use App\Domain\User\Repository\UserDeleterRepository; -use App\Domain\User\Service\Authorization\UserAuthorizationChecker; +use App\Domain\User\Service\Authorization\UserPermissionVerifier; use App\Domain\UserActivity\Service\UserActivityLogger; use Odan\Session\SessionInterface; use Psr\Log\LoggerInterface; @@ -16,7 +16,7 @@ public function __construct( private readonly LoggerInterface $logger, private readonly UserDeleterRepository $userDeleterRepository, private readonly SessionInterface $session, - private readonly UserAuthorizationChecker $userAuthorizationChecker, + private readonly UserPermissionVerifier $userPermissionVerifier, private readonly UserActivityLogger $userActivityLogger, ) { } @@ -33,7 +33,7 @@ public function __construct( public function deleteUser(int $userIdToDelete): bool { // Check if it's admin or if it's its own post - if ($this->userAuthorizationChecker->isGrantedToDelete($userIdToDelete)) { + if ($this->userPermissionVerifier->isGrantedToDelete($userIdToDelete)) { $isDeleted = $this->userDeleterRepository->deleteUserById($userIdToDelete); if ($isDeleted) { $this->userActivityLogger->logUserActivity(UserActivity::DELETED, 'user', $userIdToDelete); diff --git a/src/Domain/User/Service/UserFinder.php b/src/Domain/User/Service/UserFinder.php index a882d604..db129621 100644 --- a/src/Domain/User/Service/UserFinder.php +++ b/src/Domain/User/Service/UserFinder.php @@ -7,15 +7,17 @@ use App\Domain\User\Data\UserData; use App\Domain\User\Data\UserResultData; use App\Domain\User\Repository\UserFinderRepository; -use App\Domain\User\Service\Authorization\UserAuthorizationChecker; -use App\Domain\User\Service\Authorization\UserAuthorizationGetter; +use App\Domain\User\Service\Authorization\AuthorizedUserRoleFilterer; +use App\Domain\User\Service\Authorization\UserPermissionVerifier; +use App\Domain\User\Service\Authorization\UserPrivilegeDeterminer; class UserFinder { public function __construct( private readonly UserFinderRepository $userFinderRepository, - private readonly UserAuthorizationGetter $userAuthorizationGetter, - private readonly UserAuthorizationChecker $userAuthorizationChecker, + private readonly UserPrivilegeDeterminer $userPrivilegeDeterminer, + private readonly AuthorizedUserRoleFilterer $authorizedUserRoleFilterer, + private readonly UserPermissionVerifier $userPermissionVerifier, ) { } @@ -28,23 +30,23 @@ public function findAllUsersResultDataForList(): array foreach ($userResultArray as $key => $userResultData) { // Check if authenticated user is allowed to read user - if ($this->userAuthorizationChecker->isGrantedToRead($userResultData->id)) { + if ($this->userPermissionVerifier->isGrantedToRead($userResultData->id)) { // Authorization limits which entries are in the user role dropdown - $userResultData->availableUserRoles = $this->userAuthorizationGetter->getAuthorizedUserRoles( + $userResultData->availableUserRoles = $this->authorizedUserRoleFilterer->filterAuthorizedUserRoles( $userResultData->userRoleId ); - $userResultData->userRolePrivilege = $this->userAuthorizationGetter->getUserRoleAttributionPrivilege( + $userResultData->userRolePrivilege = $this->userPrivilegeDeterminer->determineUserRoleAssignmentPrivilege( $userResultData->availableUserRoles ); // Check if user is allowed to change status - $userResultData->statusPrivilege = $this->userAuthorizationGetter->getMutationPrivilegeForUserColumn( + $userResultData->statusPrivilege = $this->userPrivilegeDeterminer->determineMutationPrivilege( (int)$userResultData->id, 'status', ); - // General data privilege like first name, email and so on no needed for list - // $userResultData->generalPrivilege = $this->userAuthorizationGetter->getUpdatePrivilegeForUserColumn( - // 'general_data', $userResultData->id ); + // Personal info privilege like first name, email and so on no needed for list + // $userResultData->generalPrivilege = $this->userPermissionVerifier->getUpdatePrivilegeForUserColumn( + // 'personal_info', $userResultData->id ); } else { unset($userResultArray[$key]); } @@ -69,37 +71,37 @@ public function findUserById(string|int|null $id): UserData * * @param int $id * + * @return UserResultData * @throws \Exception * - * @return UserResultData */ public function findUserReadResult(int $id): UserResultData { - if ($this->userAuthorizationChecker->isGrantedToRead($id)) { + if ($this->userPermissionVerifier->isGrantedToRead($id)) { $userRow = $this->userFinderRepository->findUserById($id); if (!empty($userRow)) { $userResultData = new UserResultData($userRow); // Status privilege - $userResultData->statusPrivilege = $this->userAuthorizationGetter->getMutationPrivilegeForUserColumn( + $userResultData->statusPrivilege = $this->userPrivilegeDeterminer->determineMutationPrivilege( $id, 'status', ); // Available user roles for dropdown and privilege - $userResultData->availableUserRoles = $this->userAuthorizationGetter->getAuthorizedUserRoles( + $userResultData->availableUserRoles = $this->authorizedUserRoleFilterer->filterAuthorizedUserRoles( $userResultData->userRoleId ); - $userResultData->userRolePrivilege = $this->userAuthorizationGetter->getUserRoleAttributionPrivilege( + $userResultData->userRolePrivilege = $this->userPrivilegeDeterminer->determineUserRoleAssignmentPrivilege( $userResultData->availableUserRoles ); - // General data privilege like first name, email and so on - $userResultData->generalPrivilege = $this->userAuthorizationGetter->getMutationPrivilegeForUserColumn( + // Personal info privilege like first name, email and so on + $userResultData->generalPrivilege = $this->userPrivilegeDeterminer->determineMutationPrivilege( $id, - 'general_data', + 'personal_info', ); // Password change without verification of old password - $userResultData->passwordWithoutVerificationPrivilege = $this->userAuthorizationGetter-> - getMutationPrivilegeForUserColumn($id, 'password_without_verification'); + $userResultData->passwordWithoutVerificationPrivilege = $this->userPrivilegeDeterminer-> + determineMutationPrivilege($id, 'password_without_verification'); return $userResultData; } diff --git a/src/Domain/User/Service/UserUpdater.php b/src/Domain/User/Service/UserUpdater.php index eaa42ef8..e27508c4 100644 --- a/src/Domain/User/Service/UserUpdater.php +++ b/src/Domain/User/Service/UserUpdater.php @@ -6,7 +6,7 @@ use App\Domain\Exception\InvalidOperationException; use App\Domain\User\Enum\UserActivity; use App\Domain\User\Repository\UserUpdaterRepository; -use App\Domain\User\Service\Authorization\UserAuthorizationChecker; +use App\Domain\User\Service\Authorization\UserPermissionVerifier; use App\Domain\UserActivity\Service\UserActivityLogger; use Psr\Log\LoggerInterface; @@ -14,7 +14,7 @@ final class UserUpdater { public function __construct( private readonly UserValidator $userValidator, - private readonly UserAuthorizationChecker $userAuthorizationChecker, + private readonly UserPermissionVerifier $userPermissionVerifier, private readonly UserUpdaterRepository $userUpdaterRepository, private readonly UserActivityLogger $userActivityLogger, private readonly LoggerInterface $logger, @@ -41,7 +41,7 @@ public function updateUser(int $userIdToChange, array $userValues): bool unset($userValues['id']); // Check if it's admin or if it's its own user - if ($this->userAuthorizationChecker->isGrantedToUpdate($userValues, $userIdToChange)) { + if ($this->userPermissionVerifier->isGrantedToUpdate($userValues, $userIdToChange)) { // User values to change (cannot use object as unset values would be "null" and remove values in db) $validUpdateData = []; // Additional check (next to malformed body in action) to be sure that only columns that may be updated are sent to the database diff --git a/src/Domain/User/Service/UserUtilFinder.php b/src/Domain/User/Service/UserUtilFinder.php index 3845e58c..237dba42 100644 --- a/src/Domain/User/Service/UserUtilFinder.php +++ b/src/Domain/User/Service/UserUtilFinder.php @@ -4,12 +4,12 @@ use App\Domain\User\Enum\UserLang; use App\Domain\User\Enum\UserStatus; -use App\Domain\User\Service\Authorization\UserAuthorizationGetter; +use App\Domain\User\Service\Authorization\AuthorizedUserRoleFilterer; class UserUtilFinder { public function __construct( - private readonly UserAuthorizationGetter $userAuthorizationGetter, + private readonly AuthorizedUserRoleFilterer $authorizedUserRoleFilterer, ) { } @@ -21,7 +21,7 @@ public function __construct( public function findUserDropdownValues(): array { return [ - 'userRoles' => $this->userAuthorizationGetter->getAuthorizedUserRoles(), + 'userRoles' => $this->authorizedUserRoleFilterer->filterAuthorizedUserRoles(), 'statuses' => UserStatus::toTranslatedNamesArray(), 'languages' => UserLang::toArray(), ]; diff --git a/src/Domain/UserActivity/Service/UserActivityFinder.php b/src/Domain/UserActivity/Service/UserActivityFinder.php index 76abc48f..8d46401a 100644 --- a/src/Domain/UserActivity/Service/UserActivityFinder.php +++ b/src/Domain/UserActivity/Service/UserActivityFinder.php @@ -3,7 +3,7 @@ namespace App\Domain\UserActivity\Service; use App\Domain\User\Repository\UserFinderRepository; -use App\Domain\User\Service\Authorization\UserAuthorizationChecker; +use App\Domain\User\Service\Authorization\UserPermissionVerifier; use App\Domain\UserActivity\Repository\UserActivityRepository; use IntlDateFormatter; use InvalidArgumentException; @@ -13,7 +13,7 @@ class UserActivityFinder { public function __construct( - private readonly UserAuthorizationChecker $userAuthorizationChecker, + private readonly UserPermissionVerifier $userPermissionVerifier, private readonly RouteParserInterface $routeParser, private readonly UserFinderRepository $userFinderRepository, private readonly UserActivityRepository $userActivityRepository, @@ -52,7 +52,7 @@ private function findUserActivitiesGroupedByDate(int|string|array $userIds): arr } $grantedUserIds = []; foreach ($userIds as $userId) { - if ($this->userAuthorizationChecker->isGrantedToReadUserActivity((int)$userId)) { + if ($this->userPermissionVerifier->isGrantedToReadUserActivity((int)$userId)) { $grantedUserIds[] = (int)$userId; } } diff --git a/templates/client/client-read.html.php b/templates/client/client-read.html.php index b5af3920..fa7a360c 100644 --- a/templates/client/client-read.html.php +++ b/templates/client/client-read.html.php @@ -38,7 +38,7 @@
mainDataPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> + if ($clientAggregate->generalPrivilege->hasPrivilege(Privilege::UPDATE)) { ?>
mainDataPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> + if ($clientAggregate->generalPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> Editlocation || $clientAggregate->phone || $clientAggregate->email ? '' : 'opacity: 0;' ?>"> mainDataPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> + if ($clientAggregate->generalPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> Edit @@ -147,7 +147,7 @@ class="personal-info-icon default-icon" alt="birthdate"> data-field-element="span" data-hide-if-empty="true"> mainDataPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> + if ($clientAggregate->generalPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> Edit @@ -174,7 +174,7 @@ class="personal-info-icon default-icon" alt="sex"> data-field-element="select" data-hide-if-empty="true"> mainDataPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> + if ($clientAggregate->generalPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> Edit @@ -204,7 +204,7 @@ class="personal-info-icon default-icon" alt="location"> data-field-element="a-span" data-hide-if-empty="true"> mainDataPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> + if ($clientAggregate->generalPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> Edit @@ -223,7 +223,7 @@ class="personal-info-icon default-icon" alt="phone"> data-field-element="a-span" data-hide-if-empty="true"> mainDataPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> + if ($clientAggregate->generalPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> Edit @@ -242,7 +242,7 @@ class="personal-info-icon default-icon" alt="email"> data-field-element="a-span" data-hide-if-empty="true"> mainDataPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> + if ($clientAggregate->generalPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> Edit @@ -262,7 +262,7 @@ class="personal-info-icon default-icon" alt="vigilance_level"> data-field-element="select" data-hide-if-empty="true"> mainDataPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> + if ($clientAggregate->generalPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> Edit @@ -286,13 +286,13 @@ class="contenteditable-edit-icon cursor-pointer" alt="Edit"
mainDataPrivilege->hasPrivilege(Privilege::UPDATE)) { ?> + if ($clientAggregate->generalPrivilege->hasPrivilege(Privilege::UPDATE)) { ?>
add info - mainDataPrivilege->hasPrivilege(Privilege::DELETE) ? + generalPrivilege->hasPrivilege(Privilege::DELETE) ? ($clientAggregate->deletedAt ? 'undelete' : 'language?->value; + $langRadioButtonDisabled = $user->generalPrivilege->hasPrivilege(Privilege::UPDATE) ? '' : 'disabled'; ?>

diff --git a/tests/Integration/Client/ClientListActionTest.php b/tests/Integration/Client/ClientListActionTest.php index b3a2d0a1..d27314f9 100644 --- a/tests/Integration/Client/ClientListActionTest.php +++ b/tests/Integration/Client/ClientListActionTest.php @@ -166,7 +166,7 @@ public function testClientListWithFilterAction( 'clientStatusId' => $clientRow['client_status_id'], 'age' => (new \DateTime())->diff(new \DateTime($clientRow['birthdate']))->y, // Below not asserted as this test is about filtering not authorization - // 'mainDataPrivilege' => null + // 'personalInfoPrivilege' => null // 'clientStatusPrivilege' => 'NONE' // 'assignedUserPrivilege' => 'NONE' // 'noteCreatePrivilege' => null diff --git a/tests/Integration/Note/NoteListActionTest.php b/tests/Integration/Note/NoteListActionTest.php index 34ab3985..597dbe81 100644 --- a/tests/Integration/Note/NoteListActionTest.php +++ b/tests/Integration/Note/NoteListActionTest.php @@ -125,7 +125,7 @@ public function testNoteListActionAuthorization( 'updatedAt' => $dateFormatter->format(new \DateTime($noteData['updated_at'])), 'userFullName' => $userLinkedToNoteRow['first_name'] . ' ' . $userLinkedToNoteRow['surname'], 'clientFullName' => null, - // Has to match privilege from NoteAuthorizationGetter.php (rules are in NoteAuthorizationChecker.php) + // Has to match privilege from notePrivilegeDeterminer.php (rules are in notePermissionVerifier.php) 'privilege' => $expectedResult['privilege']->value, 'isClientMessage' => 0, ]; diff --git a/tests/Provider/Note/NoteProvider.php b/tests/Provider/Note/NoteProvider.php index 8e9f8126..c7b22ef5 100644 --- a/tests/Provider/Note/NoteProvider.php +++ b/tests/Provider/Note/NoteProvider.php @@ -437,8 +437,8 @@ public static function invalidNoteUpdateProvider(): array 'message' => 'Validation error', 'data' => [ 'errors' => [ - 'message' => [ - 0 => 'Field is required', + 'is_main' => [ + 0 => 'Request body is empty', ], ], ], diff --git a/tests/Unit/Authentication/RegisterTokenVerifierTest.php b/tests/Unit/Authentication/RegisterTokenVerifierTest.php index 2504716a..2186c6dd 100644 --- a/tests/Unit/Authentication/RegisterTokenVerifierTest.php +++ b/tests/Unit/Authentication/RegisterTokenVerifierTest.php @@ -45,7 +45,7 @@ class RegisterTokenVerifierTest extends TestCase * * @throws ContainerExceptionInterface|NotFoundExceptionInterface|\Exception */ - public function testGetUserIdIfRegisterTokenIsValidAlreadyVerified( + public function testVerifyRegisterTokenAndGetUserIdAlreadyVerified( UserVerificationData $verification, string $clearTextToken ): void { @@ -63,7 +63,7 @@ public function testGetUserIdIfRegisterTokenIsValidAlreadyVerified( $this->expectExceptionMessage('User status is not "' . UserStatus::Unverified->value . '"'); // Call function under test - $this->container->get(RegisterTokenVerifier::class)->getUserIdIfRegisterTokenIsValid( + $this->container->get(RegisterTokenVerifier::class)->verifyRegisterTokenAndGetUserId( $verification->id, $clearTextToken );