From 0e70a88e1365d51531c49704dcd5b002e8e79b1e Mon Sep 17 00:00:00 2001 From: printempw Date: Fri, 27 Jul 2018 19:27:45 +0800 Subject: [PATCH] Use random generated token for password resetting --- app/Http/Controllers/AuthController.php | 61 ++++++++------------ resources/lang/en/auth.yml | 6 +- resources/lang/zh_CN/auth.yml | 2 +- tests/AuthControllerTest.php | 77 +++++++++---------------- 4 files changed, 55 insertions(+), 91 deletions(-) diff --git a/app/Http/Controllers/AuthController.php b/app/Http/Controllers/AuthController.php index 7a20ef53..a13fe772 100644 --- a/app/Http/Controllers/AuthController.php +++ b/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')) { - // Get user instance from repository - $user = $users->get($request->input('uid')); + // Retrieve token from cache + $uid = $request->get('uid'); + $token = Cache::get("pwd_reset_token_$uid"); - if (! $user) - return redirect('auth/forgot')->with('msg', trans('auth.reset.invalid')); + // Get user instance from repository + $user = $users->get($uid); - // 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')); + if (! $user) { + return redirect('auth/forgot')->with('msg', trans('auth.reset.invalid')); } + + // 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')); - $token = substr($decoded, 0, -22); - $timestamp = intval(substr($decoded, strlen($token), 6)); + // Retrieve token from cache + $uid = $request->get('uid'); + $token = Cache::get("pwd_reset_token_$uid"); - $user = $users->get($request->input('uid')); - if (! $user) - return json(trans('auth.reset.invalid'), 1); + // Get user instance from repository + $user = $users->get($uid); - if ($user->getToken() != $token) { + if (! $user) { return json(trans('auth.reset.invalid'), 1); } - // More than 1 hour - if ((intval(substr(time(), 4, 6)) - $timestamp) > 3600) { + // No token exist or token mismatch (maybe expired) + if (is_null($token) || $token != $request->get('token')) { 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"); + $user->changePasswd($request->get('password')); return json(trans('auth.reset.success'), 0); } diff --git a/resources/lang/en/auth.yml b/resources/lang/en/auth.yml index 2c870992..a3de962b 100644 --- a/resources/lang/en/auth.yml +++ b/resources/lang/en/auth.yml @@ -43,9 +43,9 @@ reset: title: Reset Password button: Reset invalid: Invalid link. - expired: This link is expired. - message: :username, reset your email address here. - success: Password resetted successfully. + expired: This link is expired, please resend a verification email. + message: :username, reset your password here. + success: Your password was reset successfully. bind: title: Bind Email diff --git a/resources/lang/zh_CN/auth.yml b/resources/lang/zh_CN/auth.yml index a876c26f..2c97c1af 100644 --- a/resources/lang/zh_CN/auth.yml +++ b/resources/lang/zh_CN/auth.yml @@ -43,7 +43,7 @@ reset: title: 重置密码 button: 重置 invalid: 无效的链接 - expired: 链接已过期 + expired: 链接已失效，请重新发送验证邮件 message: :username，在这里重置你的密码 success: 密码重置成功 diff --git a/tests/AuthControllerTest.php b/tests/AuthControllerTest.php index ea3321e3..5f59c0c3 100644 --- a/tests/AuthControllerTest.php +++ b/tests/AuthControllerTest.php @@ -449,11 +449,8 @@ 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"; + $token = generate_random_token(); + $url = Option::get('site_url')."/auth/reset?uid={$user->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') - ->see(trans('auth.check.anonymous')); + ->seePageIs('/auth/forgot') + ->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 - $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) + // Should be redirected if `token` is invalid or expired + $this->visit("/auth/reset?uid={$user->uid}&token=nothing") ->seePageIs('/auth/forgot') ->see(trans('auth.reset.expired')); // Success - $token = base64_encode( - $user->getToken().substr(time(), 4, 6).str_random(16) - ); - $uri = $this->visit('/auth/reset?uid=' . $user->uid . '&token=' . $token) + $token = generate_random_token(); + $uri = $this->withCache(["pwd_reset_token_{$user->uid}" => $token]) + ->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 = 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') - ]); + $token = generate_random_token(); // 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 - $this->post( + // Should be forbidden if `token` is invalid or expired + $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( - $user->getToken().substr(time(), 4, 6).str_random(16) - ); - $this->post( + $this->withCache( + ["pwd_reset_token_{$user->uid}" => $token] + )->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 - // after resetting password. - $user = User::find($user->uid); - $this->assertTrue($user->verifyPassword('12345678')); + + // Re-fetch user data and verify it + $this->assertTrue($user->fresh()->verifyPassword('12345678')); } public function testVerify()