From d0936b92c17993a3dd0a5ccb5b01a195fd7f34f2 Mon Sep 17 00:00:00 2001
From: Milos Stojanovic <alcalbg@gmail.com>
Date: Fri, 20 Jan 2023 16:45:35 +0100
Subject: [PATCH] lockout bugfix and version bump

---
 CHANGELOG.md                           |  3 ++
 backend/Controllers/AuthController.php | 16 +++++-----
 dist/index.php                         |  2 +-
 tests/backend/Feature/AuthTest.php     | 41 +++++++++++++++-----------
 4 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 16d4dc5..8a2ddbc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## Upcoming...
 
+## 7.9.1 - 2023-01-20
+* Lockout bugfix
+
 ## 7.9.0 - 2023-01-20
 * Added configurable lockout for incorrect login attempts (see configuration_sample.php)
 
diff --git a/backend/Controllers/AuthController.php b/backend/Controllers/AuthController.php
index daacc1e..348eabb 100644
--- a/backend/Controllers/AuthController.php
+++ b/backend/Controllers/AuthController.php
@@ -33,14 +33,6 @@ class AuthController
         $password = $request->input('password');
         $ip = $request->getClientIp();
 
-        if ($auth->authenticate($username, $password)) {
-            $this->logger->log("Logged in {$username} from IP ".$ip);
-
-            return $response->json($auth->user());
-        }
-
-        $this->logger->log("Login failed for {$username} from IP ".$ip);
-
         $lockfile = md5($ip).'.lock';
         $lockout_attempts = $config->get('lockout_attempts', 5);
         $lockout_timeout = $config->get('lockout_timeout', 15);
@@ -55,6 +47,14 @@ class AuthController
             return $response->json('Not Allowed', 429);
         }
 
+        if ($auth->authenticate($username, $password)) {
+            $this->logger->log("Logged in {$username} from IP ".$ip);
+
+            return $response->json($auth->user());
+        }
+
+        $this->logger->log("Login failed for {$username} from IP ".$ip);
+
         $tmpfs->write($lockfile, 'x', true);
 
         return $response->json('Login failed, please try again', 422);
diff --git a/dist/index.php b/dist/index.php
index 553025a..2e3fe82 100644
--- a/dist/index.php
+++ b/dist/index.php
@@ -41,7 +41,7 @@ if (! defined('APP_PUBLIC_PATH')) {
 }
 
 define('APP_PUBLIC_DIR', __DIR__);
-define('APP_VERSION', '7.9.0');
+define('APP_VERSION', '7.9.1');
 
 use Filegator\App;
 use Filegator\Config\Config;
diff --git a/tests/backend/Feature/AuthTest.php b/tests/backend/Feature/AuthTest.php
index 39fbcdf..741fcaf 100644
--- a/tests/backend/Feature/AuthTest.php
+++ b/tests/backend/Feature/AuthTest.php
@@ -41,35 +41,42 @@ class AuthTest extends TestCase
 
     public function testBruteForceLogin()
     {
+        // standard 422 bad response code
         $this->sendRequest('POST', '/login', [
-            'username' => 'fake',
-            'password' => 'fake',
+            'username' => 'bad',
+            'password' => 'bad',
         ], [], ['REMOTE_ADDR' => '10.10.10.10']);
-        $this->assertUnprocessable();
+        $this->assertStatus(422);
 
+        // too many requests should change the response code to 429
         for ($i = 0; $i < 20; $i++) {
             $this->sendRequest('POST', '/login', [
-                'username' => 'fake',
-                'password' => 'fake',
+                'username' => 'bad',
+                'password' => 'bad',
             ], [], ['REMOTE_ADDR' => '10.10.10.10']);
         }
         $this->assertStatus(429);
 
-        for ($i = 0; $i < 20; $i++) {
-            $this->sendRequest('POST', '/login', [
-                'username' => 'fake',
-                'password' => 'fake',
-            ], [], ['REMOTE_ADDR' => '2001:db8:3333:4444:5555:6666:7777:8888']);
-        }
+        // now even the good one from this ip should fail as 429
+        $this->sendRequest('POST', '/login', [
+            'username' => 'john@example.com',
+            'password' => 'john123',
+        ], [], ['REMOTE_ADDR' => '10.10.10.10']);
         $this->assertStatus(429);
 
-
-        // new ip should be ok
+        // another ip should fail as a standard 422 bad response (unaffected)
         $this->sendRequest('POST', '/login', [
-            'username' => 'fake',
-            'password' => 'fake',
-        ], [], ['REMOTE_ADDR' => '10.10.10.1']);
-        $this->assertUnprocessable();
+            'username' => 'bad',
+            'password' => 'bad',
+        ], [], ['REMOTE_ADDR' => '2001:db8:3333:4444:5555:6666:7777:8888']);
+        $this->assertStatus(422);
+
+        // another ip with valid credentials should be ok (unaffected)
+        $this->sendRequest('POST', '/login', [
+            'username' => 'john@example.com',
+            'password' => 'john123',
+        ], [], ['REMOTE_ADDR' => '20.20.20.20']);
+        $this->assertOk();
     }
 
     public function testAlreadyLoggedIn()