From 21875100b6552e43965484fdce7aaafb133ee51c Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 14:15:38 -0700 Subject: [PATCH 01/24] Fixed missing password.token string and checked for user existing before trying to reset Signed-off-by: snipe --- .../Auth/ResetPasswordController.php | 36 ++++++++++++------- resources/lang/en/passwords.php | 3 +- resources/lang/en/reminders.php | 2 +- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/app/Http/Controllers/Auth/ResetPasswordController.php b/app/Http/Controllers/Auth/ResetPasswordController.php index 95700e2992..99c1580c10 100644 --- a/app/Http/Controllers/Auth/ResetPasswordController.php +++ b/app/Http/Controllers/Auth/ResetPasswordController.php @@ -73,6 +73,7 @@ class ResetPasswordController extends Controller public function reset(Request $request) { + $messages = [ 'password.not_in' => trans('validation.disallow_same_pwd_as_user_fields'), ]; @@ -80,27 +81,36 @@ class ResetPasswordController extends Controller $request->validate($this->rules(), $request->all(), $this->validationErrorMessages()); // Check to see if the user even exists - $user = User::where('username', '=', $request->input('username'))->first(); + if ($user = User::where('username', '=', $request->input('username'))->first()) { + $broker = $this->broker(); - $broker = $this->broker(); - if (strpos(Setting::passwordComplexityRulesSaving('store'), 'disallow_same_pwd_as_user_fields') !== false) { - $request->validate( - [ - 'password' => 'required|notIn:["'.$user->email.'","'.$user->username.'","'.$user->first_name.'","'.$user->last_name.'"', - ], $messages); - } + if (strpos(Setting::passwordComplexityRulesSaving('store'), 'disallow_same_pwd_as_user_fields') !== false) { + $request->validate( + [ + 'password' => 'required|notIn:["'.$user->email.'","'.$user->username.'","'.$user->first_name.'","'.$user->last_name.'"', + ], $messages); + } - $response = $broker->reset( - $this->credentials($request), function ($user, $password) { + $response = $broker->reset( + $this->credentials($request), function ($user, $password) { $this->resetPassword($user, $password); } ); - return $response == \Password::PASSWORD_RESET - ? $this->sendResetResponse($request, $response) - : $this->sendResetFailedResponse($request, $response); + return $response == \Password::PASSWORD_RESET + ? $this->sendResetResponse($request, $response) + : $this->sendResetFailedResponse($request, $response); + } + + // the user doesn't exist, so we're not really sending anything here + return redirect()->route('login') + ->withInput(['username'=> $request->input('username')]) + ->with('success', trans('passwords.sent')); + } + + protected function sendResetFailedResponse(Request $request, $response) { return redirect()->back() diff --git a/resources/lang/en/passwords.php b/resources/lang/en/passwords.php index 4772940015..3491f37b70 100644 --- a/resources/lang/en/passwords.php +++ b/resources/lang/en/passwords.php @@ -1,6 +1,7 @@ 'Success: If that email address exists in our system, a password recovery email has been sent.', + 'sent' => 'If that email address exists in our system, a password recovery email has been sent.', 'user' => 'No matching active user found with that email.', + "token" => "This password reset token is invalid or expired.", ]; diff --git a/resources/lang/en/reminders.php b/resources/lang/en/reminders.php index e7a476e3a2..0ca927a445 100644 --- a/resources/lang/en/reminders.php +++ b/resources/lang/en/reminders.php @@ -17,7 +17,7 @@ return array( "user" => "Username or email address is incorrect", - "token" => "This password reset token is invalid.", + "token" => "This password reset token is invalid or expired.", "sent" => "If a matching email address was found, a password reminder has been sent!", From d4c53945d9be8004847542f1e78fe8c0874a9b36 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 14:19:49 -0700 Subject: [PATCH 02/24] Tweaked language Signed-off-by: snipe --- resources/lang/en/passwords.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/lang/en/passwords.php b/resources/lang/en/passwords.php index 3491f37b70..00ca7d3015 100644 --- a/resources/lang/en/passwords.php +++ b/resources/lang/en/passwords.php @@ -1,7 +1,7 @@ 'If that email address exists in our system, a password recovery email has been sent.', - 'user' => 'No matching active user found with that email.', + 'sent' => 'If a user with a valid email address exists in our system, a password recovery email has been sent.', + 'user' => 'If a user with a valid email address exists in our system, a password recovery email has been sent.', "token" => "This password reset token is invalid or expired.", ]; From 300879847f421b6fa2c88910a44be07657055740 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 14:33:10 -0700 Subject: [PATCH 03/24] =?UTF-8?q?Added=20a=20few=20comments=20to=20make=20?= =?UTF-8?q?it=20clearer=20what=E2=80=99s=20happening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: snipe --- .../Auth/ResetPasswordController.php | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/Auth/ResetPasswordController.php b/app/Http/Controllers/Auth/ResetPasswordController.php index 99c1580c10..ec99ed9754 100644 --- a/app/Http/Controllers/Auth/ResetPasswordController.php +++ b/app/Http/Controllers/Auth/ResetPasswordController.php @@ -81,9 +81,10 @@ class ResetPasswordController extends Controller $request->validate($this->rules(), $request->all(), $this->validationErrorMessages()); // Check to see if the user even exists - if ($user = User::where('username', '=', $request->input('username'))->first()) { + if ($user = User::where('username', '=', $request->input('username'))->whereNotNull('email')->first()) { $broker = $this->broker(); + // handle the password validation rules set by the admin settings if (strpos(Setting::passwordComplexityRulesSaving('store'), 'disallow_same_pwd_as_user_fields') !== false) { $request->validate( [ @@ -91,30 +92,25 @@ class ResetPasswordController extends Controller ], $messages); } + // send the reset $response = $broker->reset( $this->credentials($request), function ($user, $password) { $this->resetPassword($user, $password); - } - ); + }); - return $response == \Password::PASSWORD_RESET - ? $this->sendResetResponse($request, $response) - : $this->sendResetFailedResponse($request, $response); } - - // the user doesn't exist, so we're not really sending anything here - return redirect()->route('login') - ->withInput(['username'=> $request->input('username')]) - ->with('success', trans('passwords.sent')); + // This is laravel magic - we override the sendResetFailedResponse further down to send a success message even if it failed + return $response == \Password::PASSWORD_RESET + ? $this->sendResetResponse($request, $response) + : $this->sendResetFailedResponse($request, $response); } - protected function sendResetFailedResponse(Request $request, $response) { return redirect()->back() ->withInput(['username'=> $request->input('username')]) - ->withErrors(['username' => trans($response), 'password' => trans($response)]); + ->with('success', trans('passwords.sent')); } } From a49ccf08632b7849216c34faafca9957a26bd4ad Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 16:13:26 -0700 Subject: [PATCH 04/24] Removed unused rules Signed-off-by: snipe --- app/Http/Controllers/Auth/ResetPasswordController.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/Http/Controllers/Auth/ResetPasswordController.php b/app/Http/Controllers/Auth/ResetPasswordController.php index ec99ed9754..8e21dafc7d 100644 --- a/app/Http/Controllers/Auth/ResetPasswordController.php +++ b/app/Http/Controllers/Auth/ResetPasswordController.php @@ -3,13 +3,11 @@ namespace App\Http\Controllers\Auth; use App\Http\Controllers\Controller; -use App\Http\Requests\SaveUserRequest; use App\Models\Setting; use App\Models\User; use Illuminate\Foundation\Auth\ResetsPasswords; use Illuminate\Http\Request; -use Illuminate\Validation\Rule; -use Illuminate\Validation\Validator; + class ResetPasswordController extends Controller { From f4f400ed879606b6bf77adfaf90345bab527b3c5 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 16:13:43 -0700 Subject: [PATCH 05/24] Handle workflow better for invalid users Signed-off-by: snipe --- .../Auth/ResetPasswordController.php | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/Auth/ResetPasswordController.php b/app/Http/Controllers/Auth/ResetPasswordController.php index 8e21dafc7d..ed8526c1b3 100644 --- a/app/Http/Controllers/Auth/ResetPasswordController.php +++ b/app/Http/Controllers/Auth/ResetPasswordController.php @@ -72,15 +72,18 @@ class ResetPasswordController extends Controller public function reset(Request $request) { + $broker = $this->broker(); + $messages = [ 'password.not_in' => trans('validation.disallow_same_pwd_as_user_fields'), ]; $request->validate($this->rules(), $request->all(), $this->validationErrorMessages()); - // Check to see if the user even exists + \Log::debug('Checking if '.$request->input('username').' exists'); + // Check to see if the user even exists - we'll treat the response the same to prevent user sniffing if ($user = User::where('username', '=', $request->input('username'))->whereNotNull('email')->first()) { - $broker = $this->broker(); + \Log::debug($user->username.' exists'); // handle the password validation rules set by the admin settings if (strpos(Setting::passwordComplexityRulesSaving('store'), 'disallow_same_pwd_as_user_fields') !== false) { @@ -90,25 +93,29 @@ class ResetPasswordController extends Controller ], $messages); } - // send the reset + // set the response + \Log::debug('Setting the broker and resetting the password'); $response = $broker->reset( $this->credentials($request), function ($user, $password) { $this->resetPassword($user, $password); }); + // Check if the password reset above actually worked + if ($response == \Password::PASSWORD_RESET) { + \Log::debug('Password reset for '.$user->username.' worked'); + return redirect('/')->with('success', trans('passwords.reset')); + } + + \Log::debug('Password reset for '.$user->username.' FAILED - this user exists but the token is not valid'); + return redirect()->back()->withInput($request->only('email'))->with('error', trans('passwords.token')); + } - // This is laravel magic - we override the sendResetFailedResponse further down to send a success message even if it failed - return $response == \Password::PASSWORD_RESET - ? $this->sendResetResponse($request, $response) - : $this->sendResetFailedResponse($request, $response); + + \Log::debug('Password reset for '.$request->input('username').' FAILED - user does not exist or does not have an email address - but make it look like it succeeded'); + return redirect()->route('login')->with('success', trans('passwords.sent')); } - protected function sendResetFailedResponse(Request $request, $response) - { - return redirect()->back() - ->withInput(['username'=> $request->input('username')]) - ->with('success', trans('passwords.sent')); - } + } From 68150d11b743d07683f3ad35a46d6044e43ecd8e Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 16:13:52 -0700 Subject: [PATCH 06/24] Make logo clickable Signed-off-by: snipe --- resources/views/layouts/basic.blade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/views/layouts/basic.blade.php b/resources/views/layouts/basic.blade.php index 7efedf73b2..1a56a77b0a 100644 --- a/resources/views/layouts/basic.blade.php +++ b/resources/views/layouts/basic.blade.php @@ -56,7 +56,7 @@ @if (($snipeSettings) && ($snipeSettings->logo!=''))
- +
@endif From de048e100911ba827f4f13304a5251580b5280de Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 16:13:59 -0700 Subject: [PATCH 07/24] Updated language Signed-off-by: snipe --- resources/lang/en/passwords.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/resources/lang/en/passwords.php b/resources/lang/en/passwords.php index 00ca7d3015..25633b4581 100644 --- a/resources/lang/en/passwords.php +++ b/resources/lang/en/passwords.php @@ -1,7 +1,8 @@ 'If a user with a valid email address exists in our system, a password recovery email has been sent.', - 'user' => 'If a user with a valid email address exists in our system, a password recovery email has been sent.', - "token" => "This password reset token is invalid or expired.", + 'sent' => 'If a matching user with a valid email address exists in our system, a password recovery email has been sent.', + 'user' => 'If a matching user with a valid email address exists in our system, a password recovery email has been sent.', + 'token' => 'This password reset token is invalid or expired, or does not match the username provided.', + 'reset' => 'Your password has been reset!', ]; From 7f8fc7add9cb0732a4b9190bee3c047f8c344a4c Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 17:57:17 -0700 Subject: [PATCH 08/24] Make SAML debugging less noisy Signed-off-by: snipe --- app/Http/Controllers/Auth/LoginController.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 9e94740506..c16f00facb 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -136,13 +136,12 @@ class LoginController extends Controller // Better logging if (!$saml->isEnabled()) { - \Log::warning("SAML page requested, but SAML does not seem to enabled."); + \Log::debug("SAML page requested, but SAML does not seem to enabled."); } else { \Log::warning("SAML page requested, but samlData seems empty."); } } - \Log::warning("Something else went wrong while trying to login as SAML user"); } From 386272a61844ecb252c0da502ca7729ee6ccc398 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 18:40:53 -0700 Subject: [PATCH 09/24] Manually add the additional routes so we can throttle them Signed-off-by: snipe --- routes/web.php | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/routes/web.php b/routes/web.php index 63019632dd..674dc07583 100644 --- a/routes/web.php +++ b/routes/web.php @@ -20,6 +20,8 @@ use App\Http\Controllers\StatuslabelsController; use App\Http\Controllers\SuppliersController; use App\Http\Controllers\ViewAssetsController; use App\Http\Controllers\Auth\LoginController; +use App\Http\Controllers\Auth\ForgotPasswordController; +use App\Http\Controllers\Auth\ResetPasswordController; use Illuminate\Support\Facades\Route; use Illuminate\Support\Facades\Auth; @@ -424,7 +426,40 @@ Route::group(['middleware' => 'web'], function () { Route::post( 'two-factor', [LoginController::class, 'postTwoFactorAuth'] - ); + )->middleware('throttle:'.config('auth.passwords.users.throttle.max_attempts').','.config('auth.passwords.users.throttle.lockout_duration')); + + + + Route::post( + 'password/email', + [ForgotPasswordController::class, 'sendResetLinkEmail'] + )->name('password.email')->middleware('throttle:'.config('auth.passwords.users.throttle.max_attempts').','.config('auth.passwords.users.throttle.lockout_duration')); + + Route::get( + 'password/reset', + [ForgotPasswordController::class, 'showLinkRequestForm'] + )->name('password.request'); + + + Route::post( + 'password/reset', + [ResetPasswordController::class, 'reset'] + )->name('password.update')->middleware('throttle:'.config('auth.passwords.users.throttle.password_max_attempts').','.config('auth.passwords.users.throttle.password_lockout_duration')); + + Route::get( + 'password/reset/{token}', + [ResetPasswordController::class, 'showResetForm'] + )->name('password.reset')->middleware('throttle:'.config('auth.passwords.users.throttle.password_max_attempts').','.config('auth.passwords.users.throttle.lockout_duration')); + + + Route::post( + 'password/email', + [ResetPasswordController::class, 'showLinkRequestForm'] + )->name('password.request')->middleware('throttle:'.config('auth.passwords.users.throttle.password_max_attempts').','.config('auth.passwords.users.throttle.password_lockout_duration')); + + + + Route::get( '/', @@ -446,7 +481,7 @@ Route::group(['middleware' => 'web'], function () { )->name('logout'); }); -Auth::routes(); +//Auth::routes(); Route::get( '/health', From 1b6df232aaab0be7eee423d0166240fbf51f229b Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 18:41:02 -0700 Subject: [PATCH 10/24] Updated string Signed-off-by: snipe --- resources/lang/en/reminders.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/lang/en/reminders.php b/resources/lang/en/reminders.php index 0ca927a445..39aff10d1c 100644 --- a/resources/lang/en/reminders.php +++ b/resources/lang/en/reminders.php @@ -17,7 +17,7 @@ return array( "user" => "Username or email address is incorrect", - "token" => "This password reset token is invalid or expired.", + "token" => 'This password reset token is invalid or expired, or does not match the username provided.', "sent" => "If a matching email address was found, a password reminder has been sent!", From 791f77f6417d649fd846e29732383db45e6e3bf7 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 18:41:12 -0700 Subject: [PATCH 11/24] Fixed throttle variables Signed-off-by: snipe --- config/auth.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/config/auth.php b/config/auth.php index 4fc626a964..8fb4d3e6fd 100644 --- a/config/auth.php +++ b/config/auth.php @@ -98,10 +98,12 @@ return [ 'email' => 'auth.emails.password', 'table' => 'password_resets', 'expire' => env('RESET_PASSWORD_LINK_EXPIRES', 900), - 'throttle' => 60, + 'throttle' => [ 'max_attempts' => env('LOGIN_MAX_ATTEMPTS', 5), - 'lockout_duration' => env('LOGIN_LOCKOUT_DURATION', 60), + 'password_max_attempts' => env('PASSWORD_MAX_ATTEMPTS', 20), + 'password_lockout_duration' => env('PASSWORD_LOCKOUT_DURATION', 60), + ] ], ], From a31bca1798abb66411f8d519d55bb8dcbaaa0edd Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 18:48:02 -0700 Subject: [PATCH 12/24] Check that the user is activated before letting them reset their password Signed-off-by: snipe --- app/Http/Controllers/Auth/ResetPasswordController.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/Auth/ResetPasswordController.php b/app/Http/Controllers/Auth/ResetPasswordController.php index ed8526c1b3..c7152c9f44 100644 --- a/app/Http/Controllers/Auth/ResetPasswordController.php +++ b/app/Http/Controllers/Auth/ResetPasswordController.php @@ -82,9 +82,10 @@ class ResetPasswordController extends Controller \Log::debug('Checking if '.$request->input('username').' exists'); // Check to see if the user even exists - we'll treat the response the same to prevent user sniffing - if ($user = User::where('username', '=', $request->input('username'))->whereNotNull('email')->first()) { + if ($user = User::where('username', '=', $request->input('username'))->where('activated', '1')->whereNotNull('email')->first()) { \Log::debug($user->username.' exists'); + // handle the password validation rules set by the admin settings if (strpos(Setting::passwordComplexityRulesSaving('store'), 'disallow_same_pwd_as_user_fields') !== false) { $request->validate( @@ -93,8 +94,8 @@ class ResetPasswordController extends Controller ], $messages); } + // set the response - \Log::debug('Setting the broker and resetting the password'); $response = $broker->reset( $this->credentials($request), function ($user, $password) { $this->resetPassword($user, $password); @@ -103,7 +104,7 @@ class ResetPasswordController extends Controller // Check if the password reset above actually worked if ($response == \Password::PASSWORD_RESET) { \Log::debug('Password reset for '.$user->username.' worked'); - return redirect('/')->with('success', trans('passwords.reset')); + return redirect()->guest('login')->with('success', trans('passwords.reset')); } \Log::debug('Password reset for '.$user->username.' FAILED - this user exists but the token is not valid'); @@ -111,8 +112,9 @@ class ResetPasswordController extends Controller } + \Log::debug('Password reset for '.$request->input('username').' FAILED - user does not exist or does not have an email address - but make it look like it succeeded'); - return redirect()->route('login')->with('success', trans('passwords.sent')); + return redirect()->guest('login')->with('success', trans('passwords.reset')); } From 2f258a3e3d4f62b0e93f78f2b205a23d182154a8 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 18:48:22 -0700 Subject: [PATCH 13/24] Make the strings match Signed-off-by: snipe --- resources/lang/en/reminders.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/resources/lang/en/reminders.php b/resources/lang/en/reminders.php index 39aff10d1c..8a197467df 100644 --- a/resources/lang/en/reminders.php +++ b/resources/lang/en/reminders.php @@ -14,11 +14,8 @@ return array( */ "password" => "Passwords must be six characters and match the confirmation.", - "user" => "Username or email address is incorrect", - "token" => 'This password reset token is invalid or expired, or does not match the username provided.', - - "sent" => "If a matching email address was found, a password reminder has been sent!", + 'sent' => 'If a matching user with a valid email address exists in our system, a password recovery email has been sent.', ); From 17ee332715b210d8949ba084ecbbfed4732379d9 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 18:53:14 -0700 Subject: [PATCH 14/24] Remove throttle from GET in password reset Signed-off-by: snipe --- routes/web.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routes/web.php b/routes/web.php index 674dc07583..c59aa8690e 100644 --- a/routes/web.php +++ b/routes/web.php @@ -449,7 +449,7 @@ Route::group(['middleware' => 'web'], function () { Route::get( 'password/reset/{token}', [ResetPasswordController::class, 'showResetForm'] - )->name('password.reset')->middleware('throttle:'.config('auth.passwords.users.throttle.password_max_attempts').','.config('auth.passwords.users.throttle.lockout_duration')); + )->name('password.reset'); Route::post( From 284dbb75532ac17bf8113cd2b3969c5ec3de8eec Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 19:11:39 -0700 Subject: [PATCH 15/24] Set higher threshhold, moved throttle settings Signed-off-by: snipe --- config/auth.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/config/auth.php b/config/auth.php index 8fb4d3e6fd..c64308f389 100644 --- a/config/auth.php +++ b/config/auth.php @@ -101,13 +101,21 @@ return [ 'throttle' => [ 'max_attempts' => env('LOGIN_MAX_ATTEMPTS', 5), 'lockout_duration' => env('LOGIN_LOCKOUT_DURATION', 60), - 'password_max_attempts' => env('PASSWORD_MAX_ATTEMPTS', 20), - 'password_lockout_duration' => env('PASSWORD_LOCKOUT_DURATION', 60), ] ], ], + + 'password_reset' => [ + 'throttle' => [ + 'max_attempts' => env('PASSWORD_MAX_ATTEMPTS', 10), + 'lockout_duration' => env('PASSWORD_LOCKOUT_DURATION', 60), + ], + ], + + + /* |-------------------------------------------------------------------------- | Password Confirmation Timeout From 172e8d463f8c8dd85c17abb306ca06167a9abafe Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 19:11:57 -0700 Subject: [PATCH 16/24] Use newer forgotten password variables Signed-off-by: snipe --- .env.example | 2 ++ routes/web.php | 10 +++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.env.example b/.env.example index ad97994320..2415070596 100644 --- a/.env.example +++ b/.env.example @@ -147,6 +147,8 @@ AWS_DEFAULT_REGION=null LOGIN_MAX_ATTEMPTS=5 LOGIN_LOCKOUT_DURATION=60 RESET_PASSWORD_LINK_EXPIRES=900 +PASSWORD_MAX_ATTEMPTS=20 +PASSWORD_LOCKOUT_DURATION=60 # -------------------------------------------- # OPTIONAL: MISC diff --git a/routes/web.php b/routes/web.php index c59aa8690e..9d5dad6487 100644 --- a/routes/web.php +++ b/routes/web.php @@ -426,14 +426,14 @@ Route::group(['middleware' => 'web'], function () { Route::post( 'two-factor', [LoginController::class, 'postTwoFactorAuth'] - )->middleware('throttle:'.config('auth.passwords.users.throttle.max_attempts').','.config('auth.passwords.users.throttle.lockout_duration')); + )->middleware('throttle:'.config('auth.password_reset.throttle.max_attempts').','.config('auth.password_reset.throttle.lockout_duration')); Route::post( 'password/email', [ForgotPasswordController::class, 'sendResetLinkEmail'] - )->name('password.email')->middleware('throttle:'.config('auth.passwords.users.throttle.max_attempts').','.config('auth.passwords.users.throttle.lockout_duration')); + )->name('password.email')->middleware('throttle:'.config('auth.password_reset.throttle.max_attempts').','.config('auth.password_reset.throttle.lockout_duration')); Route::get( 'password/reset', @@ -444,7 +444,7 @@ Route::group(['middleware' => 'web'], function () { Route::post( 'password/reset', [ResetPasswordController::class, 'reset'] - )->name('password.update')->middleware('throttle:'.config('auth.passwords.users.throttle.password_max_attempts').','.config('auth.passwords.users.throttle.password_lockout_duration')); + )->name('password.update')->middleware('throttle:'.config('auth.password_reset.throttle.max_attempts').','.config('auth.password_reset.throttle.lockout_duration')); Route::get( 'password/reset/{token}', @@ -454,8 +454,8 @@ Route::group(['middleware' => 'web'], function () { Route::post( 'password/email', - [ResetPasswordController::class, 'showLinkRequestForm'] - )->name('password.request')->middleware('throttle:'.config('auth.passwords.users.throttle.password_max_attempts').','.config('auth.passwords.users.throttle.password_lockout_duration')); + [ForgotPasswordController::class, 'sendResetLinkEmail'] + )->name('password.email')->middleware('throttle:'.config('auth.password_reset.throttle.max_attempts').','.config('auth.password_reset.throttle.lockout_duration')); From 57720cb9787a7c6ec0046c2054cae3223600ad69 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 19:12:57 -0700 Subject: [PATCH 17/24] Added comment block Signed-off-by: snipe --- config/auth.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/config/auth.php b/config/auth.php index c64308f389..509ff66e3b 100644 --- a/config/auth.php +++ b/config/auth.php @@ -106,7 +106,13 @@ return [ ], ], - + /* + |-------------------------------------------------------------------------- + | Resetting Password Requests + |-------------------------------------------------------------------------- + | This sets the throttle for forgotten password requests + | + */ 'password_reset' => [ 'throttle' => [ 'max_attempts' => env('PASSWORD_MAX_ATTEMPTS', 10), From b00db3cc5623d5f89f7cedb9b2e215c5ab9fa566 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 19:30:11 -0700 Subject: [PATCH 18/24] Added throttling to password reset token form Signed-off-by: snipe --- routes/web.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routes/web.php b/routes/web.php index 9d5dad6487..fbe02db911 100644 --- a/routes/web.php +++ b/routes/web.php @@ -438,7 +438,7 @@ Route::group(['middleware' => 'web'], function () { Route::get( 'password/reset', [ForgotPasswordController::class, 'showLinkRequestForm'] - )->name('password.request'); + )->name('password.request')->middleware('throttle:'.config('auth.password_reset.throttle.max_attempts').','.config('auth.password_reset.throttle.lockout_duration')); Route::post( From a5b857c7534d0dded06a02440745e9e76b9a91d6 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 19:30:51 -0700 Subject: [PATCH 19/24] Return error if token is incorrect Signed-off-by: snipe --- app/Http/Controllers/Auth/ResetPasswordController.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/Http/Controllers/Auth/ResetPasswordController.php b/app/Http/Controllers/Auth/ResetPasswordController.php index c7152c9f44..1405a49b83 100644 --- a/app/Http/Controllers/Auth/ResetPasswordController.php +++ b/app/Http/Controllers/Auth/ResetPasswordController.php @@ -61,6 +61,14 @@ class ResetPasswordController extends Controller public function showResetForm(Request $request, $token = null) { + + $credentials = $request->only('email', 'token'); + + if (is_null($this->broker()->getUser($credentials))) { + \Log::debug('Password reset form FAILED - this token is not valid.'); + return redirect()->route('password.request')->with('error', trans('passwords.token')); + } + return view('auth.passwords.reset')->with( [ 'token' => $token, From 1c1f3dc42c63e20b7e429e3169be5d192017238c Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 19:35:16 -0700 Subject: [PATCH 20/24] Added password requests cleanup to scheduler Signed-off-by: snipe --- app/Console/Kernel.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/Console/Kernel.php b/app/Console/Kernel.php index 9a5294c2d2..0b80d2eccd 100644 --- a/app/Console/Kernel.php +++ b/app/Console/Kernel.php @@ -24,6 +24,7 @@ class Kernel extends ConsoleKernel $schedule->command('snipeit:backup')->weekly(); $schedule->command('backup:clean')->daily(); $schedule->command('snipeit:upcoming-audits')->daily(); + $schedule->command('auth:clear-resets')->everyFifteenMinutes(); } /** From 5ff1b5fd50bccaf0daebc45787e9da7a80de27a4 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 21 Jun 2022 19:39:50 -0700 Subject: [PATCH 21/24] Increased throttle Signed-off-by: snipe --- config/auth.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/auth.php b/config/auth.php index 509ff66e3b..2bdcdb3e5b 100644 --- a/config/auth.php +++ b/config/auth.php @@ -115,7 +115,7 @@ return [ */ 'password_reset' => [ 'throttle' => [ - 'max_attempts' => env('PASSWORD_MAX_ATTEMPTS', 10), + 'max_attempts' => env('PASSWORD_MAX_ATTEMPTS', 30), 'lockout_duration' => env('PASSWORD_LOCKOUT_DURATION', 60), ], ], From 18778d372381cf4db9235efa73ee527e01185bbc Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 22 Jun 2022 09:07:48 -0700 Subject: [PATCH 22/24] Additional example variables Signed-off-by: snipe --- .env.example | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.env.example b/.env.example index 2415070596..4dac001202 100644 --- a/.env.example +++ b/.env.example @@ -146,9 +146,13 @@ AWS_DEFAULT_REGION=null # -------------------------------------------- LOGIN_MAX_ATTEMPTS=5 LOGIN_LOCKOUT_DURATION=60 -RESET_PASSWORD_LINK_EXPIRES=900 -PASSWORD_MAX_ATTEMPTS=20 -PASSWORD_LOCKOUT_DURATION=60 + +# -------------------------------------------- +# OPTIONAL: FORGOTTEN PASSWORD SETTINGS +# -------------------------------------------- +RESET_PASSWORD_LINK_EXPIRES=15 +PASSWORD_CONFIRM_TIMEOUT=10800 +PASSWORD_RESET_MAX_ATTEMPTS_PER_MIN=30 # -------------------------------------------- # OPTIONAL: MISC From a7dc6162fa8d484d8491e8e106380a61bb52c976 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 22 Jun 2022 09:11:24 -0700 Subject: [PATCH 23/24] Simplify password attempts rate limiting Signed-off-by: snipe --- .env.example | 2 +- app/Providers/RouteServiceProvider.php | 12 +++++++++++- config/auth.php | 7 ++----- routes/web.php | 10 +++++----- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/.env.example b/.env.example index 4dac001202..bd65c1935f 100644 --- a/.env.example +++ b/.env.example @@ -152,7 +152,7 @@ LOGIN_LOCKOUT_DURATION=60 # -------------------------------------------- RESET_PASSWORD_LINK_EXPIRES=15 PASSWORD_CONFIRM_TIMEOUT=10800 -PASSWORD_RESET_MAX_ATTEMPTS_PER_MIN=30 +PASSWORD_RESET_MAX_ATTEMPTS_PER_MIN=50 # -------------------------------------------- # OPTIONAL: MISC diff --git a/app/Providers/RouteServiceProvider.php b/app/Providers/RouteServiceProvider.php index 447f1bc300..90520738d0 100644 --- a/app/Providers/RouteServiceProvider.php +++ b/app/Providers/RouteServiceProvider.php @@ -75,12 +75,22 @@ class RouteServiceProvider extends ServiceProvider /** * Configure the rate limiters for the application. * + * https://laravel.com/docs/8.x/routing#rate-limiting + * * @return void */ protected function configureRateLimiting() { + + // Rate limiter for API calls RateLimiter::for('api', function (Request $request) { - return Limit::perMinute(60)->by(optional($request->user())->id ?: $request->ip()); + return Limit::perMinute(config('app.api_throttle_per_minute'))->by(optional($request->user())->id ?: $request->ip()); }); + + // Rate limiter for forgotten password requests + RateLimiter::for('forgotten_password', function (Request $request) { + return Limit::perMinute(config('auth.password_reset.max_attempts_per_min'))->by(optional($request->user())->id ?: $request->ip()); + }); + } } diff --git a/config/auth.php b/config/auth.php index 2bdcdb3e5b..7f580d68b1 100644 --- a/config/auth.php +++ b/config/auth.php @@ -114,10 +114,7 @@ return [ | */ 'password_reset' => [ - 'throttle' => [ - 'max_attempts' => env('PASSWORD_MAX_ATTEMPTS', 30), - 'lockout_duration' => env('PASSWORD_LOCKOUT_DURATION', 60), - ], + 'max_attempts_per_min' => env('PASSWORD_RESET_MAX_ATTEMPTS_PER_MIN', 50), ], @@ -133,6 +130,6 @@ return [ | */ - 'password_timeout' => 10800, + 'password_timeout' => env('PASSWORD_CONFIRM_TIMEOUT', 10800), ]; diff --git a/routes/web.php b/routes/web.php index fbe02db911..7a6d8caa01 100644 --- a/routes/web.php +++ b/routes/web.php @@ -426,25 +426,25 @@ Route::group(['middleware' => 'web'], function () { Route::post( 'two-factor', [LoginController::class, 'postTwoFactorAuth'] - )->middleware('throttle:'.config('auth.password_reset.throttle.max_attempts').','.config('auth.password_reset.throttle.lockout_duration')); + ); Route::post( 'password/email', [ForgotPasswordController::class, 'sendResetLinkEmail'] - )->name('password.email')->middleware('throttle:'.config('auth.password_reset.throttle.max_attempts').','.config('auth.password_reset.throttle.lockout_duration')); + )->name('password.email')->middleware('throttle:forgotten_password'); Route::get( 'password/reset', [ForgotPasswordController::class, 'showLinkRequestForm'] - )->name('password.request')->middleware('throttle:'.config('auth.password_reset.throttle.max_attempts').','.config('auth.password_reset.throttle.lockout_duration')); + )->name('password.request')->middleware('throttle:forgotten_password'); Route::post( 'password/reset', [ResetPasswordController::class, 'reset'] - )->name('password.update')->middleware('throttle:'.config('auth.password_reset.throttle.max_attempts').','.config('auth.password_reset.throttle.lockout_duration')); + )->name('password.update')->middleware('throttle:forgotten_password'); Route::get( 'password/reset/{token}', @@ -455,7 +455,7 @@ Route::group(['middleware' => 'web'], function () { Route::post( 'password/email', [ForgotPasswordController::class, 'sendResetLinkEmail'] - )->name('password.email')->middleware('throttle:'.config('auth.password_reset.throttle.max_attempts').','.config('auth.password_reset.throttle.lockout_duration')); + )->name('password.email')->middleware('throttle:forgotten_password'); From 5c30de517d6d87321b09fbc0c2ac0e91f582ffee Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 22 Jun 2022 09:11:40 -0700 Subject: [PATCH 24/24] Use rate limiter for API calls Signed-off-by: snipe --- routes/api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routes/api.php b/routes/api.php index 921af2f1c3..29af435687 100644 --- a/routes/api.php +++ b/routes/api.php @@ -16,7 +16,7 @@ use Illuminate\Support\Facades\Route; | */ -Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:'.config('app.api_throttle_per_minute').',1']], function () { +Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:api']], function () { Route::get('/', function () {