From 5d9bb2828169470d6ae814954e83b14d0d7db33b Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Sat, 28 Mar 2020 22:36:27 +0800 Subject: [PATCH] attempt to fix "invalid signature" issue --- app/Http/Controllers/AuthController.php | 13 ++++++++++-- app/Http/Controllers/UserController.php | 2 +- app/Http/Kernel.php | 1 - routes/web.php | 8 +++---- .../ControllersTest/AuthControllerTest.php | 21 +++++++++++++++---- 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/AuthController.php b/app/Http/Controllers/AuthController.php index f8d5de95..cf7120e0 100644 --- a/app/Http/Controllers/AuthController.php +++ b/app/Http/Controllers/AuthController.php @@ -258,7 +258,12 @@ class AuthController extends Controller $dispatcher->dispatch('auth.forgot.ready', [$user]); - $url = URL::temporarySignedRoute('auth.reset', now()->addHour(), ['uid' => $user->uid]); + $url = URL::temporarySignedRoute( + 'auth.reset', + now()->addHour(), + ['uid' => $user->uid], + false + ); try { Mail::to($email)->send(new ForgotPassword($url)); } catch (\Exception $e) { @@ -281,6 +286,8 @@ class AuthController extends Controller public function handleReset(Dispatcher $dispatcher, Request $request, $uid) { + abort_unless($request->hasValidSignature(false), 403, trans('auth.reset.invalid')); + ['password' => $password] = $this->validate($request, [ 'password' => 'required|min:8|max:32', ]); @@ -314,12 +321,14 @@ class AuthController extends Controller return redirect('/user'); } - public function verify($uid) + public function verify(Request $request, $uid) { if (!option('require_verification')) { throw new PrettyPageException(trans('user.verification.disabled'), 1); } + abort_unless($request->hasValidSignature(false), 403, trans('auth.verify.invalid')); + $user = User::find($uid); if (!$user || $user->verified) { diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index fdd03f29..f7bda66d 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -185,7 +185,7 @@ class UserController extends Controller return json(trans('user.verification.verified'), 1); } - $url = URL::signedRoute('auth.verify', ['uid' => $user->uid]); + $url = URL::signedRoute('auth.verify', ['uid' => $user->uid], null, false); try { Mail::to($user->email)->send(new EmailVerification($url)); diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 77951be2..88a210c5 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -62,7 +62,6 @@ class Kernel extends HttpKernel 'guest' => \App\Http\Middleware\RedirectIfAuthenticated::class, 'role' => \App\Http\Middleware\CheckRole::class, 'setup' => \App\Http\Middleware\CheckInstallation::class, - 'signed' => \Illuminate\Routing\Middleware\ValidateSignature::class, 'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class, 'verified' => \App\Http\Middleware\CheckUserVerified::class, ]; diff --git a/routes/web.php b/routes/web.php index f99cc7c8..b3b716c6 100644 --- a/routes/web.php +++ b/routes/web.php @@ -26,10 +26,8 @@ Route::prefix('auth')->name('auth.')->group(function () { Route::get('forgot', 'AuthController@forgot')->name('forgot'); Route::post('forgot', 'AuthController@handleForgot')->name('forgot'); - Route::middleware('signed')->name('reset')->group(function () { - Route::get('reset/{uid}', 'AuthController@reset'); - Route::post('reset/{uid}', 'AuthController@handleReset'); - }); + Route::get('reset/{uid}', 'AuthController@reset')->name('reset'); + Route::post('reset/{uid}', 'AuthController@handleReset')->name('reset'); Route::get('login/{driver}', 'AuthController@oauthLogin'); Route::get('login/{driver}/callback', 'AuthController@oauthCallback'); @@ -45,7 +43,7 @@ Route::prefix('auth')->name('auth.')->group(function () { Route::post('bind', 'AuthController@fillEmail'); }); - Route::get('verify/{uid}', 'AuthController@verify')->name('verify')->middleware('signed'); + Route::get('verify/{uid}', 'AuthController@verify')->name('verify'); }); Route::prefix('user') diff --git a/tests/HttpTest/ControllersTest/AuthControllerTest.php b/tests/HttpTest/ControllersTest/AuthControllerTest.php index abf48874..48431dc5 100644 --- a/tests/HttpTest/ControllersTest/AuthControllerTest.php +++ b/tests/HttpTest/ControllersTest/AuthControllerTest.php @@ -613,7 +613,16 @@ class AuthControllerTest extends TestCase Event::fake(); $user = factory(User::class)->create(); - $url = URL::temporarySignedRoute('auth.reset', now()->addHour(), ['uid' => $user->uid]); + $url = URL::temporarySignedRoute( + 'auth.reset', + now()->addHour(), + ['uid' => $user->uid], + false + ); + + // invalid signature + $this->postJson(route('auth.reset', ['uid' => $user->uid])) + ->assertForbidden(); // Should return a warning if `password` is empty $this->postJson($url)->assertJsonValidationErrors('password'); @@ -676,21 +685,25 @@ class AuthControllerTest extends TestCase public function testVerify() { - $url = URL::signedRoute('auth.verify', ['uid' => 1]); + $url = URL::signedRoute('auth.verify', ['uid' => 1], null, false); // Should be forbidden if account verification is disabled option(['require_verification' => false]); $this->get($url)->assertSee(trans('user.verification.disabled')); option(['require_verification' => true]); + // Invalid link + $this->get(route('auth.verify', ['uid' => 1]))->assertForbidden(); + + // Non-existed user $this->get($url)->assertSee(trans('auth.verify.invalid')); $user = factory(User::class)->create(); - $url = URL::signedRoute('auth.verify', ['uid' => $user->uid]); + $url = URL::signedRoute('auth.verify', ['uid' => $user->uid], null, false); $this->get($url)->assertSee(trans('auth.verify.invalid')); $user = factory(User::class)->create(['verified' => false]); - $url = URL::signedRoute('auth.verify', ['uid' => $user->uid]); + $url = URL::signedRoute('auth.verify', ['uid' => $user->uid], null, false); $this->get($url)->assertViewIs('auth.verify'); $this->assertEquals(1, User::find($user->uid)->verified); }