From f44637ee07ad8dc85c19da6a5d6bbb72adccfdd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krasucki?= <> Date: Thu, 12 Jan 2023 20:45:45 +0100 Subject: [PATCH 1/5] fix: missing bearer token format validation --- src/Guard.php | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/Guard.php b/src/Guard.php index 73ac4b39..c4806f11 100644 --- a/src/Guard.php +++ b/src/Guard.php @@ -117,9 +117,31 @@ protected function getTokenFromRequest(Request $request) return (string) (Sanctum::$accessTokenRetrievalCallback)($request); } - return $request->bearerToken(); + $token = $request->bearerToken(); + + return $this->hasValidBearerTokenFormat($token) ? $token : null; } + /** + * Determine if the bearer token has valid format + * + * @param string|null $token + * @return bool + */ + protected function hasValidBearerTokenFormat(string $token = null): bool + { + if (!is_null($token) && strpos($token, '|') !== false) { + $model = new Sanctum::$personalAccessTokenModel; + if ($model->getKeyType() === 'int') { + [$id, $token] = explode('|', $token, 2); + + return ctype_digit($id) && !empty($token); + } + } + + return !empty($token); + } + /** * Determine if the provided access token is valid. * From 75ec5e2352258f940200f32017328202952aad01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krasucki?= <> Date: Thu, 12 Jan 2023 20:46:29 +0100 Subject: [PATCH 2/5] tests: bearer token format validation --- tests/GuardTest.php | 62 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/tests/GuardTest.php b/tests/GuardTest.php index d0831b82..919b6f79 100644 --- a/tests/GuardTest.php +++ b/tests/GuardTest.php @@ -284,6 +284,48 @@ public function test_authentication_with_token_fails_if_user_provider_is_invalid Event::assertNotDispatched(TokenAuthenticated::class); } + /** + * @dataProvider invalidTokenDataProvider + */ + public function test_authentication_with_token_fails_if_token_has_invalid_format($invalidToken) + { + $this->loadLaravelMigrations(['--database' => 'testbench']); + $this->artisan('migrate', ['--database' => 'testbench'])->run(); + + $factory = Mockery::mock(AuthFactory::class); + + $guard = new Guard($factory, null, 'users'); + + $webGuard = Mockery::mock(stdClass::class); + + $factory->shouldReceive('guard') + ->with('web') + ->andReturn($webGuard); + + $webGuard->shouldReceive('user')->once()->andReturn(null); + + $request = Request::create('/', 'GET'); + + $user = User::forceCreate([ + 'name' => 'Taylor Otwell', + 'email' => 'taylor@laravel.com', + 'password' => '$2y$10$92IXUNpkjO0rOQ5byMi.Ye4oKoEa3Ro9llC/.og/at2.uheWG/igi', + 'remember_token' => Str::random(10), + ]); + + PersonalAccessToken::forceCreate([ + 'tokenable_id' => $user->id, + 'tokenable_type' => get_class($user), + 'name' => 'Test', + 'token' => hash('sha256', 'test'), + 'expires_at' => now()->subMinutes(60), + ]); + + $request->headers->set('Authorization', $invalidToken); + $returnedUser = $guard->__invoke($request); + $this->assertNull($returnedUser); + } + public function test_authentication_is_successful_with_token_if_user_provider_is_valid() { $this->loadLaravelMigrations(['--database' => 'testbench']); @@ -499,8 +541,26 @@ protected function getPackageProviders($app) { return [SanctumServiceProvider::class]; } -} + public function invalidTokenDataProvider(): array + { + return [ + [''], + ['|'], + ['test'], + ['|test'], + ['1ABC|test'], + ['1ABC|'], + ['1,2|test'], + ['Bearer'], + ['Bearer |test'], + ['Bearer 1,2|test'], + ['Bearer 1ABC|test'], + ['Bearer 1ABC|'], + ]; + } +} + class User extends Model implements HasApiTokensContract { use HasApiTokens; From 0f0c822e9b32ade07067a67cb15f9d67e5dd08f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krasucki?= Date: Thu, 12 Jan 2023 22:19:47 +0100 Subject: [PATCH 3/5] ci: fix styles --- src/Guard.php | 14 +++++++------- tests/GuardTest.php | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Guard.php b/src/Guard.php index c4806f11..175909da 100644 --- a/src/Guard.php +++ b/src/Guard.php @@ -123,25 +123,25 @@ protected function getTokenFromRequest(Request $request) } /** - * Determine if the bearer token has valid format + * Determine if the bearer token has valid format. * - * @param string|null $token - * @return bool + * @param string|null $token + * @return bool */ protected function hasValidBearerTokenFormat(string $token = null): bool { - if (!is_null($token) && strpos($token, '|') !== false) { + if (! is_null($token) && strpos($token, '|') !== false) { $model = new Sanctum::$personalAccessTokenModel; if ($model->getKeyType() === 'int') { [$id, $token] = explode('|', $token, 2); - return ctype_digit($id) && !empty($token); + return ctype_digit($id) && ! empty($token); } } - return !empty($token); + return ! empty($token); } - + /** * Determine if the provided access token is valid. * diff --git a/tests/GuardTest.php b/tests/GuardTest.php index 919b6f79..d94e2f54 100644 --- a/tests/GuardTest.php +++ b/tests/GuardTest.php @@ -560,7 +560,7 @@ public function invalidTokenDataProvider(): array ]; } } - + class User extends Model implements HasApiTokensContract { use HasApiTokens; From 2ed07397f71a3f4b642a96190b503e1f42f47230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krasucki?= Date: Thu, 12 Jan 2023 22:31:24 +0100 Subject: [PATCH 4/5] ci: fix styles --- src/Guard.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Guard.php b/src/Guard.php index 175909da..5ecde7b8 100644 --- a/src/Guard.php +++ b/src/Guard.php @@ -126,7 +126,7 @@ protected function getTokenFromRequest(Request $request) * Determine if the bearer token has valid format. * * @param string|null $token - * @return bool + * @return bool */ protected function hasValidBearerTokenFormat(string $token = null): bool { From 5275e789f56d8c577f2f9d0cfb3140cbfb193a68 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Fri, 13 Jan 2023 09:37:12 -0600 Subject: [PATCH 5/5] formatting --- src/Guard.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Guard.php b/src/Guard.php index 5ecde7b8..50f3c24c 100644 --- a/src/Guard.php +++ b/src/Guard.php @@ -119,19 +119,20 @@ protected function getTokenFromRequest(Request $request) $token = $request->bearerToken(); - return $this->hasValidBearerTokenFormat($token) ? $token : null; + return $this->isValidBearerToken($token) ? $token : null; } /** - * Determine if the bearer token has valid format. + * Determine if the bearer token is in the correct format. * * @param string|null $token * @return bool */ - protected function hasValidBearerTokenFormat(string $token = null): bool + protected function isValidBearerToken(string $token = null) { - if (! is_null($token) && strpos($token, '|') !== false) { + if (! is_null($token) && str_contains($token, '|')) { $model = new Sanctum::$personalAccessTokenModel; + if ($model->getKeyType() === 'int') { [$id, $token] = explode('|', $token, 2);