diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index ada57298a..77c27763a 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/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'); - $this->lockoutTime = config('auth.passwords.users.throttle.lockout_duration'); + // Set the custom lockout attempts from the env and sett the custom lockout throttle from the env. + // 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() { diff --git a/resources/lang/en-US/auth.php b/resources/lang/en-US/auth.php index db310aa1b..14d8acfce 100644 --- a/resources/lang/en-US/auth.php +++ b/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).', ); diff --git a/resources/lang/en-US/auth/message.php b/resources/lang/en-US/auth/message.php index 588c9069c..08f638d05 100644 --- a/resources/lang/en-US/auth/message.php +++ b/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.', diff --git a/tests/Feature/Authentication/LoginTest.php b/tests/Feature/Authentication/LoginTest.php index 85bbc78ad..2f688a59d 100644 --- a/tests/Feature/Authentication/LoginTest.php +++ b/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']);