diff --git a/app/Http/Controllers/AuthController.php b/app/Http/Controllers/AuthController.php index ad26fe80..a6401b48 100644 --- a/app/Http/Controllers/AuthController.php +++ b/app/Http/Controllers/AuthController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers; +use URL; use Log; use Mail; use View; @@ -181,13 +182,10 @@ class AuthController extends Controller if (! $user) return json(trans('auth.forgot.unregistered'), 1); - // Generate token for password resetting - $token = base64_encode($user->getToken().substr(time(), 4, 6).str_random(16)); + $url = URL::temporarySignedRoute('auth.reset', now()->addHour(), ['uid' => $user->uid]); try { - Mail::to($request->input('email'))->send(new ForgotPassword($user->uid, $token)); - - Log::info("[Password Reset] Mail has been sent to [{$request->input('email')}] with token [$token]"); + Mail::to($request->input('email'))->send(new ForgotPassword($url)); } catch (\Exception $e) { // Write the exception to log app(\Illuminate\Foundation\Exceptions\Handler::class)->report($e); @@ -200,63 +198,18 @@ class AuthController extends Controller return json(trans('auth.mail.success'), 0); } - public function reset(UserRepository $users, Request $request) + public function reset($uid, UserRepository $users) { - if ($request->has('uid') && $request->has('token')) { - // Get user instance from repository - $user = $users->get($request->input('uid')); - - if (! $user) - return redirect('auth/forgot')->with('msg', trans('auth.reset.invalid')); - - // Unpack to get user token & timestamp - $decoded = base64_decode($request->input('token')); - $token = substr($decoded, 0, -22); - $timestamp = substr($decoded, strlen($token), 6); - - if ($user->getToken() != $token) { - return redirect('auth/forgot')->with('msg', trans('auth.reset.invalid')); - } - - // More than 1 hour - if ((substr(time(), 4, 6) - $timestamp) > 3600) { - return redirect('auth/forgot')->with('msg', trans('auth.reset.expired')); - } - - return view('auth.reset')->with('user', $user); - } else { - return redirect('auth/login')->with('msg', trans('auth.check.anonymous')); - } + return view('auth.reset')->with('user', $users->get($uid)); } - public function handleReset(Request $request, UserRepository $users) + public function handleReset($uid, Request $request, UserRepository $users) { - $this->validate($request, [ - 'uid' => 'required|integer', + $validated = $this->validate($request, [ 'password' => 'required|min:8|max:32', - 'token' => 'required', ]); - $decoded = base64_decode($request->input('token')); - $token = substr($decoded, 0, -22); - $timestamp = intval(substr($decoded, strlen($token), 6)); - - $user = $users->get($request->input('uid')); - if (! $user) - return json(trans('auth.reset.invalid'), 1); - - if ($user->getToken() != $token) { - return json(trans('auth.reset.invalid'), 1); - } - - // More than 1 hour - if ((intval(substr(time(), 4, 6)) - $timestamp) > 3600) { - return json(trans('auth.reset.expired'), 1); - } - - $users->get($request->input('uid'))->changePasswd($request->input('password')); - - Log::info("[Password Reset] Password of user [{$request->input('uid')}] has been changed"); + $users->get($uid)->changePasswd($validated['password']); return json(trans('auth.reset.success'), 0); } diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 9abf27c1..3edf5344 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -49,5 +49,6 @@ class Kernel extends HttpKernel 'admin' => \App\Http\Middleware\CheckAdministrator::class, 'player' => \App\Http\Middleware\CheckPlayerExist::class, 'setup' => \App\Http\Middleware\CheckInstallation::class, + 'signed' => \Illuminate\Routing\Middleware\ValidateSignature::class, ]; } diff --git a/app/Mail/ForgotPassword.php b/app/Mail/ForgotPassword.php index 82159ecf..b4fcfe41 100644 --- a/app/Mail/ForgotPassword.php +++ b/app/Mail/ForgotPassword.php @@ -19,13 +19,12 @@ class ForgotPassword extends Mailable /** * Create a new message instance. * - * @param int $uid - * @param string $token + * @param string $url * @return void */ - public function __construct(int $uid, string $token) + public function __construct(string $url) { - $this->reset_url = option('site_url')."/auth/reset?uid=$uid&token=$token"; + $this->reset_url = $url; } /** diff --git a/resources/assets/src/js/__tests__/auth.test.js b/resources/assets/src/js/__tests__/auth.test.js index 731776ea..43546a07 100644 --- a/resources/assets/src/js/__tests__/auth.test.js +++ b/resources/assets/src/js/__tests__/auth.test.js @@ -327,7 +327,9 @@ describe('tests for "reset" module', () => { const url = jest.fn(path => path); const swal = jest.fn().mockReturnValue(Promise.resolve()); const showMsg = jest.fn(); - const getQueryString = jest.fn().mockReturnValue('token'); + const getQueryString = jest.fn() + .mockReturnValueOnce('expires') + .mockReturnValueOnce('signature'); const showAjaxError = jest.fn(); window.fetch = fetch; window.trans = trans; @@ -377,15 +379,14 @@ describe('tests for "reset" module', () => { $('#confirm-pwd').val('password'); await $('button').click(); - expect(getQueryString).toBeCalledWith('token'); + expect(getQueryString.mock.calls[0][0]).toBe('expires'); + expect(getQueryString.mock.calls[1][0]).toBe('signature'); expect(fetch).toBeCalledWith(expect.objectContaining({ type: 'POST', - url: 'auth/reset', + url: 'auth/reset/1?expires=expires&signature=signature', dataType: 'json', data: { - uid: '1', - password: 'password', - token: 'token' + password: 'password' } })); expect($('button').html()).toBe( diff --git a/resources/assets/src/js/auth/reset.js b/resources/assets/src/js/auth/reset.js index 49728d13..a97d30f2 100644 --- a/resources/assets/src/js/auth/reset.js +++ b/resources/assets/src/js/auth/reset.js @@ -2,11 +2,9 @@ $('#reset-button').click(e => { e.preventDefault(); - + const data = { - uid: $('#uid').val(), password: $('#password').val(), - token: getQueryString('token') }; (function validate({ password }, callback) { @@ -29,9 +27,9 @@ $('#reset-button').click(e => { try { const { errno, msg } = await fetch({ type: 'POST', - url: url('auth/reset'), + url: `${url('auth/reset')}/${$('#uid').val()}?expires=${getQueryString('expires')}&signature=${getQueryString('signature')}`, dataType: 'json', - data: data, + data, beforeSend: () => { $('#reset-button').html( ' ' + trans('auth.resetting') diff --git a/routes/web.php b/routes/web.php index 18e5b3fd..493e9b08 100644 --- a/routes/web.php +++ b/routes/web.php @@ -23,10 +23,10 @@ Route::group(['prefix' => 'auth'], function () { Route::group(['middleware' => 'guest'], function () { - Route::view('/login', 'auth.login'); - Route::get ('/register', 'AuthController@register'); - Route::get ('/forgot', 'AuthController@forgot'); - Route::get ('/reset', 'AuthController@reset'); + Route::view('/login', 'auth.login'); + Route::get ('/register', 'AuthController@register'); + Route::get ('/forgot', 'AuthController@forgot'); + Route::get ('/reset/{uid}', 'AuthController@reset')->name('auth.reset')->middleware('signed'); }); Route::any('/logout', 'AuthController@logout'); @@ -36,7 +36,7 @@ Route::group(['prefix' => 'auth'], function () Route::post('/register', 'AuthController@handleRegister'); Route::post('/forgot', 'AuthController@handleForgot'); - Route::post('/reset', 'AuthController@handleReset'); + Route::post('/reset/{uid}', 'AuthController@handleReset')->middleware('signed'); }); /** diff --git a/tests/AuthControllerTest.php b/tests/AuthControllerTest.php index e2c17ca3..baf0e96e 100644 --- a/tests/AuthControllerTest.php +++ b/tests/AuthControllerTest.php @@ -441,11 +441,6 @@ class AuthControllerTest extends TestCase 'msg' => trans('auth.forgot.unregistered') ]); - $uid = $user->uid; - $token = base64_encode( - $user->getToken().substr(time(), 4, 6).str_random(16) - ); - $url = Option::get('site_url')."/auth/reset?uid=$uid&token=$token"; $this->postJson('/auth/forgot', [ 'email' => $user->email, 'captcha' => 'a' @@ -453,8 +448,8 @@ class AuthControllerTest extends TestCase 'errno' => 0, 'msg' => trans('auth.mail.success') ])->assertSessionHas('last_mail_time'); - Mail::assertSent(ForgotPassword::class, function ($mail) use ($url, $user) { - return stristr($url, $mail->reset_url) == 0 && $mail->hasTo($user->email); + Mail::assertSent(ForgotPassword::class, function ($mail) use ($user) { + return $mail->hasTo($user->email); }); // Should handle exception when sending email @@ -476,59 +471,19 @@ class AuthControllerTest extends TestCase { $user = factory(User::class)->create(); - // Should be redirected if `uid` or `token` is empty - $this->get('/auth/reset') - ->assertRedirect('/auth/login'); - - // Should be redirected if `uid` is invalid - $this->get('/auth/reset?uid=-1&token=nothing') - ->assertRedirect('/auth/forgot'); - - // Should be redirected if `token` is invalid - $this->get('/auth/reset?uid=' . $user->uid . '&token=nothing') - ->assertRedirect('/auth/forgot'); - - // Should be redirected if expired - $token = base64_encode( - $user->getToken().substr(time() - 60 * 60 * 2, 4, 6).str_random(16) - ); - $this->get('/auth/reset?uid=' . $user->uid . '&token=' . $token) - ->assertRedirect('/auth/forgot'); - - // Success - $token = base64_encode( - $user->getToken().substr(time(), 4, 6).str_random(16) - ); - $this->get('/auth/reset?uid=' . $user->uid . '&token=' . $token); + $this->get( + URL::temporarySignedRoute('auth.reset', now()->addHour(), ['uid' => $user->uid]) + )->assertSuccessful(); } public function testHandleReset() { $user = factory(User::class)->create(); - - // Should return a warning if `uid` is empty - $this->postJson('/auth/reset', [], [ - 'X-Requested-With' => 'XMLHttpRequest' - ])->assertJson([ - 'errno' => 1, - 'msg' => trans('validation.required', ['attribute' => 'uid']) - ]); - - // Should return a warning if `uid` is not an integer - $this->postJson('/auth/reset', [ - 'uid' => 'string' - ], [ - 'X-Requested-With' => 'XMLHttpRequest' - ])->assertJson([ - 'errno' => 1, - 'msg' => trans('validation.integer', ['attribute' => 'uid']) - ]); + $url = URL::temporarySignedRoute('auth.reset', now()->addHour(), ['uid' => $user->uid]); // Should return a warning if `password` is empty $this->postJson( - '/auth/reset', [ - 'uid' => $user->uid - ], [ + $url, [], [ 'X-Requested-With' => 'XMLHttpRequest' ])->assertJson([ 'errno' => 1, @@ -537,8 +492,7 @@ class AuthControllerTest extends TestCase // Should return a warning if `password` is too short $this->postJson( - '/auth/reset', [ - 'uid' => $user->uid, + $url, [ 'password' => '123' ], [ 'X-Requested-With' => 'XMLHttpRequest' @@ -549,8 +503,7 @@ class AuthControllerTest extends TestCase // Should return a warning if `password` is too long $this->postJson( - '/auth/reset', [ - 'uid' => $user->uid, + $url, [ 'password' => str_random(33) ], [ 'X-Requested-With' => 'XMLHttpRequest' @@ -559,64 +512,10 @@ class AuthControllerTest extends TestCase 'msg' => trans('validation.max.string', ['attribute' => 'password', 'max' => 32]) ]); - // Should be forbidden if `token` is missing - $this->postJson( - '/auth/reset', [ - 'uid' => $user->uid, - 'password' => '12345678' - ], ['X-Requested-With' => 'XMLHttpRequest'])->assertJson([ - 'errno' => 1, - 'msg' => trans('validation.required', ['attribute' => 'token']) - ]); - - // Should be forbidden if expired - $token = base64_encode( - $user->getToken().substr(time() - 60 * 60 * 2, 4, 6).str_random(16) - ); - $this->postJson( - '/auth/reset', [ - 'uid' => $user->uid, - 'password' => '12345678', - 'token' => $token - ])->assertJson([ - 'errno' => 1, - 'msg' => trans('auth.reset.expired') - ]); - - // Should return a warning if the user is not existed - $token = base64_encode( - $user->getToken().substr(time(), 4, 6).str_random(16) - ); - $this->postJson( - '/auth/reset', [ - 'uid' => -1, - 'password' => '12345678', - 'token' => $token - ])->assertJson([ - 'errno' => 1, - 'msg' => trans('auth.reset.invalid') - ]); - - // Should be forbidden if `token` is invalid - $this->postJson( - '/auth/reset', [ - 'uid' => $user->uid, - 'password' => '12345678', - 'token' => 'invalid' - ])->assertJson([ - 'errno' => 1, - 'msg' => trans('auth.reset.invalid') - ]); - // Success - $token = base64_encode( - $user->getToken().substr(time(), 4, 6).str_random(16) - ); $this->postJson( - '/auth/reset', [ - 'uid' => $user->uid, + $url, [ 'password' => '12345678', - 'token' => $token ])->assertJson([ 'errno' => 0, 'msg' => trans('auth.reset.success')