Add security check before resetting password

This commit is contained in:
Pig Fang 2017-11-05 10:25:20 +08:00
parent ed27972608
commit 8d1adc7e4d
4 changed files with 82 additions and 8 deletions

View File

@ -214,9 +214,9 @@ class AuthController extends Controller
return redirect('auth/forgot')->with('msg', trans('auth.reset.invalid')); return redirect('auth/forgot')->with('msg', trans('auth.reset.invalid'));
// unpack to get user token & timestamp // unpack to get user token & timestamp
$encrypted = base64_decode($request->input('token')); $decoded = base64_decode($request->input('token'));
$token = substr($encrypted, 0, -22); $token = substr($decoded, 0, -22);
$timestamp = substr($encrypted, strlen($token), 6); $timestamp = substr($decoded, strlen($token), 6);
if ($user->getToken() != $token) { if ($user->getToken() != $token) {
return redirect('auth/forgot')->with('msg', trans('auth.reset.invalid')); return redirect('auth/forgot')->with('msg', trans('auth.reset.invalid'));
@ -238,8 +238,26 @@ class AuthController extends Controller
$this->validate($request, [ $this->validate($request, [
'uid' => 'required|integer', 'uid' => 'required|integer',
'password' => 'required|min:8|max:16', 'password' => 'required|min:8|max:16',
'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')); $users->get($request->input('uid'))->changePasswd($request->input('password'));
Log::info("[Password Reset] Password of user [{$request->input('uid')}] has been changed"); Log::info("[Password Reset] Password of user [{$request->input('uid')}] has been changed");

View File

@ -302,12 +302,14 @@ describe('tests for "reset" module', () => {
const url = jest.fn(path => path); const url = jest.fn(path => path);
const swal = jest.fn().mockReturnValue(Promise.resolve()); const swal = jest.fn().mockReturnValue(Promise.resolve());
const showMsg = jest.fn(); const showMsg = jest.fn();
const getQueryString = jest.fn().mockReturnValue('token');
window.fetch = fetch; window.fetch = fetch;
window.trans = trans; window.trans = trans;
window.url = url; window.url = url;
window.swal = swal; window.swal = swal;
window.showMsg = showMsg; window.showMsg = showMsg;
window.refreshCaptcha = jest.fn(); window.refreshCaptcha = jest.fn();
window.getQueryString = getQueryString;
document.body.innerHTML = ` document.body.innerHTML = `
<input id="uid" value="1" /> <input id="uid" value="1" />
@ -348,13 +350,15 @@ describe('tests for "reset" module', () => {
$('#confirm-pwd').val('password'); $('#confirm-pwd').val('password');
await $('button').click(); await $('button').click();
expect(getQueryString).toBeCalledWith('token');
expect(fetch).toBeCalledWith(expect.objectContaining({ expect(fetch).toBeCalledWith(expect.objectContaining({
type: 'POST', type: 'POST',
url: 'auth/reset', url: 'auth/reset',
dataType: 'json', dataType: 'json',
data: { data: {
uid: '1', uid: '1',
password: 'password' password: 'password',
token: 'token'
} }
})); }));
expect($('button').html()).toBe( expect($('button').html()).toBe(

View File

@ -5,7 +5,8 @@ $('#reset-button').click(e => {
let data = { let data = {
uid: $('#uid').val(), uid: $('#uid').val(),
password: $('#password').val() password: $('#password').val(),
token: getQueryString('token')
}; };
(function validate({ password }, callback) { (function validate({ password }, callback) {

View File

@ -589,11 +589,64 @@ class AuthControllerTest extends TestCase
'msg' => trans('validation.max.string', ['attribute' => 'password', 'max' => 16]) 'msg' => trans('validation.max.string', ['attribute' => 'password', 'max' => 16])
]); ]);
// Success // Should be forbidden if `token` is missing
$this->post( $this->post(
'/auth/reset', [ '/auth/reset', [
'uid' => $user->uid, 'uid' => $user->uid,
'password' => '12345678' 'password' => '12345678'
], ['X-Requested-With' => 'XMLHttpRequest'])->seeJson([
'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->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,
'password' => '12345678',
'token' => $token
])->seeJson([
'errno' => 1,
'msg' => trans('auth.reset.invalid')
]);
// Should be forbidden if `token` is invalid
$this->post(
'/auth/reset', [
'uid' => $user->uid,
'password' => '12345678',
'token' => 'invalid'
])->seeJson([
'errno' => 1,
'msg' => trans('auth.reset.invalid')
]);
// Success
$token = base64_encode(
$user->getToken().substr(time(), 4, 6).str_random(16)
);
$this->post(
'/auth/reset', [
'uid' => $user->uid,
'password' => '12345678',
'token' => $token
])->seeJson([ ])->seeJson([
'errno' => 0, 'errno' => 0,
'msg' => trans('auth.reset.success') 'msg' => trans('auth.reset.success')
@ -603,8 +656,6 @@ class AuthControllerTest extends TestCase
// after resetting password. // after resetting password.
$user = User::find($user->uid); $user = User::find($user->uid);
$this->assertTrue($user->verifyPassword('12345678')); $this->assertTrue($user->verifyPassword('12345678'));
// TODO: Security check before resetting password!!!
} }
public function testCaptcha() public function testCaptcha()