From de6ae4b12ec3b9497c3d4193a5d8994482d00769 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 27 Jul 2020 13:16:19 -0500 Subject: [PATCH 1/2] fix cookie handling for security release --- composer.json | 18 +++++++++--------- src/Guards/TokenGuard.php | 5 +++-- tests/Unit/TokenGuardTest.php | 11 ++++++----- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/composer.json b/composer.json index 557498ae9..b9cb299e1 100644 --- a/composer.json +++ b/composer.json @@ -18,15 +18,15 @@ "ext-json": "*", "firebase/php-jwt": "^5.0", "guzzlehttp/guzzle": "^6.0|^7.0", - "illuminate/auth": "^6.0|^7.0", - "illuminate/console": "^6.0|^7.0", - "illuminate/container": "^6.0|^7.0", - "illuminate/contracts": "^6.0|^7.0", - "illuminate/cookie": "^6.0|^7.0", - "illuminate/database": "^6.0|^7.0", - "illuminate/encryption": "^6.0|^7.0", - "illuminate/http": "^6.0|^7.0", - "illuminate/support": "^6.0|^7.0", + "illuminate/auth": "^6.18.30|^7.22.3", + "illuminate/console": "^6.18.30|^7.22.3", + "illuminate/container": "^6.18.30|^7.22.3", + "illuminate/contracts": "^6.18.30|^7.22.3", + "illuminate/cookie": "^6.18.30|^7.22.3", + "illuminate/database": "^6.18.30|^7.22.3", + "illuminate/encryption": "^6.18.30|^7.22.3", + "illuminate/http": "^6.18.30|^7.22.3", + "illuminate/support": "^6.18.30|^7.22.3", "laminas/laminas-diactoros": "^2.2", "league/oauth2-server": "^8.1", "nyholm/psr7": "^1.0", diff --git a/src/Guards/TokenGuard.php b/src/Guards/TokenGuard.php index e4e01d1c2..2aab387cc 100644 --- a/src/Guards/TokenGuard.php +++ b/src/Guards/TokenGuard.php @@ -7,6 +7,7 @@ use Illuminate\Container\Container; use Illuminate\Contracts\Debug\ExceptionHandler; use Illuminate\Contracts\Encryption\Encrypter; +use Illuminate\Cookie\CookieValuePrefix; use Illuminate\Cookie\Middleware\EncryptCookies; use Illuminate\Http\Request; use Laminas\Diactoros\ResponseFactory; @@ -270,7 +271,7 @@ protected function getTokenViaCookie($request) protected function decodeJwtTokenCookie($request) { return (array) JWT::decode( - $this->encrypter->decrypt($request->cookie(Passport::cookie()), Passport::$unserializesCookies), + CookieValuePrefix::remove($this->encrypter->decrypt($request->cookie(Passport::cookie()), Passport::$unserializesCookies)), $this->encrypter->getKey(), ['HS256'] ); @@ -301,7 +302,7 @@ protected function getTokenFromRequest($request) $token = $request->header('X-CSRF-TOKEN'); if (! $token && $header = $request->header('X-XSRF-TOKEN')) { - $token = $this->encrypter->decrypt($header, static::serialized()); + $token = CookieValuePrefix::remove($this->encrypter->decrypt($header, static::serialized())); } return $token; diff --git a/tests/Unit/TokenGuardTest.php b/tests/Unit/TokenGuardTest.php index 929d19ede..a7f4b8556 100644 --- a/tests/Unit/TokenGuardTest.php +++ b/tests/Unit/TokenGuardTest.php @@ -6,6 +6,7 @@ use Firebase\JWT\JWT; use Illuminate\Container\Container; use Illuminate\Contracts\Debug\ExceptionHandler; +use Illuminate\Cookie\CookieValuePrefix; use Illuminate\Encryption\Encrypter; use Illuminate\Http\Request; use Laravel\Passport\ClientRepository; @@ -127,7 +128,7 @@ public function test_users_may_be_retrieved_from_cookies_with_csrf_token_header( $request = Request::create('/'); $request->headers->set('X-CSRF-TOKEN', 'token'); $request->cookies->set('laravel_token', - $encrypter->encrypt(JWT::encode([ + $encrypter->encrypt(CookieValuePrefix::create('laravel_token', $encrypter->getKey()).JWT::encode([ 'sub' => 1, 'aud' => 1, 'csrf' => 'token', @@ -158,9 +159,9 @@ public function test_users_may_be_retrieved_from_cookies_with_xsrf_token_header( $guard = new TokenGuard($resourceServer, $userProvider, $tokens, $clients, $encrypter); $request = Request::create('/'); - $request->headers->set('X-XSRF-TOKEN', $encrypter->encrypt('token', false)); + $request->headers->set('X-XSRF-TOKEN', $encrypter->encrypt(CookieValuePrefix::create('X-XSRF-TOKEN', $encrypter->getKey()).'token', false)); $request->cookies->set('laravel_token', - $encrypter->encrypt(JWT::encode([ + $encrypter->encrypt(CookieValuePrefix::create('laravel_token', $encrypter->getKey()).JWT::encode([ 'sub' => 1, 'aud' => 1, 'csrf' => 'token', @@ -298,7 +299,7 @@ public function test_csrf_check_can_be_disabled() $request = Request::create('/'); $request->cookies->set('laravel_token', - $encrypter->encrypt(JWT::encode([ + $encrypter->encrypt(CookieValuePrefix::create('laravel_token', $encrypter->getKey()).JWT::encode([ 'sub' => 1, 'aud' => 1, 'expiry' => Carbon::now()->addMinutes(10)->getTimestamp(), @@ -396,7 +397,7 @@ public function test_clients_may_be_retrieved_from_cookies() $request = Request::create('/'); $request->headers->set('X-CSRF-TOKEN', 'token'); $request->cookies->set('laravel_token', - $encrypter->encrypt(JWT::encode([ + $encrypter->encrypt(CookieValuePrefix::create('laravel_token', $encrypter->getKey()).JWT::encode([ 'sub' => 1, 'aud' => 1, 'csrf' => 'token', From 4977c8268bd3f84b1d2a4a00cd2b8a7ed3bdf666 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Mon, 27 Jul 2020 13:16:41 -0500 Subject: [PATCH 2/2] Apply fixes from StyleCI (#1321) --- tests/Unit/AuthorizationControllerTest.php | 2 +- tests/Unit/AuthorizedAccessTokenControllerTest.php | 2 +- tests/Unit/PersonalAccessTokenControllerTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Unit/AuthorizationControllerTest.php b/tests/Unit/AuthorizationControllerTest.php index 2ceefadc3..333bb1101 100644 --- a/tests/Unit/AuthorizationControllerTest.php +++ b/tests/Unit/AuthorizationControllerTest.php @@ -55,7 +55,7 @@ public function test_authorization_view_is_presented() $client->shouldReceive('skipsAuthorization')->andReturn(false); - $response->shouldReceive('view')->once()->andReturnUsing(function ($view, $data) use ($authRequest, $client, $user) { + $response->shouldReceive('view')->once()->andReturnUsing(function ($view, $data) use ($client, $user) { $this->assertEquals('passport::authorize', $view); $this->assertEquals($client, $data['client']); $this->assertEquals($user, $data['user']); diff --git a/tests/Unit/AuthorizedAccessTokenControllerTest.php b/tests/Unit/AuthorizedAccessTokenControllerTest.php index 63b4f9ca2..89154c7fa 100644 --- a/tests/Unit/AuthorizedAccessTokenControllerTest.php +++ b/tests/Unit/AuthorizedAccessTokenControllerTest.php @@ -63,7 +63,7 @@ public function test_tokens_can_be_retrieved_for_users() $this->tokenRepository->shouldReceive('forUser')->andReturn($userTokens); - $request->setUserResolver(function () use ($token1, $token2) { + $request->setUserResolver(function () { $user = m::mock(); $user->shouldReceive('getAuthIdentifier')->andReturn(1); diff --git a/tests/Unit/PersonalAccessTokenControllerTest.php b/tests/Unit/PersonalAccessTokenControllerTest.php index 007d87bce..0fdbbe577 100644 --- a/tests/Unit/PersonalAccessTokenControllerTest.php +++ b/tests/Unit/PersonalAccessTokenControllerTest.php @@ -36,7 +36,7 @@ public function test_tokens_can_be_retrieved_for_users() $tokenRepository = m::mock(TokenRepository::class); $tokenRepository->shouldReceive('forUser')->andReturn($userTokens); - $request->setUserResolver(function () use ($token1, $token2) { + $request->setUserResolver(function () { $user = m::mock(); $user->shouldReceive('getAuthIdentifier')->andReturn(1);