Skip to content

Commit

Permalink
Authorization class and functions renaming [SLE-192][SLE-66]
Browse files Browse the repository at this point in the history
  • Loading branch information
samuelgfeller committed Dec 1, 2023
1 parent 12fb40d commit 9b6d5c7
Show file tree
Hide file tree
Showing 62 changed files with 1,127 additions and 1,041 deletions.
2 changes: 1 addition & 1 deletion docs/implementation-examples.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
2 changes: 1 addition & 1 deletion docs/testing/testing-cheatsheet.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
12 changes: 6 additions & 6 deletions public/assets/client/note/client-read-template-note.html.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,27 @@ export function getNoteHtml(note) {
<a href="${window.location.href}#note-${id}-container"
class="note-left-side-label no-style-a">${createdAt}</a>
${/*Show active eye icon if hidden*/ hidden === 1 || hidden === '1' ? `<img
class="btn-above-note hide-note-btn ${userHasPrivilegeTo(privilege, 'U') ? `` : `
class="btn-above-note hide-note-btn ${privilege.includes('U') ? `` : `
not-clickable` /*Add not clickable class when not allowed to update*/}" alt="hide"
style="display: inline-block" src="assets/general/general-img/eye-icon-active.svg"
>` : /* Else the non-active one if allowed*/ userHasPrivilegeTo(privilege, 'U') ? `
>` : /* Else the non-active one if allowed*/ privilege.includes('U') ? `
<img class="btn-above-note hide-note-btn" alt="hide" src="assets/general/general-img/eye-icon.svg">` : ''}
${/*Show delete button */ userHasPrivilegeTo(privilege, 'D') ? `<img
${/*Show delete button */ privilege.includes('D') ? `<img
class="btn-above-note delete-note-btn" alt="delete" src="assets/general/general-img/del-icon.svg">` : ''}
<span class="discrete-text note-right-side-label-span
${isClientMessage === 1 ? 'client-message-label' : ''}">${escapeHtml(userFullName)}</span>
</label>
<!-- Extra div necessary to position circle loader to relative parent without taking label into account -->
<div class="relative">
${userHasPrivilegeTo(privilege, 'R') ? '' : `<div class="hidden-textarea-overlay"></div>`}
${privilege.includes('R') ? '' : `<div class="hidden-textarea-overlay"></div>`}
<!-- Textarea opening and closing has to be on the same line to prevent unnecessary line break -->
<textarea class="auto-resize-textarea ${isClientMessage === 1 ? 'client-message-textarea' : ''}
${/* Blur note text if not allowed to read*/ userHasPrivilegeTo(privilege, 'R') ? `
${/* Blur note text if not allowed to read*/ privilege.includes('R') ? `
` : 'hidden-note-message'}"
id="note-${id}"
data-note-id="${id}"
minlength="4" maxlength="1000" required
data-editable="${userHasPrivilegeTo(privilege, 'U') ? '1' : '0'}"
data-editable="${privilege.includes('U') ? '1' : '0'}"
name="message">${escapeHtml(message)}</textarea>
<div class="circle-loader client-note" data-note-id="${id}">
<div class="checkmark draw"></div>
Expand Down
31 changes: 17 additions & 14 deletions public/assets/client/read/client-read-main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

/**
Expand Down
6 changes: 3 additions & 3 deletions public/assets/general/ajax/submit-update-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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));
}
Expand Down
1 change: 1 addition & 0 deletions public/assets/general/validation/form-validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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']
);
Expand Down
10 changes: 3 additions & 7 deletions src/Application/Action/Authentication/Ajax/LoginSubmitAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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']
);
Expand Down
6 changes: 3 additions & 3 deletions src/Application/Action/Client/Page/ClientListPageAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
) {
}

Expand All @@ -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');
Expand Down
6 changes: 3 additions & 3 deletions src/Application/Action/User/Page/UserListPageAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -12,7 +12,7 @@ final class UserListPageAction
{
public function __construct(
private readonly TemplateRenderer $templateRenderer,
private readonly UserAuthorizationChecker $userAuthorizationChecker,
private readonly UserPermissionVerifier $userPermissionVerifier,
) {
}

Expand All @@ -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');
}

Expand Down
6 changes: 3 additions & 3 deletions src/Application/Middleware/PhpViewExtensionMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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');
Expand Down Expand Up @@ -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
Expand Down
14 changes: 6 additions & 8 deletions src/Application/Middleware/UserAuthenticationMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,16 @@ 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
* @param RequestHandlerInterface $handler
*
* @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
Expand All @@ -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 = [];
Expand All @@ -70,15 +68,15 @@ 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,
['loginUrl' => $this->routeParser->urlFor('login-page', [], $queryParams)],
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);
Expand Down
19 changes: 19 additions & 0 deletions src/Domain/Authentication/Repository/UserRoleFinderRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions src/Domain/Authentication/Service/AuthenticationLogger.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace App\Domain\Authentication\Service;

use App\Application\Data\UserNetworkSessionData;
use App\Domain\Security\Repository\AuthenticationLoggerRepository;

class AuthenticationLogger
{
public function __construct(
private readonly UserNetworkSessionData $userNetworkSessionData,
private readonly AuthenticationLoggerRepository $authenticationLoggerRepository,
) {
}

/**
* Log login request.
*
* @param string $email
* @param bool $success whether login request was a successful login or not
* @param int|null $userId
*
* @return int
*/
public function logLoginRequest(string $email, bool $success, ?int $userId = null): int
{
return $this->authenticationLoggerRepository->logLoginRequest(
$email,
$this->userNetworkSessionData->ipAddress,
$success,
$userId
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 9b6d5c7

Please sign in to comment.