Skip to content

Commit

Permalink
fix cookie jwt
Browse files Browse the repository at this point in the history
  • Loading branch information
hafezdivandari committed Nov 23, 2024
1 parent 7b597d4 commit ca016b9
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 45 deletions.
7 changes: 3 additions & 4 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,14 @@ jobs:
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php }}
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, sqlite, pdo_sqlite, bcmath, soap, intl, gd, exif, iconv, imagick
extensions: dom, curl, libxml, mbstring, zip
ini-values: error_reporting=E_ALL
tools: composer:v2
coverage: none

- name: Install dependencies
run: |
composer require "illuminate/contracts=^${{ matrix.laravel }}" --no-update
composer update --prefer-dist --no-interaction --no-progress
composer update --prefer-dist --no-interaction --no-progress --with="illuminate/contracts=^${{ matrix.laravel }}"
- name: Execute tests
run: vendor/bin/phpunit
run: vendor/bin/phpunit --display-deprecations
7 changes: 4 additions & 3 deletions src/ApiTokenCookieFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public function make(string|int $userId, string $csrfToken): Cookie
$config['secure'],
true,
false,
$config['same_site'] ?? null
$config['same_site'] ?? null,
$config['partitioned'] ?? false
);
}

Expand All @@ -48,8 +49,8 @@ protected function createToken(string|int $userId, string $csrfToken, int $expir
{
return JWT::encode([
'sub' => $userId,
'csrf' => $csrfToken,
'expiry' => $expiration,
'jti' => $csrfToken,
'exp' => $expiration,
], Passport::tokenEncryptionKey($this->encrypter), 'HS256');
}
}
14 changes: 9 additions & 5 deletions src/Guards/TokenGuard.php
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,17 @@ protected function getTokenViaCookie(): ?array
return null;
}

// Token's expiration time is checked using the "exp" claim during decoding, but
// legacy tokens may have an "expiry" claim instead of the standard "exp". So
// we must manually check token's expiry, if the "expiry" claim is present.
if (isset($token['expiry']) && time() >= $token['expiry']) {
return null;
}

// We will compare the CSRF token in the decoded API token against the CSRF header
// sent with the request. If they don't match then this request isn't sent from
// a valid source and we won't authenticate the request for further handling.
if (! Passport::$ignoreCsrfToken &&
(! $this->validCsrf($token) || time() >= $token['expiry'])) {
if (! Passport::$ignoreCsrfToken && ! $this->validCsrf($token)) {
return null;
}

Expand Down Expand Up @@ -244,9 +250,7 @@ protected function decodeJwtTokenCookie(): array
*/
protected function validCsrf(array $token): bool
{
return isset($token['csrf']) && hash_equals(
$token['csrf'], $this->getTokenFromRequest()
);
return isset($token['jti']) && hash_equals($token['jti'], $this->getTokenFromRequest());
}

/**
Expand Down
22 changes: 8 additions & 14 deletions tests/Feature/AccessTokenControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use Laravel\Passport\Database\Factories\ClientFactory;
use Laravel\Passport\Passport;
use Laravel\Passport\Token;
use League\OAuth2\Server\Entities\AccessTokenEntityInterface;
use League\OAuth2\Server\ResponseTypes\BearerTokenResponse;
use Orchestra\Testbench\Concerns\WithLaravelMigrations;
use Workbench\Database\Factories\UserFactory;

Expand Down Expand Up @@ -253,25 +255,17 @@ public function testGettingCustomResponseType()
}
}

class IdTokenResponse extends \League\OAuth2\Server\ResponseTypes\BearerTokenResponse
class IdTokenResponse extends BearerTokenResponse
{
/**
* @var string Id token.
*/
protected $idToken;

/**
* @param string $idToken
*/
public function __construct($idToken)
{
$this->idToken = $idToken;
public function __construct(
protected string $idToken
) {
}

/**
* @inheritdoc
* {@inheritdoc}
*/
protected function getExtraParams(\League\OAuth2\Server\Entities\AccessTokenEntityInterface $accessToken): array
protected function getExtraParams(AccessTokenEntityInterface $accessToken): array
{
return [
'id_token' => $this->idToken,
Expand Down
38 changes: 19 additions & 19 deletions tests/Unit/TokenGuardTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ public function test_users_may_be_retrieved_from_cookies_with_csrf_token_header(
$encrypter->encrypt(CookieValuePrefix::create('laravel_token', $encrypter->getKey()).JWT::encode([
'sub' => 1,
'aud' => 1,
'csrf' => 'token',
'expiry' => Carbon::now()->addMinutes(10)->getTimestamp(),
'jti' => 'token',
'exp' => Carbon::now()->addMinutes(10)->getTimestamp(),
], str_repeat('a', 16), 'HS256'), false)
);

Expand Down Expand Up @@ -203,8 +203,8 @@ public function test_users_may_be_retrieved_from_cookies_with_xsrf_token_header(
$encrypter->encrypt(CookieValuePrefix::create('laravel_token', $encrypter->getKey()).JWT::encode([
'sub' => 1,
'aud' => 1,
'csrf' => 'token',
'expiry' => Carbon::now()->addMinutes(10)->getTimestamp(),
'jti' => 'token',
'exp' => Carbon::now()->addMinutes(10)->getTimestamp(),
], str_repeat('a', 16), 'HS256'), false)
);

Expand All @@ -231,8 +231,8 @@ public function test_cookie_xsrf_is_verified_against_csrf_token_header()
$encrypter->encrypt(JWT::encode([
'sub' => 1,
'aud' => 1,
'csrf' => 'token',
'expiry' => Carbon::now()->addMinutes(10)->getTimestamp(),
'jti' => 'token',
'exp' => Carbon::now()->addMinutes(10)->getTimestamp(),
], str_repeat('a', 16), 'HS256'))
);

Expand All @@ -256,8 +256,8 @@ public function test_cookie_xsrf_is_verified_against_xsrf_token_header()
$encrypter->encrypt(JWT::encode([
'sub' => 1,
'aud' => 1,
'csrf' => 'token',
'expiry' => Carbon::now()->addMinutes(10)->getTimestamp(),
'jti' => 'token',
'exp' => Carbon::now()->addMinutes(10)->getTimestamp(),
], str_repeat('a', 16), 'HS256'))
);

Expand Down Expand Up @@ -289,8 +289,8 @@ public function test_users_may_be_retrieved_from_cookies_with_xsrf_token_header_
$encrypter->encrypt(CookieValuePrefix::create('laravel_token', $encrypter->getKey()).JWT::encode([
'sub' => 1,
'aud' => 1,
'csrf' => 'token',
'expiry' => Carbon::now()->addMinutes(10)->getTimestamp(),
'jti' => 'token',
'exp' => Carbon::now()->addMinutes(10)->getTimestamp(),
], Passport::tokenEncryptionKey($encrypter), 'HS256'), false)
);

Expand Down Expand Up @@ -329,8 +329,8 @@ public function test_users_may_be_retrieved_from_cookies_without_encryption()
JWT::encode([
'sub' => 1,
'aud' => 1,
'csrf' => 'token',
'expiry' => Carbon::now()->addMinutes(10)->getTimestamp(),
'jti' => 'token',
'exp' => Carbon::now()->addMinutes(10)->getTimestamp(),
], Passport::tokenEncryptionKey($encrypter), 'HS256')
);

Expand Down Expand Up @@ -361,8 +361,8 @@ public function test_xsrf_token_cookie_without_a_token_header_is_not_accepted()
$encrypter->encrypt(JWT::encode([
'sub' => 1,
'aud' => 1,
'csrf' => 'token',
'expiry' => Carbon::now()->addMinutes(10)->getTimestamp(),
'jti' => 'token',
'exp' => Carbon::now()->addMinutes(10)->getTimestamp(),
], str_repeat('a', 16), 'HS256'))
);

Expand All @@ -386,8 +386,8 @@ public function test_expired_cookies_may_not_be_used()
$encrypter->encrypt(JWT::encode([
'sub' => 1,
'aud' => 1,
'csrf' => 'token',
'expiry' => Carbon::now()->subMinutes(10)->getTimestamp(),
'jti' => 'token',
'exp' => Carbon::now()->subMinutes(10)->getTimestamp(),
], str_repeat('a', 16), 'HS256'))
);

Expand Down Expand Up @@ -416,7 +416,7 @@ public function test_csrf_check_can_be_disabled()
$encrypter->encrypt(CookieValuePrefix::create('laravel_token', $encrypter->getKey()).JWT::encode([
'sub' => 1,
'aud' => 1,
'expiry' => Carbon::now()->addMinutes(10)->getTimestamp(),
'exp' => Carbon::now()->addMinutes(10)->getTimestamp(),
], str_repeat('a', 16), 'HS256'), false)
);

Expand Down Expand Up @@ -536,8 +536,8 @@ public function test_clients_may_be_retrieved_from_cookies()
$encrypter->encrypt(CookieValuePrefix::create('laravel_token', $encrypter->getKey()).JWT::encode([
'sub' => 1,
'aud' => 1,
'csrf' => 'token',
'expiry' => Carbon::now()->addMinutes(10)->getTimestamp(),
'jti' => 'token',
'exp' => Carbon::now()->addMinutes(10)->getTimestamp(),
], str_repeat('a', 16), 'HS256'), false)
);

Expand Down

0 comments on commit ca016b9

Please sign in to comment.