Retrieve user ip and id in middleware [SLE-193]

composer.json

@@ -9,7 +9,7 @@
     "php-di/php-di": "^7.0",
     "ext-pdo": "*",
     "ext-json": "*",
-    "cakephp/database": "^4",
+    "cakephp/database": "4.4.17",
     "odan/session": "^6",
     "slim/php-view": "^3.0",
     "selective/basepath": "^2.0",

config/local/env.test.php

@@ -8,8 +8,6 @@
 // Optional setting but used in unit test
 $settings['security']['global_daily_email_threshold'] = 300;
 $settings['security']['global_monthly_email_threshold'] = 1000;
-// Throttling of successful requests has to be enabled as it's tested
-$settings['security']['throttle_login_success'] = true;
 
 // Using the null adapter to prevent emails from actually being sent
 $settings['smtp']['type'] = 'null';

config/middleware.php

@@ -16,7 +16,10 @@
     // Language middleware has to be after PhpViewExtensionMiddleware as it needs the $route parameter
     $app->add(\App\Application\Middleware\LocaleMiddleware::class);
 
-    // Put everything possible before PhpViewExtensionMiddleware as if there is an error in a middleware,
+    // Retrieve and store ip address, user agent and user id (has to be BEFORE SessionStartMiddleware as it is using it)
+    $app->add(\App\Application\Middleware\UserNetworkSessionDataMiddleware::class);
+
+    // ? Put everything possible before PhpViewExtensionMiddleware as if there is an error in a middleware,
     // the error page (and layout as well as everything else) needs this middleware loaded to work.
     $app->add(PhpViewExtensionMiddleware::class);
 

src/Application/Data/UserNetworkSessionData.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace App\Application\Data;
+
+/**
+ * DTO that contains client's network data such as
+ * IP address and user agent and user identity id.
+ */
+class UserNetworkSessionData
+{
+    // Initialize vars with default values
+    public ?string $ipAddress = null;
+    public ?string $userAgent = null;
+    public null|int|string $userId = null;
+}

src/Application/Middleware/UserNetworkSessionDataMiddleware.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace App\Application\Middleware;
+
+use App\Application\Data\UserNetworkSessionData;
+use Odan\Session\SessionInterface;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\MiddlewareInterface;
+use Psr\Http\Server\RequestHandlerInterface;
+
+/**
+ * Middleware that adds client network data such as IP address and user agent
+ * as well as user identity id to the clientNetworkData DTO.
+ */
+class UserNetworkSessionDataMiddleware implements MiddlewareInterface
+{
+    public function __construct(
+        private readonly UserNetworkSessionData $clientNetworkData,
+        private readonly SessionInterface $session,
+    ) {
+    }
+
+    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
+    {
+        // Server params will be null in testing
+        $ipAddress = $request->getServerParams()['REMOTE_ADDR'];
+        $userAgent = $request->getServerParams()['HTTP_USER_AGENT'];
+
+        // Add ip address to the ipAddressData DTO object
+        $this->clientNetworkData->ipAddress = $ipAddress;
+        $this->clientNetworkData->userAgent = $userAgent;
+        $this->clientNetworkData->userId = $this->session->get('user_id');
+
+        return $handler->handle($request);
+    }
+}

src/Domain/Authentication/Service/LoginVerifier.php

@@ -2,6 +2,7 @@
 
 namespace App\Domain\Authentication\Service;
 
+use App\Application\Data\UserNetworkSessionData;
 use App\Domain\Authentication\Exception\InvalidCredentialsException;
 use App\Domain\Security\Service\SecurityLoginChecker;
 use App\Domain\User\Enum\UserActivity;
@@ -22,8 +23,8 @@ public function __construct(
         private readonly AuthenticationLoggerRepository $authenticationLoggerRepository,
         private readonly LoginNonActiveUserHandler $loginNonActiveUserHandler,
         private readonly UserActivityManager $userActivityManager,
+        private readonly UserNetworkSessionData $ipAddressData,
     ) {
-
     }
 
     /**
@@ -58,7 +59,7 @@ public function getUserIdIfAllowedToLogin(
                 // Insert login success request
                 $this->authenticationLoggerRepository->logLoginRequest(
                     $dbUser->email,
-                    $_SERVER['REMOTE_ADDR'],
+                    $this->ipAddressData->ipAddress,
                     true,
                     $dbUser->id
                 );
@@ -81,7 +82,7 @@ public function getUserIdIfAllowedToLogin(
         // Password not correct or user not existing - insert login request for ip
         $this->authenticationLoggerRepository->logLoginRequest(
             $userLoginValues['email'],
-            $_SERVER['REMOTE_ADDR'],
+            $this->ipAddressData->ipAddress,
             false,
             $dbUser->id
         );

src/Domain/Security/Service/LoginRequestFinder.php

@@ -2,6 +2,7 @@
 
 namespace App\Domain\Security\Service;
 
+use App\Application\Data\UserNetworkSessionData;
 use App\Domain\Settings;
 use App\Infrastructure\SecurityLogging\LoginLogFinderRepository;
 
@@ -11,6 +12,7 @@ class LoginRequestFinder
 
     public function __construct(
         private readonly LoginLogFinderRepository $loginRequestFinderRepository,
+        private readonly UserNetworkSessionData $ipAddressData,
         Settings $settings
     ) {
         $this->securitySettings = $settings->get('security');
@@ -31,7 +33,7 @@ public function findLoginLogEntriesInTimeLimit(string $email): array
         // Stats concerning given email in last timespan
         return $this->loginRequestFinderRepository->getLoginSummaryFromEmailAndIp(
             $email,
-            $_SERVER['REMOTE_ADDR'],
+            $this->ipAddressData->ipAddress,
             $this->securitySettings['timespan']
         );
     }
@@ -47,7 +49,7 @@ public function findLatestLoginRequestTimestamp(string $email): int
     {
         $createdAt = $this->loginRequestFinderRepository->findLatestLoginTimestampFromUserOrIp(
             $email,
-            $_SERVER['REMOTE_ADDR']
+            $this->ipAddressData->ipAddress,
         );
         if ($createdAt) {
             return (new \DateTime($createdAt))->format('U');

src/Domain/Security/Service/SecurityLoginChecker.php

@@ -83,17 +83,14 @@ public function performLoginSecurityCheck(string $email, ?string $reCaptchaRespo
      */
     private function performLoginCheck(array $loginsByIp, array $loginsByEmail, string $email): void
     {
-        $throttleSuccess = $this->securitySettings['throttle_login_success'];
         // Reverse order to compare fails the longest delay first and then go down from there
         krsort($this->securitySettings['login_throttle_rule']);
         // Fails on specific user or coming from specific IP
         foreach ($this->securitySettings['login_throttle_rule'] as $requestLimit => $delay) {
             // Check that there aren't more login successes or failures than tolerated
             if (
                 ($loginsByIp['failures'] >= $requestLimit && $loginsByIp['failures'] !== 0)
-                || ($throttleSuccess && $loginsByIp['successes'] >= $requestLimit && $loginsByIp['successes'] !== 0)
                 || ($loginsByEmail['failures'] >= $requestLimit && $loginsByEmail['failures'] !== 0)
-                || ($throttleSuccess && $loginsByEmail['successes'] >= $requestLimit && $loginsByEmail['successes'] !== 0)
             ) {
                 // If truthy means: too many ip fails OR too many ip successes
                 // OR too many failed login tries on specific user OR too many succeeding login requests on specific user

src/Domain/User/Service/UserActivityManager.php

@@ -2,6 +2,7 @@
 
 namespace App\Domain\User\Service;
 
+use App\Application\Data\UserNetworkSessionData;
 use App\Domain\User\Data\UserActivityData;
 use App\Domain\User\Enum\UserActivity;
 use App\Infrastructure\User\UserActivityRepository;
@@ -12,6 +13,7 @@ class UserActivityManager
     public function __construct(
         private readonly UserActivityRepository $userActivityRepository,
         private readonly SessionInterface $session,
+        private readonly UserNetworkSessionData $userNetworkSessionData,
     ) {
     }
 
@@ -34,9 +36,9 @@ public function addUserActivity(
         ?int $userId = null,
     ): int {
         $userActivity = new UserActivityData();
-        $userActivity->ipAddress = $_SERVER['REMOTE_ADDR'];
-        $userActivity->userAgent = $_SERVER['HTTP_USER_AGENT'] ?? null;
-        $userActivity->userId = $this->session->get('user_id') ?? $userId;
+        $userActivity->ipAddress = $this->userNetworkSessionData->ipAddress;
+        $userActivity->userAgent = $this->userNetworkSessionData->userAgent;
+        $userActivity->userId = $this->userNetworkSessionData->userId ?? $userId;
         $userActivity->action = $userActivityAction;
         $userActivity->table = $table;
         $userActivity->rowId = $rowId;

src/Infrastructure/SecurityLogging/AuthenticationLoggerRepository.php

@@ -15,12 +15,12 @@ public function __construct(
      * Insert new login request.
      *
      * @param string $email
-     * @param string $ip
+     * @param ?string $ip
      * @param bool $success whether login request was a successful login or not
      * @param int|null $userId
      * @return string
      */
-    public function logLoginRequest(string $email, string $ip, bool $success, ?int $userId = null): string
+    public function logLoginRequest(string $email, ?string $ip, bool $success, ?int $userId = null): string
     {
         $query = $this->queryFactory->newQuery();
         $query->insert(['email', 'ip_address', 'is_success', 'user_id', 'created_at'])

src/Infrastructure/SecurityLogging/LoginLogFinderRepository.php

@@ -23,16 +23,16 @@ public function __construct(
      *     logins_by_ip: array{successes: int, failures: int},
      * }
      */
-    public function getLoginSummaryFromEmailAndIp(string $email, string $ip, int $seconds): array
+    public function getLoginSummaryFromEmailAndIp(string $email, ?string $ip, int $seconds): array
     {
-        $summary = ['logins_by_ip' => [], 'logins_by_email' => []];
+        $summary = ['logins_by_ip' => ['successes' => 0, 'failures' => 0], 'logins_by_email' => []];
 
         // Only return values if not empty string as it doesn't represent a user request
         if ($email !== '') {
-            $summary['logins_by_email'] = $this->getLoginRequestStats(['email' => $email], $seconds);
+            $summary['logins_by_email'] = $this->getLoginRequestSummary(['email' => $email], $seconds);
         }
-        if ($ip !== '') {
-            $summary['logins_by_ip'] = $this->getLoginRequestStats(['ip_address' => $ip], $seconds);
+        if ($ip && $ip !== '') {
+            $summary['logins_by_ip'] = $this->getLoginRequestSummary(['ip_address' => $ip], $seconds);
         }
 
         return $summary;
@@ -46,7 +46,7 @@ public function getLoginSummaryFromEmailAndIp(string $email, string $ip, int $se
      *
      * @return array{successes: int, failures: int}
      */
-    private function getLoginRequestStats(array $whereEmailOrIpArr, int $seconds): array
+    private function getLoginRequestSummary(array $whereEmailOrIpArr, int $seconds): array
     {
         $query = $this->queryFactory->newQuery();
         $query->select(

tests/Traits/AppTestTrait.php

@@ -64,9 +64,6 @@ protected function setUp(): void
             $this->insertFixtures([UserRoleFixture::class]);
         }
 
-        // Per default not set when script executed with cli and used at least in all security checks
-        $_SERVER['REMOTE_ADDR'] = '127.0.0.1';
-
         // XDebug start_with_request produces errors when testing (SLE-102)
         // if (!isset($_ENV['AUTO_XDEBUG_DISABLED'])) {
         // Disable xdebug.start_with_request (when already disabled, delay is approx 200ms for 80 tests)