Use random generated token for password resetting

app/Http/Controllers/AuthController.php

@@ -198,8 +198,7 @@ class AuthController extends Controller
 
         $uid = $user->uid;
         // Generate token for password resetting
-        $token = base64_encode($user->getToken().substr(time(), 4, 6).str_random(16));
+        $token = generate_random_token();
-
         $url = Option::get('site_url')."/auth/reset?uid=$uid&token=$token";
 
         try {
@@ -216,6 +215,7 @@ class AuthController extends Controller
             return json(trans('auth.forgot.failed', ['msg' => $e->getMessage()]), 2);
         }
 
+        Cache::put("pwd_reset_token_$uid", $token, 60);
         Cache::put($lastMailCacheKey, time(), 60);
 
         return json(trans('auth.forgot.success'), 0);
@@ -223,31 +223,23 @@ class AuthController extends Controller
 
     public function reset(UserRepository $users, Request $request)
     {
-        if ($request->has('uid') && $request->has('token')) {
+        // Retrieve token from cache
-            // Get user instance from repository
+        $uid = $request->get('uid');
-            $user = $users->get($request->input('uid'));
+        $token = Cache::get("pwd_reset_token_$uid");
 
-            if (! $user)
+        // Get user instance from repository
-                return redirect('auth/forgot')->with('msg', trans('auth.reset.invalid'));
+        $user = $users->get($uid);
 
-            // Unpack to get user token & timestamp
+        if (! $user) {
-            $decoded   = base64_decode($request->input('token'));
+            return redirect('auth/forgot')->with('msg', trans('auth.reset.invalid'));
-            $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'));
         }
+
+        // No token exist or token mismatch (maybe expired)
+        if (is_null($token) || $token != $request->get('token')) {
+            return redirect('auth/forgot')->with('msg', trans('auth.reset.expired'));
+        }
+
+        return view('auth.reset')->with('user', $user);
     }
 
     public function handleReset(Request $request, UserRepository $users)
@@ -258,26 +250,23 @@ class AuthController extends Controller
             'token'    => 'required',
         ]);
 
-        $decoded   = base64_decode($request->input('token'));
+        // Retrieve token from cache
-        $token     = substr($decoded, 0, -22);
+        $uid = $request->get('uid');
-        $timestamp = intval(substr($decoded, strlen($token), 6));
+        $token = Cache::get("pwd_reset_token_$uid");
 
-        $user = $users->get($request->input('uid'));
+        // Get user instance from repository
-        if (! $user)
+        $user = $users->get($uid);
-            return json(trans('auth.reset.invalid'), 1);
 
-        if ($user->getToken() != $token) {
+        if (! $user) {
             return json(trans('auth.reset.invalid'), 1);
         }
 
-        // More than 1 hour
+        // No token exist or token mismatch (maybe expired)
-        if ((intval(substr(time(), 4, 6)) - $timestamp) > 3600) {
+        if (is_null($token) || $token != $request->get('token')) {
             return json(trans('auth.reset.expired'), 1);
         }
 
-        $users->get($request->input('uid'))->changePasswd($request->input('password'));
+        $user->changePasswd($request->get('password'));
-
-        Log::info("[Password Reset] Password of user [{$request->input('uid')}] has been changed");
 
         return json(trans('auth.reset.success'), 0);
     }

resources/lang/en/auth.yml

@@ -43,9 +43,9 @@ reset:
   title: Reset Password
   button: Reset
   invalid: Invalid link.
-  expired: This link is expired.
+  expired: This link is expired, please resend a verification email.
-  message: :username, reset your email address here.
+  message: :username, reset your password here.
-  success: Password resetted successfully.
+  success: Your password was reset successfully.
 
 bind:
   title: Bind Email

resources/lang/zh_CN/auth.yml

@@ -43,7 +43,7 @@ reset:
   title: 重置密码
   button: 重置
   invalid: 无效的链接
-  expired: 链接已过期
+  expired: 链接已失效，请重新发送验证邮件
   message: :username，在这里重置你的密码
   success: 密码重置成功
 

tests/AuthControllerTest.php

@@ -449,11 +449,8 @@ class AuthControllerTest extends TestCase
             'msg' => trans('auth.forgot.unregistered')
         ]);
 
-        $uid = $user->uid;
+        $token = generate_random_token();
-        $token = base64_encode(
+        $url = Option::get('site_url')."/auth/reset?uid={$user->uid}&token=$token";
-            $user->getToken().substr(time(), 4, 6).str_random(16)
-        );
-        $url = Option::get('site_url')."/auth/reset?uid=$uid&token=$token";
         // An email should be send
         // Laravel supports `Mail::fake()` since v5.4, but now we cannot
         // Thanks: https://stackoverflow.com/questions/31120567/unittesting-laravel-5-mail-using-mock
@@ -490,7 +487,10 @@ class AuthControllerTest extends TestCase
         ])->seeJson([
             'errno' => 0,
             'msg' => trans('auth.forgot.success')
-        ])->assertCacheHas($lastMailCacheKey);
+        ])->assertCacheHas([
+            $lastMailCacheKey,
+            "pwd_reset_token_{$user->uid}"
+        ]);
         $this->flushCache();
 
         // Should handle exception when sending email
@@ -514,32 +514,23 @@ class AuthControllerTest extends TestCase
 
         // Should be redirected if `uid` or `token` is empty
         $this->visit('/auth/reset')
-            ->seePageIs('/auth/login')
+            ->seePageIs('/auth/forgot')
-            ->see(trans('auth.check.anonymous'));
+            ->see(trans('auth.reset.invalid'));
 
         // Should be redirected if `uid` is invalid
         $this->visit('/auth/reset?uid=-1&token=nothing')
             ->seePageIs('/auth/forgot')
             ->see(trans('auth.reset.invalid'));
 
-        // Should be redirected if `token` is invalid
+        // Should be redirected if `token` is invalid or expired
-        $this->visit('/auth/reset?uid=' . $user->uid . '&token=nothing')
+        $this->visit("/auth/reset?uid={$user->uid}&token=nothing")
-            ->seePageIs('/auth/forgot')
-            ->see(trans('auth.reset.invalid'));
-
-        // Should be redirected if expired
-        $token = base64_encode(
-            $user->getToken().substr(time() - 60 * 60 * 2, 4, 6).str_random(16)
-        );
-        $this->visit('/auth/reset?uid=' . $user->uid . '&token=' . $token)
             ->seePageIs('/auth/forgot')
             ->see(trans('auth.reset.expired'));
 
         // Success
-        $token = base64_encode(
+        $token = generate_random_token();
-            $user->getToken().substr(time(), 4, 6).str_random(16)
+        $uri = $this->withCache(["pwd_reset_token_{$user->uid}" => $token])
-        );
+                    ->visit("/auth/reset?uid={$user->uid}&token=$token")
-        $uri = $this->visit('/auth/reset?uid=' . $user->uid . '&token=' . $token)
                     ->currentUri;
         $this->assertContains('/auth/reset', $uri);
     }
@@ -611,24 +602,9 @@ class AuthControllerTest extends TestCase
             'msg' => trans('validation.required', ['attribute' => 'token'])
         ]);
 
-        // Should be forbidden if expired
+        $token = generate_random_token();
-        $token = base64_encode(
-            $user->getToken().substr(time() - 60 * 60 * 2, 4, 6).str_random(16)
-        );
-        $this->post(
-            '/auth/reset', [
-            'uid' => $user->uid,
-            'password' => '12345678',
-            'token' => $token
-        ])->seeJson([
-            '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->post(
             '/auth/reset', [
             'uid' => -1,
@@ -639,22 +615,23 @@ class AuthControllerTest extends TestCase
             'msg' => trans('auth.reset.invalid')
         ]);
 
-        // Should be forbidden if `token` is invalid
+        // Should be forbidden if `token` is invalid or expired
-        $this->post(
+        $this->withCache(
+            ["pwd_reset_token_{$user->uid}" => $token]
+        )->post(
             '/auth/reset', [
             'uid' => $user->uid,
             'password' => '12345678',
-            'token' => 'invalid'
+            'token' => 'something-else'
         ])->seeJson([
             'errno' => 1,
-            'msg' => trans('auth.reset.invalid')
+            'msg' => trans('auth.reset.expired')
         ]);
 
         // Success
-        $token = base64_encode(
+        $this->withCache(
-            $user->getToken().substr(time(), 4, 6).str_random(16)
+            ["pwd_reset_token_{$user->uid}" => $token]
-        );
+        )->post(
-        $this->post(
             '/auth/reset', [
             'uid' => $user->uid,
             'password' => '12345678',
@@ -663,11 +640,9 @@ class AuthControllerTest extends TestCase
             'errno' => 0,
             'msg' => trans('auth.reset.success')
         ]);
-        // We must re-query the user model,
+
-        // because the old instance hasn't been changed
+        // Re-fetch user data and verify it
-        // after resetting password.
+        $this->assertTrue($user->fresh()->verifyPassword('12345678'));
-        $user = User::find($user->uid);
-        $this->assertTrue($user->verifyPassword('12345678'));
     }
 
     public function testVerify()