Merge pull request #11358 from snipe/fixes/missing_token_lang

Fixed missing password.token string and checked for user existing before attempting to send reset email
This commit is contained in:
snipe 2022-06-22 11:15:08 -07:00 committed by GitHub
commit 89c234b1c2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 126 additions and 42 deletions

View file

@ -146,7 +146,13 @@ AWS_DEFAULT_REGION=null
# -------------------------------------------- # --------------------------------------------
LOGIN_MAX_ATTEMPTS=5 LOGIN_MAX_ATTEMPTS=5
LOGIN_LOCKOUT_DURATION=60 LOGIN_LOCKOUT_DURATION=60
RESET_PASSWORD_LINK_EXPIRES=900
# --------------------------------------------
# OPTIONAL: FORGOTTEN PASSWORD SETTINGS
# --------------------------------------------
RESET_PASSWORD_LINK_EXPIRES=15
PASSWORD_CONFIRM_TIMEOUT=10800
PASSWORD_RESET_MAX_ATTEMPTS_PER_MIN=50
# -------------------------------------------- # --------------------------------------------
# OPTIONAL: MISC # OPTIONAL: MISC

View file

@ -24,6 +24,7 @@ class Kernel extends ConsoleKernel
$schedule->command('snipeit:backup')->weekly(); $schedule->command('snipeit:backup')->weekly();
$schedule->command('backup:clean')->daily(); $schedule->command('backup:clean')->daily();
$schedule->command('snipeit:upcoming-audits')->daily(); $schedule->command('snipeit:upcoming-audits')->daily();
$schedule->command('auth:clear-resets')->everyFifteenMinutes();
} }
/** /**

View file

@ -136,13 +136,12 @@ class LoginController extends Controller
// Better logging // Better logging
if (!$saml->isEnabled()) { 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 { } else {
\Log::warning("SAML page requested, but samlData seems empty."); \Log::warning("SAML page requested, but samlData seems empty.");
} }
} }
\Log::warning("Something else went wrong while trying to login as SAML user");
} }

View file

@ -3,13 +3,11 @@
namespace App\Http\Controllers\Auth; namespace App\Http\Controllers\Auth;
use App\Http\Controllers\Controller; use App\Http\Controllers\Controller;
use App\Http\Requests\SaveUserRequest;
use App\Models\Setting; use App\Models\Setting;
use App\Models\User; use App\Models\User;
use Illuminate\Foundation\Auth\ResetsPasswords; use Illuminate\Foundation\Auth\ResetsPasswords;
use Illuminate\Http\Request; use Illuminate\Http\Request;
use Illuminate\Validation\Rule;
use Illuminate\Validation\Validator;
class ResetPasswordController extends Controller class ResetPasswordController extends Controller
{ {
@ -63,6 +61,14 @@ class ResetPasswordController extends Controller
public function showResetForm(Request $request, $token = null) 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( return view('auth.passwords.reset')->with(
[ [
'token' => $token, 'token' => $token,
@ -73,38 +79,53 @@ class ResetPasswordController extends Controller
public function reset(Request $request) public function reset(Request $request)
{ {
$broker = $this->broker();
$messages = [ $messages = [
'password.not_in' => trans('validation.disallow_same_pwd_as_user_fields'), 'password.not_in' => trans('validation.disallow_same_pwd_as_user_fields'),
]; ];
$request->validate($this->rules(), $request->all(), $this->validationErrorMessages()); $request->validate($this->rules(), $request->all(), $this->validationErrorMessages());
// Check to see if the user even exists \Log::debug('Checking if '.$request->input('username').' exists');
$user = User::where('username', '=', $request->input('username'))->first(); // 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'))->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(
[
'password' => 'required|notIn:["'.$user->email.'","'.$user->username.'","'.$user->first_name.'","'.$user->last_name.'"',
], $messages);
}
// set the response
$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()->guest('login')->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'));
$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);
} }
$response = $broker->reset(
$this->credentials($request), function ($user, $password) {
$this->resetPassword($user, $password);
}
);
return $response == \Password::PASSWORD_RESET \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');
? $this->sendResetResponse($request, $response) return redirect()->guest('login')->with('success', trans('passwords.reset'));
: $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)]);
}
} }

View file

@ -75,12 +75,22 @@ class RouteServiceProvider extends ServiceProvider
/** /**
* Configure the rate limiters for the application. * Configure the rate limiters for the application.
* *
* https://laravel.com/docs/8.x/routing#rate-limiting
*
* @return void * @return void
*/ */
protected function configureRateLimiting() protected function configureRateLimiting()
{ {
// Rate limiter for API calls
RateLimiter::for('api', function (Request $request) { 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());
});
} }
} }

View file

@ -98,14 +98,27 @@ return [
'email' => 'auth.emails.password', 'email' => 'auth.emails.password',
'table' => 'password_resets', 'table' => 'password_resets',
'expire' => env('RESET_PASSWORD_LINK_EXPIRES', 900), 'expire' => env('RESET_PASSWORD_LINK_EXPIRES', 900),
'throttle' => 60, 'throttle' => [
'max_attempts' => env('LOGIN_MAX_ATTEMPTS', 5), 'max_attempts' => env('LOGIN_MAX_ATTEMPTS', 5),
'lockout_duration' => env('LOGIN_LOCKOUT_DURATION', 60), 'lockout_duration' => env('LOGIN_LOCKOUT_DURATION', 60),
]
], ],
], ],
/*
|--------------------------------------------------------------------------
| Resetting Password Requests
|--------------------------------------------------------------------------
| This sets the throttle for forgotten password requests
|
*/
'password_reset' => [
'max_attempts_per_min' => env('PASSWORD_RESET_MAX_ATTEMPTS_PER_MIN', 50),
],
/* /*
|-------------------------------------------------------------------------- |--------------------------------------------------------------------------
| Password Confirmation Timeout | Password Confirmation Timeout
@ -117,6 +130,6 @@ return [
| |
*/ */
'password_timeout' => 10800, 'password_timeout' => env('PASSWORD_CONFIRM_TIMEOUT', 10800),
]; ];

View file

@ -1,6 +1,8 @@
<?php <?php
return [ return [
'sent' => 'Success: If that email address exists in our system, a password recovery email has been sent.', 'sent' => 'If a matching user with a valid email address exists in our system, a password recovery email has been sent.',
'user' => 'No matching active user found with that email.', '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!',
]; ];

View file

@ -14,11 +14,8 @@ return array(
*/ */
"password" => "Passwords must be six characters and match the confirmation.", "password" => "Passwords must be six characters and match the confirmation.",
"user" => "Username or email address is incorrect", "user" => "Username or email address is incorrect",
"token" => 'This password reset token is invalid or expired, or does not match the username provided.',
"token" => "This password reset token is invalid.", 'sent' => 'If a matching user with a valid email address exists in our system, a password recovery email has been sent.',
"sent" => "If a matching email address was found, a password reminder has been sent!",
); );

View file

@ -56,7 +56,7 @@
@if (($snipeSettings) && ($snipeSettings->logo!='')) @if (($snipeSettings) && ($snipeSettings->logo!=''))
<center> <center>
<img id="login-logo" src="{{ Storage::disk('public')->url('').e($snipeSettings->logo) }}"> <a href="{{ config('app.url') }}"><img id="login-logo" src="{{ Storage::disk('public')->url('').e($snipeSettings->logo) }}"></a>
</center> </center>
@endif @endif
<!-- Content --> <!-- Content -->

View file

@ -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 () { Route::get('/', function () {

View file

@ -20,6 +20,8 @@ use App\Http\Controllers\StatuslabelsController;
use App\Http\Controllers\SuppliersController; use App\Http\Controllers\SuppliersController;
use App\Http\Controllers\ViewAssetsController; use App\Http\Controllers\ViewAssetsController;
use App\Http\Controllers\Auth\LoginController; 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\Route;
use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Auth;
@ -426,6 +428,39 @@ Route::group(['middleware' => 'web'], function () {
[LoginController::class, 'postTwoFactorAuth'] [LoginController::class, 'postTwoFactorAuth']
); );
Route::post(
'password/email',
[ForgotPasswordController::class, 'sendResetLinkEmail']
)->name('password.email')->middleware('throttle:forgotten_password');
Route::get(
'password/reset',
[ForgotPasswordController::class, 'showLinkRequestForm']
)->name('password.request')->middleware('throttle:forgotten_password');
Route::post(
'password/reset',
[ResetPasswordController::class, 'reset']
)->name('password.update')->middleware('throttle:forgotten_password');
Route::get(
'password/reset/{token}',
[ResetPasswordController::class, 'showResetForm']
)->name('password.reset');
Route::post(
'password/email',
[ForgotPasswordController::class, 'sendResetLinkEmail']
)->name('password.email')->middleware('throttle:forgotten_password');
Route::get( Route::get(
'/', '/',
[ [
@ -446,7 +481,7 @@ Route::group(['middleware' => 'web'], function () {
)->name('logout'); )->name('logout');
}); });
Auth::routes(); //Auth::routes();
Route::get( Route::get(
'/health', '/health',