Use signed URL to simplify resetting password

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
+        $url = URL::temporarySignedRoute('auth.reset', now()->addHour(), ['uid' => $user->uid]);
-        $token = base64_encode($user->getToken().substr(time(), 4, 6).str_random(16));
 
         try {
-            Mail::to($request->input('email'))->send(new ForgotPassword($user->uid, $token));
+            Mail::to($request->input('email'))->send(new ForgotPassword($url));
-
-            Log::info("[Password Reset] Mail has been sent to [{$request->input('email')}] with token [$token]");
         } 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')) {
+        return view('auth.reset')->with('user', $users->get($uid));
-            // 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'));
-        }
     }
 
-    public function handleReset(Request $request, UserRepository $users)
+    public function handleReset($uid, Request $request, UserRepository $users)
     {
-        $this->validate($request, [
+        $validated = $this->validate($request, [
-            'uid'      => 'required|integer',
             'password' => 'required|min:8|max:32',
-            'token'    => 'required',
         ]);
 
-        $decoded   = base64_decode($request->input('token'));
+        $users->get($uid)->changePasswd($validated['password']);
-        $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");
 
         return json(trans('auth.reset.success'), 0);
     }

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,
     ];
 }

app/Mail/ForgotPassword.php

@@ -19,13 +19,12 @@ class ForgotPassword extends Mailable
     /**
      * Create a new message instance.
      *
-     * @param int    $uid
+     * @param string $url
-     * @param string $token
      * @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;
     }
 
     /**

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'
-        password: 'password',
-        token: 'token'
       }
     }));
     expect($('button').html()).toBe(

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(
                         '<i class="fa fa-spinner fa-spin"></i> ' + trans('auth.resetting')

routes/web.php

@@ -23,10 +23,10 @@ Route::group(['prefix' => 'auth'], function ()
 {
     Route::group(['middleware' => 'guest'], function ()
     {
-        Route::view('/login',     'auth.login');
+        Route::view('/login',        'auth.login');
-        Route::get ('/register',  'AuthController@register');
+        Route::get ('/register',    'AuthController@register');
-        Route::get ('/forgot',    'AuthController@forgot');
+        Route::get ('/forgot',      'AuthController@forgot');
-        Route::get ('/reset',     'AuthController@reset');
+        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');
 });
 
 /**

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) {
+        Mail::assertSent(ForgotPassword::class, function ($mail) use ($user) {
-            return stristr($url, $mail->reset_url) == 0 && $mail->hasTo($user->email);
+            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(
-        $this->get('/auth/reset')
+            URL::temporarySignedRoute('auth.reset', now()->addHour(), ['uid' => $user->uid])
-            ->assertRedirect('/auth/login');
+        )->assertSuccessful();
-
-        // 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);
     }
 
     public function testHandleReset()
     {
         $user = factory(User::class)->create();
-
+        $url = URL::temporarySignedRoute('auth.reset', now()->addHour(), ['uid' => $user->uid]);
-        // 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'])
-        ]);
 
         // Should return a warning if `password` is empty
         $this->postJson(
-            '/auth/reset', [
+            $url, [], [
-            'uid' => $user->uid
-        ], [
             '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', [
+            $url, [
-            'uid' => $user->uid,
             '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', [
+            $url, [
-            'uid' => $user->uid,
             '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', [
+            $url, [
-            'uid' => $user->uid,
             'password' => '12345678',
-            'token' => $token
         ])->assertJson([
             'errno' => 0,
             'msg' => trans('auth.reset.success')