Merge pull request #16609 from snipe/login_throttle_update

Updated login attempts and throttle duration

app/Http/Controllers/Auth/LoginController.php

@@ -284,8 +284,11 @@ class LoginController extends Controller
             return redirect()->back()->withInput()->withErrors($validator);
         }
 
-        $this->maxLoginAttempts = config('auth.passwords.users.throttle.max_attempts');
+        // Set the custom lockout attempts from the env and sett the custom lockout throttle from the env.
-        $this->lockoutTime = config('auth.passwords.users.throttle.lockout_duration');
+        // We divide decayMinutes by 60 here to get minutes, since Laravel changed the default from minutes
+        // to seconds, and we don't want to break limits on existing systems
+        $this->maxAttempts = config('auth.passwords.users.throttle.max_attempts');
+        $this->decayMinutes = (config('auth.passwords.users.throttle.lockout_duration') / 60);
 
         if ($lockedOut = $this->hasTooManyLoginAttempts($request)) {
             $this->fireLockoutEvent($request);
@@ -355,7 +358,7 @@ class LoginController extends Controller
 
         // We wouldn't normally see this page if 2FA isn't enforced via the
         // \App\Http\Middleware\CheckForTwoFactor middleware AND if a device isn't enrolled,
-        // but let's check check anyway in case there's a browser history or back button thing.
+        // but let's check anyway in case there's a browser history or back button thing.
         // While you can access this page directly, enrolling a device when 2FA isn't enforced
         // won't cause any harm.
 
@@ -521,45 +524,6 @@ class LoginController extends Controller
         return 'username';
     }
 
-    /**
-     * Redirect the user after determining they are locked out.
-     *
-     * @param  \Illuminate\Http\Request  $request
-     * @return \Illuminate\Http\RedirectResponse
-     */
-    protected function sendLockoutResponse(Request $request)
-    {
-        $seconds = $this->limiter()->availableIn(
-            $this->throttleKey($request)
-        );
-
-        $minutes = round($seconds / 60);
-
-        $message = trans('auth/message.throttle', ['minutes' => $minutes]);
-
-        return redirect()->back()
-            ->withInput($request->only($this->username(), 'remember'))
-            ->withErrors([$this->username() => $message]);
-    }
-
-
-    /**
-     * Override the lockout time and duration
-     *
-     * @param  \Illuminate\Http\Request  $request
-     * @return bool
-     */
-    protected function hasTooManyLoginAttempts(Request $request)
-    {
-        $lockoutTime = config('auth.passwords.users.throttle.lockout_duration');
-        $maxLoginAttempts = config('auth.passwords.users.throttle.max_attempts');
-
-        return $this->limiter()->tooManyAttempts(
-            $this->throttleKey($request),
-            $maxLoginAttempts,
-            $lockoutTime
-        );
-    }
 
     public function legacyAuthRedirect()
     {

resources/lang/en-US/auth.php

@@ -15,6 +15,6 @@ return array(
 
     'failed' => 'These credentials do not match our records.',
     'password' => 'The provided password is incorrect.',
-    'throttle' => 'Too many login attempts. Please try again in :seconds seconds.',
+    'throttle' => 'Too many login attempts. Please try again in :minutes minute(s).',
 
 );

resources/lang/en-US/auth/message.php

@@ -6,8 +6,6 @@ return array(
     'account_not_found'      => 'The username or password is incorrect.',
     'account_not_activated'  => 'This user account is not activated.',
     'account_suspended'      => 'This user account is suspended.',
-    'account_banned'         => 'This user account is banned.',
-    'throttle'               => 'Too many failed login attempts. Please try again in :minutes minutes.',
 
     'two_factor' => array(
         'already_enrolled'      => 'Your device is already enrolled.',

tests/Feature/Authentication/LoginTest.php

@@ -9,6 +9,7 @@ class LoginTest extends TestCase
 {
     public function testLogsFailedLoginAttempt()
     {
+
         User::factory()->create(['username' => 'username_here']);
 
         $this->withServerVariables(['REMOTE_ADDR' => '127.0.0.100'])
@@ -27,6 +28,38 @@ class LoginTest extends TestCase
         ]);
     }
 
+
+    public function testLoginThrottleConfigIsRespected()
+    {
+
+       User::factory()->create(['username' => 'username_here']);
+
+       config(['auth.passwords.users.throttle.max_attempts' => 1]);
+       config(['auth.passwords.users.throttle.lockout_duration' => 1]);
+
+        for ($i = 0; $i < 2; ++$i) {
+            $this->from('/login')
+                ->withServerVariables(['REMOTE_ADDR' => '127.0.0.100'])
+                ->post('/login', [
+                    'username' => 'invalid username',
+                    'password' => 'invalid password',
+                ]);
+        }
+
+
+        $response = $this->from('/login')
+            ->withServerVariables(['REMOTE_ADDR' => '127.0.0.100'])
+            ->post('/login', [
+                'username' => 'invalid username',
+                'password' => 'invalid password',
+            ])
+            ->assertSessionHasErrors(['username'])
+            ->assertStatus(302)
+            ->assertRedirect('/login');
+
+        $this->followRedirects($response)->assertSee(trans('auth.throttle', ['minutes' => 1]));
+    }
+
     public function testLogsSuccessfulLogin()
     {
         User::factory()->create(['username' => 'username_here']);