diff --git a/README.md b/README.md index 523af43b..4f944c7e 100644 --- a/README.md +++ b/README.md @@ -1,25 +1,21 @@ # Slim Example Project [![Latest Version on Packagist](https://img.shields.io/github/release/samuelgfeller/slim-example-project.svg)](https://packagist.org/packages/samuelgfeller/slim-example-project) -[![Coverage](https://sonarcloud.io/api/project_badges/measure?project=samuelgfeller_slim-example-project&metric=coverage)](https://sonarcloud.io/summary/new_code?id=samuelgfeller_slim-example-project) -[![Quality Gate Status](https://sonarcloud.io/api/project_badges/measure?project=samuelgfeller_slim-example-project&metric=alert_status)](https://sonarcloud.io/summary/new_code?id=samuelgfeller_slim-example-project) -[![Maintainability Rating](https://sonarcloud.io/api/project_badges/measure?project=samuelgfeller_slim-example-project&metric=sqale_rating)](https://sonarcloud.io/summary/new_code?id=samuelgfeller_slim-example-project) -[![Lines of Code](https://sonarcloud.io/api/project_badges/measure?project=samuelgfeller_slim-example-project&metric=ncloc)](https://sonarcloud.io/summary/new_code?id=samuelgfeller_slim-example-project) +[![Code Coverage](https://scrutinizer-ci.com/g/samuelgfeller/slim-example-project/badges/coverage.png?b=master)](https://scrutinizer-ci.com/g/samuelgfeller/slim-example-project/?branch=master) +[![Build Status](https://scrutinizer-ci.com/g/samuelgfeller/slim-example-project/badges/build.png?b=master)](https://scrutinizer-ci.com/g/samuelgfeller/slim-example-project/build-status/master) +[![Quality Score](https://img.shields.io/scrutinizer/quality/g/samuelgfeller/slim-example-project.svg)](https://scrutinizer-ci.com/g/samuelgfeller/slim-example-project/?branch=master) +[![Software License](https://img.shields.io/badge/license-MIT-brightgreen.svg)](LICENSE) -This project aims to be a real-world example of a modern [Slim 4](https://www.slimframework.com/) -web application with a scalable structure and -a range of practical components and features. +Real-world example of a modern [Slim 4](https://www.slimframework.com/) web application with a scalable +structure and a variety of components and features to get started quickly. -It showcases the implementation of a simple yet robust +This project showcases the implementation of a simple yet robust [architecture](https://github.com/samuelgfeller/slim-example-project/wiki/Architecture) with a variety of backend and frontend features built using the Slim 4 micro-framework. The base for this project was the official [Slim-Skeleton](https://github.com/slimphp/Slim-Skeleton) and Odan's [slim4-skeleton](https://github.com/odan/slim4-skeleton). -This repository can serve as a learning example or be adapted for developing new -applications. - External library dependencies are [kept to a minimum](https://github.com/samuelgfeller/slim-example-project/wiki/Libraries-and-Framework) to facilitate maintenance and ensure long-term viability. @@ -34,8 +30,8 @@ to get started. Stripped down versions of this repository are available as skeleton templates. -With frontend [`slim-starter`](https://github.com/samuelgfeller/slim-starter) or just for an API: -[`slim-api-starter`](https://github.com/samuelgfeller/slim-api-starter). +With frontend [slim-starter](https://github.com/samuelgfeller/slim-starter) or just for an API: +[slim-api-starter](https://github.com/samuelgfeller/slim-api-starter). ## Features All the features were developed with an effort to ensure maximum user-friendliness. diff --git a/src/Domain/Client/Data/ClientData.php b/src/Domain/Client/Data/ClientData.php index d8c8e203..ba927446 100644 --- a/src/Domain/Client/Data/ClientData.php +++ b/src/Domain/Client/Data/ClientData.php @@ -33,7 +33,7 @@ public function __construct(?array $clientData = []) $this->id = $clientData['id'] ?? null; $this->firstName = $clientData['first_name'] ?? null; $this->lastName = $clientData['last_name'] ?? null; - $this->birthdate = isset($clientData['birthdate']) ? new \DateTimeImmutable($clientData['birthdate']) : null; + $this->birthdate = $this->createDateTimeImmutableString($clientData['birthdate'] ?? null); $this->location = $clientData['location'] ?? null; $this->phone = $clientData['phone'] ?? null; $this->email = $clientData['email'] ?? null; @@ -45,15 +45,19 @@ public function __construct(?array $clientData = []) // Cast to int if user id is set and not empty (null / empty string) $this->userId = !empty($clientData['user_id']) ? (int)$clientData['user_id'] : null; $this->clientStatusId = !empty($clientData['client_status_id']) ? (int)$clientData['client_status_id'] : null; - $this->updatedAt = $clientData['updated_at'] ?? null ? new \DateTimeImmutable($clientData['updated_at']) : null; - $this->createdAt = $clientData['created_at'] ?? null ? new \DateTimeImmutable($clientData['created_at']) : null; - $this->deletedAt = $clientData['deleted_at'] ?? null ? new \DateTimeImmutable($clientData['deleted_at']) : null; - + $this->updatedAt = $this->createDateTimeImmutableString($clientData['updated_at'] ?? null); + $this->createdAt = $this->createDateTimeImmutableString($clientData['created_at'] ?? null); + $this->deletedAt = $this->createDateTimeImmutableString($clientData['deleted_at'] ?? null); if ($this->birthdate) { $this->age = (new \DateTime())->diff($this->birthdate)->y; } } + private function createDateTimeImmutableString(?string $date): ?\DateTimeImmutable + { + return isset($date) ? new \DateTimeImmutable($date) : null; + } + /** * Returns all values of object as array for the database. * The array keys should match with the database diff --git a/src/Domain/UserActivity/Repository/UserActivityRepository.php b/src/Domain/UserActivity/Repository/UserActivityRepository.php index 994d530d..4b057699 100644 --- a/src/Domain/UserActivity/Repository/UserActivityRepository.php +++ b/src/Domain/UserActivity/Repository/UserActivityRepository.php @@ -38,11 +38,11 @@ public function findUserActivities(int|array $userIds): array /** * Insert user activity in database. * - * @param mixed $userActivityRow + * @param array $userActivityRow * * @return int lastInsertId */ - public function insertUserActivity($userActivityRow): int + public function insertUserActivity(array $userActivityRow): int { return (int)$this->queryFactory->insertQueryWithData($userActivityRow)->into('user_activity')->execute()->lastInsertId(); } diff --git a/src/Infrastructure/Service/Mailer.php b/src/Infrastructure/Service/Mailer.php index 1c815697..3e410970 100644 --- a/src/Infrastructure/Service/Mailer.php +++ b/src/Infrastructure/Service/Mailer.php @@ -12,6 +12,7 @@ /** * Mailer class with added method to get the html string from a template and added logging in the send() function. * Test sender score: https://www.mail-tester.com/. + * Documentation: https://github.com/samuelgfeller/slim-example-project/wiki/Mailing */ final readonly class Mailer { diff --git a/templates/authentication/email/new-account.email.php b/templates/authentication/email/new-account.email.php index a4b15650..c1f351e6 100644 --- a/templates/authentication/email/new-account.email.php +++ b/templates/authentication/email/new-account.email.php @@ -10,6 +10,7 @@ Hello

Your account has been created.

+ To verify that this email address belongs to you, please click on the following link:
Verify account.

diff --git a/templates/authentication/login.html.php b/templates/authentication/login.html.php index 220ab4d3..830a981e 100644 --- a/templates/authentication/login.html.php +++ b/templates/authentication/login.html.php @@ -22,7 +22,7 @@ fetch( 'layout/assets.html.php', diff --git a/templates/authentication/reset-password.html.php b/templates/authentication/reset-password.html.php index e0d24380..aca91730 100644 --- a/templates/authentication/reset-password.html.php +++ b/templates/authentication/reset-password.html.php @@ -80,13 +80,13 @@ class="form" method="post" autocomplete="on"> ' . html($validation['password2'][0]) . '' : '' ?> - ' . html($validation['password2'][1]) . '' : '' ?> - - + + fetch('layout/request-throttle.html.php') ?> diff --git a/templates/client/clients-list.html.php b/templates/client/clients-list.html.php index 6a83d321..834e092a 100644 --- a/templates/client/clients-list.html.php +++ b/templates/client/clients-list.html.php @@ -6,14 +6,11 @@ * active: array{\App\Domain\FilterSetting\Data\FilterData[]}, * } client list filters * - * @var $clientCreatePrivilege Privilege create or none + * @var $clientCreatePrivilege App\Domain\Authorization\Privilege create or none */ -use App\Domain\Authorization\Privilege; - $this->setLayout('layout/layout.html.php'); -$this->addAttribute('libs', ['fontawesome.css']); // Define assets that should be included // Populate variable $css for layout which then generates the HTML code to include assets diff --git a/templates/error/error-details.html.php b/templates/error/error-details.html.php deleted file mode 100644 index e16d9b1f..00000000 --- a/templates/error/error-details.html.php +++ /dev/null @@ -1,76 +0,0 @@ -setLayout(''); -?> - - - - - - - - Error - - - -
-

| - -

-

in - - on line . -

-
-
- - - - - - - $entry) { ?> - - - - - - - -
- - - - - diff --git a/templates/layout/layout.email.php b/templates/layout/layout.email.php deleted file mode 100644 index 3feec263..00000000 --- a/templates/layout/layout.email.php +++ /dev/null @@ -1,26 +0,0 @@ - - - - - - - - - Test Email Sample - - - - - - - - - - \ No newline at end of file diff --git a/templates/layout/layout.html.php b/templates/layout/layout.html.php index 301e7d5a..f558c29c 100644 --- a/templates/layout/layout.html.php +++ b/templates/layout/layout.html.php @@ -65,7 +65,7 @@ - +
diff --git a/templates/user/user-list.html.php b/templates/user/user-list.html.php index 330ba1ca..57651aa1 100644 --- a/templates/user/user-list.html.php +++ b/templates/user/user-list.html.php @@ -13,8 +13,8 @@ 'assets/general/page-component/modal/form-modal.css', 'assets/general/page-component/skeleton-loader/skeleton-loader.css', 'assets/user/list/user-list-skeleton-loader.css', + // Page-specific css has to come last to overwrite other styles 'assets/user/list/user-list.css', - // user-list.css has to come last to overwrite other styles ]); // Js files that import things from other js files $this->addAttribute( diff --git a/templates/user/user-read.html.php b/templates/user/user-read.html.php index 000e9cfa..63f053bf 100644 --- a/templates/user/user-read.html.php +++ b/templates/user/user-read.html.php @@ -20,8 +20,8 @@ 'assets/general/page-component/modal/form-modal.css', 'assets/general/dark-mode/dark-mode-toggle-switch.css', 'assets/general/page-component/contenteditable/contenteditable.css', + // Page-specific css has to come last to overwrite other styles 'assets/user/user.css', - // user.css has to come last to overwrite other styles ]); diff --git a/tests/Fixture/ClientFixture.php b/tests/Fixture/ClientFixture.php index 11840093..e25409e0 100644 --- a/tests/Fixture/ClientFixture.php +++ b/tests/Fixture/ClientFixture.php @@ -96,14 +96,4 @@ class ClientFixture 'deleted_at' => null, ], ]; - - public function getRecords(): array - { - return $this->records; - } - - public function getTable(): string - { - return $this->table; - } } diff --git a/tests/Fixture/ClientStatusFixture.php b/tests/Fixture/ClientStatusFixture.php index 95c5ee91..abf7bfd8 100644 --- a/tests/Fixture/ClientStatusFixture.php +++ b/tests/Fixture/ClientStatusFixture.php @@ -25,14 +25,4 @@ class ClientStatusFixture 'deleted_at' => null, ], ]; - - public function getTable(): string - { - return $this->table; - } - - public function getRecords(): array - { - return $this->records; - } } diff --git a/tests/Fixture/NoteFixture.php b/tests/Fixture/NoteFixture.php index c440100c..527630bb 100644 --- a/tests/Fixture/NoteFixture.php +++ b/tests/Fixture/NoteFixture.php @@ -100,14 +100,4 @@ class NoteFixture 'deleted_at' => null, ], ]; - - public function getTable(): string - { - return $this->table; - } - - public function getRecords(): array - { - return $this->records; - } } diff --git a/tests/Fixture/UserActivityFixture.php b/tests/Fixture/UserActivityFixture.php index 38f8c372..2e905b74 100644 --- a/tests/Fixture/UserActivityFixture.php +++ b/tests/Fixture/UserActivityFixture.php @@ -21,14 +21,4 @@ class UserActivityFixture 'user_agent' => 'Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/109.0', ], ]; - - public function getTable(): string - { - return $this->table; - } - - public function getRecords(): array - { - return $this->records; - } } diff --git a/tests/Fixture/UserFilterSettingFixture.php b/tests/Fixture/UserFilterSettingFixture.php index 4703685a..f64b0f6a 100644 --- a/tests/Fixture/UserFilterSettingFixture.php +++ b/tests/Fixture/UserFilterSettingFixture.php @@ -15,14 +15,4 @@ class UserFilterSettingFixture 'module' => 'dashboard-panel', ], ]; - - public function getTable(): string - { - return $this->table; - } - - public function getRecords(): array - { - return $this->records; - } } diff --git a/tests/Fixture/UserFixture.php b/tests/Fixture/UserFixture.php index e54b6941..1cceefff 100644 --- a/tests/Fixture/UserFixture.php +++ b/tests/Fixture/UserFixture.php @@ -31,14 +31,4 @@ class UserFixture 'deleted_at' => null, ], ]; - - public function getTable(): string - { - return $this->table; - } - - public function getRecords(): array - { - return $this->records; - } } diff --git a/tests/Fixture/UserRoleFixture.php b/tests/Fixture/UserRoleFixture.php index f70e7aa0..a26810aa 100644 --- a/tests/Fixture/UserRoleFixture.php +++ b/tests/Fixture/UserRoleFixture.php @@ -2,6 +2,10 @@ namespace App\Test\Fixture; +/** + * User roles are automatically inserted in the setup of all test + * functions using the database. + */ class UserRoleFixture { // Table name @@ -30,14 +34,4 @@ class UserRoleFixture 'hierarchy' => 4, ], ]; - - public function getTable(): string - { - return $this->table; - } - - public function getRecords(): array - { - return $this->records; - } } diff --git a/tests/Integration/Authentication/LoginPageActionTest.php b/tests/Integration/Authentication/LoginPageActionTest.php index 75664356..150a1f69 100644 --- a/tests/Integration/Authentication/LoginPageActionTest.php +++ b/tests/Integration/Authentication/LoginPageActionTest.php @@ -32,7 +32,8 @@ public function testLoginPageAction(): void } /** - * Test login page when already authenticated with redirect link. + * Test that the "redirect" GET param on the login page actually redirects + * to the specified route when logged in login. * * @return void */ @@ -52,7 +53,7 @@ public function testLoginPageActionAlreadyLoggedIn(): void $response = $this->app->handle($request); // Assert 302 Found (redirect) self::assertSame(StatusCodeInterface::STATUS_FOUND, $response->getStatusCode()); - + // Assert redirect location self::assertSame($requestRouteAfterLogin, $response->getHeaderLine('Location')); } } diff --git a/tests/Integration/Authentication/LoginSecurityTest.php b/tests/Integration/Authentication/LoginSecurityTest.php index f39964e3..325ccf8d 100644 --- a/tests/Integration/Authentication/LoginSecurityTest.php +++ b/tests/Integration/Authentication/LoginSecurityTest.php @@ -9,8 +9,6 @@ use Fig\Http\Message\StatusCodeInterface; use Odan\Session\SessionInterface; use PHPUnit\Framework\TestCase; -use Psr\Container\ContainerExceptionInterface; -use Psr\Container\NotFoundExceptionInterface; use TestTraits\Trait\DatabaseTestTrait; use TestTraits\Trait\FixtureTestTrait; use TestTraits\Trait\HttpTestTrait; @@ -25,11 +23,8 @@ class LoginSecurityTest extends TestCase use RouteTestTrait; /** - * Test thresholds and according delays of login failures - * If login request amount exceeds threshold, the user has to wait a certain delay. - * - * @throws NotFoundExceptionInterface - * @throws ContainerExceptionInterface + * Test thresholds and according delays of login failures. + * If the login request amount exceeds the threshold, the user has to wait a certain delay. * * @return void */ @@ -54,17 +49,19 @@ public function testLoginThrottlingWrongCredentials(): void // Login request with incorrect credentials $request = $this->createFormRequest('POST', $this->urlFor('login-submit'), $loginRequestBody); + // Get the throttle rules from the settings $throttleRules = $this->container->get('settings')['security']['login_throttle_rule']; $lowestThreshold = array_key_first($throttleRules); - // It should be tested with the most strict throttle as well. This means that last run should match last threshold + // It should be tested with the strictest throttle as well. + // This means that the last run should match the last threshold $thresholdForStrictestThrottle = array_key_last($throttleRules); // Reverse throttleRules for the loop iterations so that it will check for the big (stricter) delays first krsort($throttleRules); - // Simulate amount of login requests to go through for every throttling level + // Simulate the amount of login requests to go through for every throttling level // $nthLoginRequest is the current nth amount of login request that is made (4th, 5th, 6th etc.) for ($nthLoginRequest = 1; $nthLoginRequest <= $thresholdForStrictestThrottle; $nthLoginRequest++) { // * Until the lowest threshold is reached, the login requests are normal and should not be throttled @@ -76,20 +73,21 @@ public function testLoginThrottlingWrongCredentials(): void continue; // Skip to next loop iteration } - // * Lowest threshold reached, assert that correct throttle is applied + // * Lowest threshold reached, assert that the correct throttle is applied foreach ($throttleRules as $threshold => $delay) { - // Apply correct throttling rule in relation to nth login request by checking which threshold is reached + // Check if the number of login requests reached the threshold if ($nthLoginRequest >= $threshold) { - // If the amount of made login requests reaches the first threshold -> throttle + // If the number of made login requests reaches the first threshold -> throttle try { - // Creates an additional authentication_log entry that is needed to assert for next iterations - // with higher throttling + // Handle request again to create an additional authentication_log entry which is needed for the + // assertions in the next iterations with higher throttling. $this->app->handle($request); self::fail( 'SecurityException should be thrown' . "\nnthLoginrequest: $nthLoginRequest, threshold: $threshold" ); } catch (SecurityException $se) { + // Assert the delay and security type self::assertEqualsWithDelta( $delay, $se->getRemainingDelay(), @@ -102,7 +100,7 @@ public function testLoginThrottlingWrongCredentials(): void // Below this request after waiting the delay is with WRONG credentials if (is_numeric($delay)) { // After waiting the delay, user is allowed to make new login request but if the credentials - // are wrong again (using same $request), an exception is expected from the **second + // are wrong again (using the same $request), an exception is expected from the **second // security check** after the failed request in LoginVerifier. // (This second security check is not done if request has correct credentials) // Prepone last authentication_log to simulate waiting @@ -122,14 +120,15 @@ public function testLoginThrottlingWrongCredentials(): void $this->deleteLastAuthenticationLog(); } - // * Assert that after waiting the delay, a successful request can be made with correct credentials + // * Assert that after waiting the delay, a successful login request can be made with + // the correct credentials. $requestAfterWaitingDelay = $this->createFormRequest( 'POST', $this->urlFor('login-submit'), $correctLoginRequestBody ); $responseAfterWaiting = $this->app->handle($requestAfterWaitingDelay); - // Again delete the most recent authentication log entry as the request above is also an + // Again, delete the most recent authentication log entry as the request above is also an // "aside" test that should not influence login stats of the next iterations. $this->deleteLastAuthenticationLog(); @@ -143,15 +142,16 @@ public function testLoginThrottlingWrongCredentials(): void $responseAfterWaiting->getStatusCode() ); - // Reset last login request so that it's correct for the next request as security check is - // done before the new `authentication_log` entry is made. + // Reset the last login request so that it's correct for the next request as security check + // is done before the new `authentication_log` entry is made. // $this->postponeLastAuthenticationLog($delay); // Edit: the above is not needed. } } - // If right threshold is reached and asserted, go out of nthLogin loop to test the next iteration - // Otherwise the throttling foreach continues and the delay assertion fails as it would be too small - // once nthLoginRequest reaches the second-strictest throttling. + // If the right threshold is reached and asserted, go out of nthLogin loop to test the next + // iteration. + // Otherwise, the throttling foreach continues and the delay assertion fails as it would be + // too small once nthLoginRequest reaches the next throttling step. continue 2; } } diff --git a/tests/Integration/Authentication/LoginSubmitActionTest.php b/tests/Integration/Authentication/LoginSubmitActionTest.php index 055911b8..6f4ab3da 100644 --- a/tests/Integration/Authentication/LoginSubmitActionTest.php +++ b/tests/Integration/Authentication/LoginSubmitActionTest.php @@ -20,10 +20,11 @@ * Test the login submit actions. Contents of this test: * - normal login submit with correct credentials (302 Found redirect) * - login request with incorrect password (401 Unverified) - * - login request with invalid values (400 Bad request) - * - login request on unverified account (401 Unverified + email with verification token) - * - login request on suspended account (401 Unverified + email with info) - * - login request on locked account (401 Unverified + email with unlock token). + * - login request with invalid values (422 Unprocessable Entity) + * - login request on non-active account: + * - login request on unverified account (401 Unverified + email with verification token) + * - login request on suspended account (401 Unverified + email with info) + * - login request on locked account (401 Unverified + email with unlock token). * * Login tests involving request throttle are done in @see LoginSecurityTest */ @@ -75,7 +76,7 @@ public function testLoginSubmitAction(): void /** * Test that 401 Unauthorized is returned when trying to log in - * with wrong password. + * with the wrong password. */ public function testLoginSubmitActionWrongPassword(): void { @@ -138,7 +139,7 @@ public function testLoginSubmitActionInvalidValues(array $invalidLoginValues, st /** * Test login with user status unverified. - * When account is unverified, a verification link is sent to the user via the email. + * When the account is unverified, a verification link is sent to the user via the email. * * @param UserStatus $status * @param string $partialEmailBody @@ -169,7 +170,7 @@ public function testLoginSubmitActionNotActiveAccount(UserStatus $status, string self::assertNull($session->get('user_id')); // When the account is unverified, a verification link is sent to the user via the email - // Assert that correct email was sent (email body contains string) + // Assert that the correct email was sent (email body contains string) $email = $this->getMailerMessage(); $this->assertEmailHtmlBodyContains( $email, diff --git a/tests/Integration/Authentication/LogoutActionTest.php b/tests/Integration/Authentication/LogoutActionTest.php index 187bb92b..b0a98cb2 100644 --- a/tests/Integration/Authentication/LogoutActionTest.php +++ b/tests/Integration/Authentication/LogoutActionTest.php @@ -37,7 +37,7 @@ public function testLogoutPageAction(): void } /** - * Test that when user is not active but still logged in and tries + * Test that when a user is not active but still logged in and tries * to access a protected route, the user is logged out. * * @return void diff --git a/tests/Integration/Authentication/PasswordForgottenEmailSubmitActionTest.php b/tests/Integration/Authentication/PasswordForgottenEmailSubmitActionTest.php index 5df9e48a..3d645b0a 100644 --- a/tests/Integration/Authentication/PasswordForgottenEmailSubmitActionTest.php +++ b/tests/Integration/Authentication/PasswordForgottenEmailSubmitActionTest.php @@ -17,9 +17,9 @@ /** * Integration testing email submit of forgotten password form * - submit valid email + * - submit valid email after email abuse threshold reached -> unprocessable entity * - submit email of non-existing user -> act normal but no database change - * - submit invalid email -> 422 backend validation fail - * - submit malformed request body -> HttpBadRequestException. + * - submit invalid email -> 422 backend validation fails */ class PasswordForgottenEmailSubmitActionTest extends TestCase { @@ -105,6 +105,7 @@ public function testPasswordForgottenEmailSubmit(): void /** * Assert that verification email is not sent if the email threshold is reached. + * Using the same provider as the general security email test. * * @param int|string $delay * @param int $emailLogAmountInTimeSpan @@ -123,7 +124,7 @@ public function testPasswordForgottenEmailSubmitSecurityThrottling( $userId = $userRow['id']; $email = $userRow['email']; - // Insert email log entries + // Insert max amount of email log entries before throttling for ($i = 0; $i < $emailLogAmountInTimeSpan; $i++) { $this->insertFixtureRow( 'email_log', @@ -162,7 +163,7 @@ public function testPasswordForgottenEmailSubmitSecurityThrottling( */ public function testPasswordForgottenEmailSubmitUserNotExisting(): void { - // Not inserting user as it shouldn't exist + // Not inserting a user as it shouldn't exist $request = $this->createFormRequest( 'POST', // Request to change password diff --git a/tests/Integration/Authentication/PasswordResetSubmitActionTest.php b/tests/Integration/Authentication/PasswordResetSubmitActionTest.php index 3c1657bf..d9077766 100644 --- a/tests/Integration/Authentication/PasswordResetSubmitActionTest.php +++ b/tests/Integration/Authentication/PasswordResetSubmitActionTest.php @@ -19,9 +19,9 @@ /** * Integration testing password change from authenticated user * - request to set new password with valid token - * - request to set new password with invalid, expired and used token -> redirect to login page - * - request to set new password with valid token but invalid data -> 400 Bad request - * - request to set new password with malformed request body -> HttpBadRequestException. + * - request to set new password with invalid, expired and used token + * - request to set new password with valid token but invalid password (too short) + * - request to set new password with malformed request body -> bad request. */ class PasswordResetSubmitActionTest extends TestCase { @@ -104,7 +104,7 @@ public function testResetPasswordSubmitInvalidToken( $response = $this->app->handle($request); - // Assert 200 password reset first email form loaded + // Assert 200 OK password reset first email form loaded self::assertSame(StatusCodeInterface::STATUS_OK, $response->getStatusCode()); // Assert that token had NOT been used (except if already used) @@ -137,8 +137,6 @@ public function testResetPasswordSubmitInvalidData( UserVerificationData $verification, string $clearTextToken ): void { - // Invalid new password - $newPassword = '1'; // Insert user id 2 role: user $userRow = $this->insertFixture(UserFixture::class, ['id' => $verification->userId]); @@ -148,8 +146,9 @@ public function testResetPasswordSubmitInvalidData( 'POST', // Request to change password $this->urlFor('password-reset-submit'), [ - 'password' => $newPassword, - 'password2' => $newPassword, + // Password too short + 'password' => '1', + 'password2' => '1', 'token' => $clearTextToken, 'id' => (string)$verification->id, ] @@ -190,7 +189,7 @@ public function testPasswordResetPageAction(): void } /** - * Test that password reset page loads with status code 400 if token is missing. + * Test that password reset page loads with status code 400 if the token is missing. * * @return void */ diff --git a/tests/Integration/Authentication/RegisterVerifyActionTest.php b/tests/Integration/Authentication/RegisterVerifyActionTest.php index 60001065..34d66057 100644 --- a/tests/Integration/Authentication/RegisterVerifyActionTest.php +++ b/tests/Integration/Authentication/RegisterVerifyActionTest.php @@ -18,11 +18,7 @@ /** * Test that the link sent to user when creating an account - * works correctly and safely. Test covered in this class: - * - Set user account status from unverified to active with valid token - * - Attempt to verify user account that is already active - * - Attempt to verify user with used, invalid and expired token - * - Test action with invalid parameters (400 Bad request). + * works correctly and safely. */ class RegisterVerifyActionTest extends TestCase { diff --git a/tests/Integration/Client/ApiClientCreateActionTest.php b/tests/Integration/Client/ApiClientCreateActionTest.php index de0c056b..28b3d560 100644 --- a/tests/Integration/Client/ApiClientCreateActionTest.php +++ b/tests/Integration/Client/ApiClientCreateActionTest.php @@ -19,7 +19,7 @@ use TestTraits\Trait\RouteTestTrait; /** - * Client creation from public page submit tests. + * Client creation from external frontend application tests. */ class ApiClientCreateActionTest extends TestCase { diff --git a/tests/Integration/Client/ClientCreateActionTest.php b/tests/Integration/Client/ClientCreateActionTest.php index 9b2c00b2..fefd45f5 100644 --- a/tests/Integration/Client/ClientCreateActionTest.php +++ b/tests/Integration/Client/ClientCreateActionTest.php @@ -21,9 +21,7 @@ use TestTraits\Trait\RouteTestTrait; /** - * Client creation submit tests - * - Normal client creation - * - With invalid values -> 422. + * Client creation tests. */ class ClientCreateActionTest extends TestCase { @@ -42,7 +40,7 @@ class ClientCreateActionTest extends TestCase * @param array $authenticatedUserRow authenticated user attributes containing the user_role_id * @param array $expectedResult HTTP status code, bool if db_entry_created and json_response * - * @throws \JsonException|ContainerExceptionInterface|NotFoundExceptionInterface + * @throws ContainerExceptionInterface|NotFoundExceptionInterface * * @return void */ @@ -111,7 +109,7 @@ public function testClientSubmitCreateActionAuthorization( $userActivityRow['id'] ); // Assert relevant user activity data - $decodedUserActivityDataFromDb = json_decode($userActivityRow['data'], true, 512, JSON_THROW_ON_ERROR); + $decodedUserActivityDataFromDb = json_decode($userActivityRow['data'], true, 512, JSON_PARTIAL_OUTPUT_ON_ERROR); // Done separately as we only want to test the relevant data for the creation, and we cannot control the order self::assertEqualsCanonicalizing( $clientCreationValues, @@ -131,7 +129,7 @@ public function testClientSubmitCreateActionAuthorization( 'action' => UserActivity::CREATED->value, 'table' => 'note', 'row_id' => $noteId, - 'data' => json_encode($noteValues, JSON_THROW_ON_ERROR), + 'data' => json_encode($noteValues, JSON_PARTIAL_OUTPUT_ON_ERROR), ], 'user_activity', (int)$this->findTableRowsByColumn('user_activity', 'table', 'note')[0]['id'] @@ -200,8 +198,8 @@ public function testClientSubmitCreateActionInvalid(array $requestBody, array $j * E.g. ->maxLength('first_name', 100) has the consequence that it expects * a non-null value for the first_name. Without ->allowEmptyString('first_name') * the validation would fail with "This field cannot be left empty". - * I did not expect this behaviour and ran into this when testing in the GUI so this test - * makes sense to me in order to not forget to always add ->allow[Whatever] when value is optional. + * I did not expect this behaviour and ran into this when testing, so this test + * reminds to always add ->allow[Whatever] when value is optional. * * @param array $requestBody * @@ -243,7 +241,7 @@ public function testClientSubmitCreateActionValid(array $requestBody): void } /** - * Client creation with valid data. + * Unauthenticated client creation. * * @return void */ diff --git a/tests/Integration/Client/ClientCreateDropdownOptionsTest.php b/tests/Integration/Client/ClientCreateDropdownOptionsTest.php index a836fbe0..91ff5895 100644 --- a/tests/Integration/Client/ClientCreateDropdownOptionsTest.php +++ b/tests/Integration/Client/ClientCreateDropdownOptionsTest.php @@ -32,8 +32,7 @@ class ClientCreateDropdownOptionsTest extends TestCase * authenticated user roles. * * @param array $authenticatedUserRow authenticated user attributes containing the user_role_id - * @param array $otherUserRow other user (that appears in dropdown) attributes containing the user_role_id - * user role not relevant as if authorized every user can be selected + * @param array $otherUserRow another user (that appears in dropdown) attributes containing the user_role_id * @param array $expectedUserNames * *@throws NotFoundExceptionInterface @@ -52,7 +51,7 @@ public function testClientCreateAssignedUserDropdownOptionsAuthorization( // Client statuses, sexes and vigilance levels are returned too but not tested here (authorization most important) - // Simulate logged-in user with same user as linked to client + // Simulate logged-in user with the same user as linked to client $this->container->get(SessionInterface::class)->set('user_id', $authenticatedUserRow['id']); $request = $this->createJsonRequest( diff --git a/tests/Integration/Client/ClientListActionTest.php b/tests/Integration/Client/ClientListActionTest.php index 5df982b0..cf6da761 100644 --- a/tests/Integration/Client/ClientListActionTest.php +++ b/tests/Integration/Client/ClientListActionTest.php @@ -24,16 +24,11 @@ use TestTraits\Trait\HttpTestTrait; use TestTraits\Trait\RouteTestTrait; -/** - * Copied and pasted content from client for now. - */ - /** * - client list page action - * - Unauthenticated - * - Authenticated - * - client list json request - * - client list filtered + * - authorization + * - unauthenticated + * - client list with filter json request * - client list with invalid filters. */ class ClientListActionTest extends TestCase diff --git a/tests/Integration/Client/ClientReadPageActionTest.php b/tests/Integration/Client/ClientReadPageActionTest.php index 90fa6dbf..0c57cf0d 100644 --- a/tests/Integration/Client/ClientReadPageActionTest.php +++ b/tests/Integration/Client/ClientReadPageActionTest.php @@ -17,8 +17,8 @@ /** * Test cases for client read page load - * - Authenticated - * - Unauthenticated. + * - authorization + * - unauthenticated. */ class ClientReadPageActionTest extends TestCase { diff --git a/tests/Integration/Client/ClientUpdateActionTest.php b/tests/Integration/Client/ClientUpdateActionTest.php index c206f721..8d20e332 100644 --- a/tests/Integration/Client/ClientUpdateActionTest.php +++ b/tests/Integration/Client/ClientUpdateActionTest.php @@ -23,12 +23,10 @@ /** * Client update integration test: - * - normal update - * - invalid note update + * - normal update with different user roles + * - invalid update requests * - unauthenticated client update * - client update request with value to change being the same as in database - * NOT in this test: - * - edit non-existing client - reason: delete request on non-existing client is tested. */ class ClientUpdateActionTest extends TestCase { diff --git a/tests/Integration/Dashboard/DashboardTogglePanelActionTest.php b/tests/Integration/Dashboard/DashboardTogglePanelActionTest.php index 834cecb7..d469ef39 100644 --- a/tests/Integration/Dashboard/DashboardTogglePanelActionTest.php +++ b/tests/Integration/Dashboard/DashboardTogglePanelActionTest.php @@ -23,7 +23,7 @@ class DashboardTogglePanelActionTest extends TestCase use HttpJsonTestTrait; /** - * Test that when user clicks to enable 2 panels it is + * Test that when a user clicks to enable two panels, it is * saved in the database table user_filter_setting. * * @return void @@ -63,7 +63,7 @@ public function testDashboardTogglePanelActionAuthenticated(): void 'filter_id', 'unassigned-panel' ); - // Assert that panel assigned to me exists and has correct values + // Assert that the panel assigned to me exists and has correct values $this->assertTableRowsByColumn( [ 'user_id' => $userId, diff --git a/tests/Integration/Note/NoteCreateActionTest.php b/tests/Integration/Note/NoteCreateActionTest.php index 6e979976..4056c045 100644 --- a/tests/Integration/Note/NoteCreateActionTest.php +++ b/tests/Integration/Note/NoteCreateActionTest.php @@ -25,7 +25,7 @@ * Test cases for client read note creation * - Authenticated with different user roles * - Unauthenticated - * - Invalid data (validation test). + * - Invalid data. */ class NoteCreateActionTest extends TestCase { @@ -81,7 +81,7 @@ public function testNoteSubmitCreateActionAuthorization( // Make request $response = $this->app->handle($request); - // Assert 201 Created redirect to login url + // Assert 201 Created self::assertSame($expectedResult['creation'][StatusCodeInterface::class], $response->getStatusCode()); // Assert database @@ -100,7 +100,7 @@ public function testNoteSubmitCreateActionAuthorization( 'client_id' => $clientRow['id'], 'is_main' => 0, 'user_id' => $authenticatedUserRow['id'], - ], JSON_THROW_ON_ERROR), + ], JSON_PARTIAL_OUTPUT_ON_ERROR), ], 'user_activity', (int)$this->findLastInsertedTableRow('user_activity')['id'] diff --git a/tests/Integration/Note/NoteDeleteActionTest.php b/tests/Integration/Note/NoteDeleteActionTest.php index 021d5743..cb868a3b 100644 --- a/tests/Integration/Note/NoteDeleteActionTest.php +++ b/tests/Integration/Note/NoteDeleteActionTest.php @@ -21,7 +21,7 @@ use TestTraits\Trait\RouteTestTrait; /** - * Test cases for client read note deletion + * Test cases for note deletion * - Authenticated with different user roles * - Unauthenticated. */ diff --git a/tests/Integration/Note/NoteListActionTest.php b/tests/Integration/Note/NoteListActionTest.php index a6c3831c..e64ddbe9 100644 --- a/tests/Integration/Note/NoteListActionTest.php +++ b/tests/Integration/Note/NoteListActionTest.php @@ -97,7 +97,7 @@ public function testNoteListActionAuthorization( // Assert status code self::assertSame(StatusCodeInterface::STATUS_OK, $response->getStatusCode()); - // If user has not privilege to read note, the message is replaced by lorem ipsum + // If the user doesn't have the privilege to read note, the message is replaced by lorem ipsum and blurred $loremIpsum = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit @@ -235,10 +235,8 @@ public function testNoteListFilter( * @return void */ #[DataProviderExternal(\App\Test\Provider\Note\NoteProvider::class, 'invalidNoteListFilterProvider')] - public function testNoteListFilterInvalid( - array $filterQueryParams, - string $exceptionMessage, - ): void { + public function testNoteListFilterInvalid(array $filterQueryParams, string $exceptionMessage,): void + { $loggedInUserId = $this->insertFixture(UserFixture::class)['id']; $this->container->get(SessionInterface::class)->set('user_id', $loggedInUserId); diff --git a/tests/Integration/Note/NoteReadPageActionTest.php b/tests/Integration/Note/NoteReadPageActionTest.php index 3a9823b1..ad00b98b 100644 --- a/tests/Integration/Note/NoteReadPageActionTest.php +++ b/tests/Integration/Note/NoteReadPageActionTest.php @@ -29,16 +29,16 @@ class NoteReadPageActionTest extends TestCase use AuthorizationTestTrait; /** - * Notes don't have an own read page. When accessed + * Notes don't have an own read page. When accessed, * there should be a redirect to the client read page * scrolling to the note. * - * @throws ContainerExceptionInterface + * @return void * @throws NotFoundExceptionInterface * - * @return void + * @throws ContainerExceptionInterface */ - public function testNoteReadPageActionAuthenticated(): void + public function testNoteReadPageAction(): void { // Insert authenticated user newcomer which is allowed to read the page (only his user will load however) $userId = $this->insertFixture( @@ -46,44 +46,57 @@ public function testNoteReadPageActionAuthenticated(): void $this->addUserRoleId(['user_role_id' => UserRole::NEWCOMER]), )['id']; + // Insert fixtures for client and note + $clientStatusId = $this->insertFixture(ClientStatusFixture::class)['id']; + $clientId = $this->insertFixture(ClientFixture::class, ['client_status_id' => $clientStatusId], )['id']; + $noteId = $this->insertFixture(NoteFixture::class, ['client_id' => $clientId, 'user_id' => $userId], )['id']; + // Simulate logged-in user by setting the user_id session variable $this->container->get(SessionInterface::class)->set('user_id', $userId); - // *Test request on NOT existing note - $requestNotExistingNote = $this->createRequest('GET', $this->urlFor('note-read-page', ['note_id' => '1'])); - $response = $this->app->handle($requestNotExistingNote); + $requestExistingNote = $this->createRequest( + 'GET', + $this->urlFor('note-read-page', ['note_id' => $noteId]) + ); + $response = $this->app->handle($requestExistingNote); // Assert 200 OK self::assertSame(StatusCodeInterface::STATUS_FOUND, $response->getStatusCode()); // Assert that user is redirected to client-list page if note doesn't exist - $expectedClientReadUrl = $this->urlFor('client-list-page'); + $expectedClientReadUrl = $this->urlFor('client-read-page', ['client_id' => $clientId]); + // Url with added hash + $expectedClientReadUrl .= "#note-$noteId-container"; self::assertSame($expectedClientReadUrl, $response->getHeaderLine('Location')); + } - // *Test request on existing note - $clientStatusId = $this->insertFixture(ClientStatusFixture::class)['id']; - $clientId = $this->insertFixture( - ClientFixture::class, - ['client_status_id' => $clientStatusId], - )['id']; - $noteId = $this->insertFixture( - NoteFixture::class, - ['client_id' => $clientId, 'user_id' => $userId], + /** + * Test that when trying to read a note that doesn't exist, the user is redirected to the client list page. + * + * @return void + * @throws ContainerExceptionInterface + * @throws NotFoundExceptionInterface + */ + public function testNotExistingNoteReadPageAction(): void + { + // Insert authenticated user newcomer which is allowed to read the page (only his user will load however) + $userId = $this->insertFixture( + UserFixture::class, + $this->addUserRoleId(['user_role_id' => UserRole::NEWCOMER]), )['id']; - $requestExistingNote = $this->createRequest( - 'GET', - $this->urlFor('note-read-page', ['note_id' => $noteId]) - ); - $response = $this->app->handle($requestExistingNote); + // Simulate logged-in user by setting the user_id session variable + $this->container->get(SessionInterface::class)->set('user_id', $userId); + + // Test request on NOT existing note + $requestNotExistingNote = $this->createRequest('GET', $this->urlFor('note-read-page', ['note_id' => '1'])); + $response = $this->app->handle($requestNotExistingNote); // Assert 200 OK self::assertSame(StatusCodeInterface::STATUS_FOUND, $response->getStatusCode()); // Assert that user is redirected to client-list page if note doesn't exist - $expectedClientReadUrl = $this->urlFor('client-read-page', ['client_id' => $clientId]); - // Url with added hash - $expectedClientReadUrl .= "#note-$noteId-container"; + $expectedClientReadUrl = $this->urlFor('client-list-page'); self::assertSame($expectedClientReadUrl, $response->getHeaderLine('Location')); } diff --git a/tests/Integration/Note/NoteUpdateActionTest.php b/tests/Integration/Note/NoteUpdateActionTest.php index 5838898b..8cc23afd 100644 --- a/tests/Integration/Note/NoteUpdateActionTest.php +++ b/tests/Integration/Note/NoteUpdateActionTest.php @@ -24,7 +24,7 @@ * Test cases for client read note modification * - Authenticated with different user roles * - Unauthenticated - * - Invalid data (validation test). + * - Invalid data. */ class NoteUpdateActionTest extends TestCase { @@ -239,7 +239,7 @@ public function testNoteSubmitUpdateActionInvalid(array $invalidRequestBody, arr ['client_id' => $clientRow['id'], 'user_id' => $userId, 'is_main' => 0], ); - // Simulate logged-in user with same user as linked to client + // Simulate logged-in user with the same user as linked to client $this->container->get(SessionInterface::class)->set('user_id', $userId); $request = $this->createJsonRequest( diff --git a/tests/Integration/User/PasswordChangeSubmitActionTest.php b/tests/Integration/User/PasswordChangeSubmitActionTest.php index 671fd633..ee6b2ef9 100644 --- a/tests/Integration/User/PasswordChangeSubmitActionTest.php +++ b/tests/Integration/User/PasswordChangeSubmitActionTest.php @@ -20,7 +20,7 @@ /** * Integration testing password change from authenticated user * - change password authenticated - authorization - * - change password authenticated - invalid data -> 400 Bad request + * - change password authenticated - invalid data * - change password not authenticated -> 302 to login page with correct redirect param. */ class PasswordChangeSubmitActionTest extends TestCase @@ -148,7 +148,7 @@ public function testChangePasswordSubmitActionInvalid(array $requestBody, array // Simulate logged-in user by setting the user_id session variable $this->container->get(SessionInterface::class)->set('user_id', $userRow['id']); $response = $this->app->handle($request); - // Assert 200 OK + // Assert 422 Unprocessable Entity self::assertSame(StatusCodeInterface::STATUS_UNPROCESSABLE_ENTITY, $response->getStatusCode()); // database should be unchanged $this->assertTableRowEquals($userRow, 'user', $userRow['id']); diff --git a/tests/Integration/User/UserCreateActionTest.php b/tests/Integration/User/UserCreateActionTest.php index 004ffd11..2cf9c49a 100644 --- a/tests/Integration/User/UserCreateActionTest.php +++ b/tests/Integration/User/UserCreateActionTest.php @@ -18,6 +18,7 @@ use TestTraits\Trait\FixtureTestTrait; use TestTraits\Trait\HttpJsonTestTrait; use TestTraits\Trait\HttpTestTrait; +use TestTraits\Trait\MailerTestTrait; use TestTraits\Trait\RouteTestTrait; /** @@ -32,6 +33,7 @@ class UserCreateActionTest extends TestCase use DatabaseTestTrait; use FixtureTestTrait; use AuthorizationTestTrait; + use MailerTestTrait; /** * Create user authorization test with different user roles. @@ -73,11 +75,13 @@ public function testUserSubmitCreateAuthorization( $this->urlFor('user-create-submit'), $requestData ); + // Simulate logged-in user by setting the user_id session variable $this->container->get(SessionInterface::class)->set('user_id', $authenticatedUserRow['id']); $response = $this->app->handle($request); // Assert status code self::assertSame($expectedResult[StatusCodeInterface::class], $response->getStatusCode()); + // Assert database if ($expectedResult['dbChanged'] === true) { $userDbRow = $this->findLastInsertedTableRow('user'); @@ -91,7 +95,7 @@ public function testUserSubmitCreateAuthorization( 'action' => UserActivity::CREATED->value, 'table' => 'user', 'row_id' => $userDbRow['id'], - 'data' => json_encode($requestData, JSON_THROW_ON_ERROR), + 'data' => json_encode($requestData, JSON_PARTIAL_OUTPUT_ON_ERROR), ], 'user_activity', (int)$this->findTableRowsByColumn('user_activity', 'table', 'user')[0]['id'] @@ -108,11 +112,28 @@ public function testUserSubmitCreateAuthorization( 'user_activity', (int)$this->findTableRowsByColumn('user_activity', 'table', 'user_verification')[0]['id'] ); - } else { - // Only 1 rows (authenticated user) expected in user table + + // When the account is created and unverified, a verification link is sent to the user via the email + // Assert that the correct email was sent (email body contains string) + $email = $this->getMailerMessage(); + $this->assertEmailHtmlBodyContains( + $email, + 'To verify that this email address belongs to you, please click on the following link', + ); + // Assert that email was sent to the right person in the right format + $this->assertEmailHeaderSame( + $email, + 'To', + $requestData['first_name'] . ' ' . + $requestData['surname'] . ' <' . $requestData['email'] . '>' + ); + } else { // Database must be unchanged + // Only 1 row (authenticated user) expected in user table $this->assertTableRowCount(1, 'user'); $this->assertTableRowCount(0, 'user_activity'); } + + $this->assertJsonData($expectedResult['jsonResponse'], $response); } diff --git a/tests/Integration/User/UserCreateDropdownOptionsTest.php b/tests/Integration/User/UserCreateDropdownOptionsTest.php index e9d20028..7216138d 100644 --- a/tests/Integration/User/UserCreateDropdownOptionsTest.php +++ b/tests/Integration/User/UserCreateDropdownOptionsTest.php @@ -50,7 +50,7 @@ public function testUserCreateDropdownOptionsAuthorization( $this->addUserRoleId($authenticatedUserAttributes) ); - // Simulate logged-in user with same user as linked to client + // Simulate logged-in user with the same user as linked to client $this->container->get(SessionInterface::class)->set('user_id', $authenticatedUserRow['id']); $request = $this->createJsonRequest( diff --git a/tests/Integration/User/UserDeleteActionTest.php b/tests/Integration/User/UserDeleteActionTest.php index 68960dc1..d121c475 100644 --- a/tests/Integration/User/UserDeleteActionTest.php +++ b/tests/Integration/User/UserDeleteActionTest.php @@ -16,7 +16,7 @@ use TestTraits\Trait\RouteTestTrait; /** - * User submit delete action tests + * User delete action tests * - Authenticated with different user roles * - Unauthenticated. */ @@ -88,7 +88,7 @@ public function testUserSubmitDeleteActionAuthorization( } /** - * Test that when user is not logged in 401 Unauthorized is returned. + * Test that when the user is not logged in 401 Unauthorized is returned. * * @return void */ diff --git a/tests/Integration/User/UserFetchActivityActionTest.php b/tests/Integration/User/UserFetchActivityActionTest.php index c9ae5553..07b8bcc9 100644 --- a/tests/Integration/User/UserFetchActivityActionTest.php +++ b/tests/Integration/User/UserFetchActivityActionTest.php @@ -29,7 +29,7 @@ class UserFetchActivityActionTest extends TestCase use AuthorizationTestTrait; /** - * Test that when fetching user activity an array is returned. + * Test that when fetching user activity, an array is returned. * * @throws NotFoundExceptionInterface * @throws ContainerExceptionInterface @@ -43,27 +43,27 @@ public function testUserListActivityFetchAction(): void UserFixture::class, $this->addUserRoleId(['user_role_id' => UserRole::NEWCOMER]), )['id']; - // Insert user activity to cover most possible code with this test + // Insert user activity to cover the most possible code with this test $this->insertFixture(UserActivityFixture::class, ['user_id' => $userId]); // Simulate logged-in user by setting the user_id session variable $this->container->get(SessionInterface::class)->set('user_id', $userId); + // Make request to fetch user activity for 2 users (even if only one exists) $request = $this->createJsonRequest('GET', $this->urlFor('user-get-activity')) - ->withQueryParams(['user' => $userId]); + ->withQueryParams(['user' => [$userId, $userId + 1]]); $response = $this->app->handle($request); // Assert status code 200 OK self::assertSame(StatusCodeInterface::STATUS_OK, $response->getStatusCode()); // Get response as array $responseData = $this->getJsonData($response); - // Only assert if array is returned as there is quite a logic how its built and I don't think its - // pertinent to test this in detail and when it's changed it'd be annoying having to update it each time here + // Only assert that array is returned as building the entire expected response would be complicated. self::assertIsArray($responseData); self::assertNotEmpty($responseData); } /** - * Test that when fetching user activity without query params + * Test that when fetching user activity without query params, * an empty array is returned. * * @throws NotFoundExceptionInterface @@ -87,8 +87,7 @@ public function testUserListActivityFetchActionWithoutQueryParams(): void self::assertSame(StatusCodeInterface::STATUS_OK, $response->getStatusCode()); // Get response as array $responseData = $this->getJsonData($response); - // Only assert if array is returned as there is quite a logic how its built and I don't think its - // pertinent to test this in detail and when it's changed it'd be annoying having to update it each time here + // Only assert that array is returned as building the entire expected response would be complicated. self::assertIsArray($responseData); self::assertEmpty($responseData); } diff --git a/tests/Integration/User/UserReadPageActionTest.php b/tests/Integration/User/UserReadPageActionTest.php index fc58db81..d46dc2a0 100644 --- a/tests/Integration/User/UserReadPageActionTest.php +++ b/tests/Integration/User/UserReadPageActionTest.php @@ -54,7 +54,7 @@ public function testUserReadPageActionAuthorization( /** * Test that http not found exception is thrown when - * user tries to read non-existing user page. + * a user tries to read non-existing user page. * * @return void */ diff --git a/tests/Integration/User/UserUpdateActionTest.php b/tests/Integration/User/UserUpdateActionTest.php index d027d224..5ea075fa 100644 --- a/tests/Integration/User/UserUpdateActionTest.php +++ b/tests/Integration/User/UserUpdateActionTest.php @@ -38,7 +38,7 @@ class UserUpdateActionTest extends TestCase * @param array $userToChangeRow user to change attributes containing the user_role_id * @param array $authenticatedUserRow authenticated user attributes containing the user_role_id * @param array $requestData array of data for the request body - * @param array $expectedResult HTTP status code, bool if db_entry_created and json_response + * @param array $expectedResult HTTP status code, bool if db entry is created and json response * *@throws NotFoundExceptionInterface * @throws \JsonException @@ -138,7 +138,7 @@ public function testUserSubmitUpdateInvalid(array $requestBody, array $jsonRespo // Simulate logged-in user by setting the user_id session variable $this->container->get(SessionInterface::class)->set('user_id', $userRow['id']); $response = $this->app->handle($request); - // Assert 200 OK + // Assert 422 Unprocessable Entity self::assertSame(StatusCodeInterface::STATUS_UNPROCESSABLE_ENTITY, $response->getStatusCode()); // database must be unchanged $this->assertTableRowEquals($userRow, 'user', $userRow['id']); diff --git a/tests/Integration/UtilityTest/TranslateActionTest.php b/tests/Integration/UtilityTest/TranslateActionTest.php index 10d57f93..5928df8c 100644 --- a/tests/Integration/UtilityTest/TranslateActionTest.php +++ b/tests/Integration/UtilityTest/TranslateActionTest.php @@ -33,18 +33,18 @@ public function testTranslateAction(): void $response = $this->app->handle($request); // Assert 200 OK self::assertSame(StatusCodeInterface::STATUS_OK, $response->getStatusCode()); - // Testing only response structure and not if strings actually were translated as they don't work locally + // Testing only response structure and not if strings were translated as localization doesn't work locally $this->assertJsonData(['Hello' => 'Hello', 'World' => 'World'], $response); } /** - * Test that password reset page loads with status code 400 if token is missing. + * Test that translate action returns 400 Bad request when query params are missing. * * @return void */ public function testPasswordResetPageActionTokenMissing(): void { - // Create token with missing token + // Create token with missing query params $request = $this->createRequest('GET', $this->urlFor('translate')); $response = $this->app->handle($request); // Assert 400 Bad request diff --git a/tests/Provider/Authentication/AuthenticationProvider.php b/tests/Provider/Authentication/AuthenticationProvider.php index 091a4935..e0806578 100644 --- a/tests/Provider/Authentication/AuthenticationProvider.php +++ b/tests/Provider/Authentication/AuthenticationProvider.php @@ -3,33 +3,9 @@ namespace App\Test\Provider\Authentication; use App\Domain\User\Enum\UserStatus; -use Fig\Http\Message\StatusCodeInterface; class AuthenticationProvider { - /** - * Provide status and partial email content for registration test on existing user. - * - * @return array - */ - public static function registerExistingUserStatusAndEmailCases(): array - { - return [ - [ - 'existing_user_status' => UserStatus::Active, - 'partial_email_body' => 'If this was you, then you can login with your credentials by navigating to the', - ], - [ - 'existing_user_status' => UserStatus::Locked, - 'partial_email_body' => 'If this was you, then we have the regret to inform you that your account is locked for security reasons', - ], - [ - 'existing_user_status' => UserStatus::Suspended, - 'partial_email_body' => 'If this was you, then we have the regret to inform you that your account is suspended', - ], - ]; - } - /** * Provide status and partial email content for login test user that is not active. * @@ -53,26 +29,6 @@ public static function nonActiveAuthenticationRequestCases(): array ]; } - /** - * Provide status and partial email content for login test user that is not active - * In provider mainly to reset database between correct and incorrect requests. - * - * @return array - */ - public static function authenticationSecurityCases(): array - { - return [ - [ - 'correct_credentials' => true, - 'status_code' => StatusCodeInterface::STATUS_FOUND, - ], - // [ - // 'correct_credentials' => false, - // 'status_code' => StatusCodeInterface::STATUS_UNAUTHORIZED, - // ], - ]; - } - /** * Invalid login credentials provider that should fail validation. */ diff --git a/tests/Provider/Client/ApiClientCreateProvider.php b/tests/Provider/Client/ApiClientCreateProvider.php index 1ce1af79..bd470356 100644 --- a/tests/Provider/Client/ApiClientCreateProvider.php +++ b/tests/Provider/Client/ApiClientCreateProvider.php @@ -6,7 +6,7 @@ class ApiClientCreateProvider { /** * Returns combinations of invalid data to trigger validation exception - * for client creation from public page. + * for client creation from external frontend. * * @return array */ diff --git a/tests/Provider/Client/ClientCreateProvider.php b/tests/Provider/Client/ClientCreateProvider.php index e962057c..abd0a9cd 100644 --- a/tests/Provider/Client/ClientCreateProvider.php +++ b/tests/Provider/Client/ClientCreateProvider.php @@ -25,11 +25,11 @@ public static function clientCreationDropdownOptionsCases(): array $newcomerAttributes = ['first_name' => 'Newcomer', 'user_role_id' => UserRole::NEWCOMER]; // Newcomer must not receive any available user - // Advisor is allowed to create client but only assign it to himself or leave user_id empty + // Advisor is allowed to create a client but only assign it to himself or leave user_id empty // Managing advisor and higher should receive all available users return [ // "owner" means from the perspective of the authenticated user - [ // ? Newcomer - not allowed so nothing should be returned + [ // ? Newcomer - not allowed, so nothing should be returned 'authenticatedUserRow' => $newcomerAttributes, 'otherUserRow' => $advisorAttributes, 'expectedUserNames' => [], @@ -239,8 +239,8 @@ public static function invalidClientCreationProvider(): array * E.g. ->maxLength('first_name', 100) has the consequence that it expects * a non-null value for the first_name. Without ->allowEmptyString('first_name') * the validation would fail with "This field cannot be left empty". - * I did not expect this behaviour and ran into this when testing in the GUI so this test - * makes sense to me in order to not forget to always add ->allow[Whatever] when value is optional. + * I did not expect this behaviour and ran into this when testing, so this test + * * reminds to always add ->allow[Whatever] when value is optional. * * @return array */ diff --git a/tests/Provider/Client/ClientDeleteProvider.php b/tests/Provider/Client/ClientDeleteProvider.php index 08723957..07d9d22c 100644 --- a/tests/Provider/Client/ClientDeleteProvider.php +++ b/tests/Provider/Client/ClientDeleteProvider.php @@ -81,7 +81,7 @@ public static function clientUndeleteDeleteProvider(): array ], ]; - // nly managing advisors and higher may delete clients + // Only managing advisors and higher may delete clients return [ // * Advisor 'advisor owner undelete' => [ // ? Advisor owner - undelete client - not allowed diff --git a/tests/Provider/Client/ClientReadProvider.php b/tests/Provider/Client/ClientReadProvider.php index 78218fb5..8aa968c6 100644 --- a/tests/Provider/Client/ClientReadProvider.php +++ b/tests/Provider/Client/ClientReadProvider.php @@ -15,7 +15,7 @@ public static function clientReadAuthorizationCases(): array $advisorAttr = ['user_role_id' => UserRole::ADVISOR]; $newcomerAttr = ['user_role_id' => UserRole::NEWCOMER]; - // Testing authorization: with the lowest allowed privilege and with highest not allowed + // Testing authorization: with the lowest allowed privilege and with the highest not allowed return [ // User owner is the user itself [// ? newcomer not owner - allowed reading undeleted client - but not update or create main note 'userRow' => $managingAdvisorAttr, diff --git a/tests/Provider/TestHydrator.php b/tests/Provider/TestHydrator.php deleted file mode 100644 index 8e2ce279..00000000 --- a/tests/Provider/TestHydrator.php +++ /dev/null @@ -1,28 +0,0 @@ - $class The FQN - * - * @return T[] The list of object - */ - public function hydrate(array $rows, string $class): array - { - /** @var T[] $result */ - $result = []; - - foreach ($rows as $row) { - $result[] = new $class($row); - } - - return $result; - } -} diff --git a/tests/Provider/User/UserChangePasswordProvider.php b/tests/Provider/User/UserChangePasswordProvider.php index 712da18d..8aac8f00 100644 --- a/tests/Provider/User/UserChangePasswordProvider.php +++ b/tests/Provider/User/UserChangePasswordProvider.php @@ -20,7 +20,7 @@ public static function userPasswordChangeAuthorizationCases(): array // Set different user role attributes $adminAttr = ['user_role_id' => UserRole::ADMIN, 'password_hash' => $passwordHash]; $managingAdvisorAttr = ['user_role_id' => UserRole::MANAGING_ADVISOR, 'password_hash' => $passwordHash]; - // If one attribute is different they are differentiated and 2 separated users are added to the db + // If one attribute is different, they are differentiated and 2 separated users are added to the db $otherManagingAdvisorAttr = [ 'first_name' => 'Other', 'user_role_id' => UserRole::MANAGING_ADVISOR, @@ -146,40 +146,4 @@ public static function invalidPasswordChangeCases(): array ], ]; } - - /** - * Provide malformed bodies for password change submit request as well as - * according error messages. - * - * @return array[] - */ - public static function malformedPasswordChangeRequestCases(): array - { - return [ - [ - // Empty body - 'requestBody' => [], - ], - [ - // Body "null" (because both can happen ) - 'requestBody' => null, - ], - // Missing 'old_password' currently not tested as it's not required when privileged user tries to - // modify other user - [ - // Missing 'password' - 'requestBody' => [ - 'old_password' => '', - 'password2' => '', - ], - ], - [ - // Missing 'password2' - 'requestBody' => [ - 'old_password' => '', - 'password' => '', - ], - ], - ]; - } } diff --git a/tests/Provider/User/UserCreateProvider.php b/tests/Provider/User/UserCreateProvider.php index c0f35ab6..ae829985 100644 --- a/tests/Provider/User/UserCreateProvider.php +++ b/tests/Provider/User/UserCreateProvider.php @@ -182,7 +182,7 @@ public static function userCreationDropdownOptionsCases(): array $advisorAttributes = ['user_role_id' => UserRole::ADVISOR]; // Newcomer must not receive any available user - // Advisor is allowed to create client but only assign it to himself or leave user_id empty + // Advisor is allowed to create a client but only assign it to himself or leave user_id empty // Managing advisor and higher should receive all available users return [ [ // ? Advisor - not allowed to create user so no available roles diff --git a/tests/Provider/User/UserListProvider.php b/tests/Provider/User/UserListProvider.php index e7deb04b..7c87a61b 100644 --- a/tests/Provider/User/UserListProvider.php +++ b/tests/Provider/User/UserListProvider.php @@ -21,7 +21,7 @@ public static function userListAuthorizationCases(): array $advisorAttr = ['user_role_id' => UserRole::ADVISOR]; $newcomerAttr = ['user_role_id' => UserRole::NEWCOMER]; - // Testing authorization: with the lowest allowed privilege and with highest not allowed + // Testing authorization: with the lowest allowed privilege and with the highest not allowed return [ // User owner is the user itself [// ? advisor owner and newcomer other - not allowed to read other - only allowed to read own status and role 'userRow' => $newcomerAttr, diff --git a/tests/Trait/AppTestTrait.php b/tests/Trait/AppTestTrait.php index b018d8ce..f0ee81d3 100644 --- a/tests/Trait/AppTestTrait.php +++ b/tests/Trait/AppTestTrait.php @@ -46,7 +46,7 @@ protected function setUp(): void // Create tables $this->setUpDatabase($this->container->get('settings')['root_dir'] . '/resources/schema/schema.sql'); - // If fixtureTestTrait is included in the test class, insert default user roles + // If DatabaseTestTrait is included in the test class (function below exits), insert default user roles if (method_exists($this, 'insertDefaultFixtureRecords')) { // Automatically insert user roles $this->insertDefaultFixtureRecords([UserRoleFixture::class]); diff --git a/tests/Unit/Security/SecurityLoginCheckerTest.php b/tests/Unit/Security/SecurityLoginCheckerTest.php index ce531f85..9f60b102 100644 --- a/tests/Unit/Security/SecurityLoginCheckerTest.php +++ b/tests/Unit/Security/SecurityLoginCheckerTest.php @@ -17,7 +17,7 @@ /** * Threats: * - Rapid fire attacks (when bots try to log in with 1000 different passwords on one user account) - * - Password spraying attacks (try to log in 1000 different users with most common password). + * - Password spraying attacks (try to log in 1000 different users with the most common password). * * Testing whole function performLoginSecurityCheck() and performEmailAbuseCheck() and not sub-functions directly as * they are private mainly because here (https://stackoverflow.com/a/2798203/9013718 comments), they say: