Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/auth/src/OAuth/GenericOAuthClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public function authenticate(Request $request, Closure $map): Authenticatable

$this->session->remove($this->sessionKey);

if ($expectedState !== $actualState) {
if ($expectedState === null || $expectedState === '' || $expectedState !== $actualState) {
throw new OAuthStateWasInvalid();
}

Expand Down
2 changes: 1 addition & 1 deletion packages/auth/src/OAuth/Testing/TestingOAuthClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public function authenticate(Request $request, Closure $map): Authenticatable

$this->state = null;

if ($expectedState !== $actualState) {
if ($expectedState === null || $expectedState === '' || $expectedState !== $actualState) {
throw new OAuthStateWasInvalid();
}

Expand Down
69 changes: 69 additions & 0 deletions tests/Integration/Auth/OAuth/GenericOAuthClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,22 @@

namespace Tests\Tempest\Integration\Auth\OAuth;

use League\OAuth2\Client\Provider\AbstractProvider;
use League\OAuth2\Client\Provider\ResourceOwnerInterface;
use League\OAuth2\Client\Token\AccessToken;
use LogicException;
use PHPUnit\Framework\Attributes\Test;
use Psr\Http\Message\ResponseInterface;
use ReflectionClass;
use Tempest\Auth\Exceptions\OAuthStateWasInvalid;
use Tempest\Auth\Exceptions\OAuthWasNotConfigured;
use Tempest\Auth\OAuth\Config\GitHubOAuthConfig;
use Tempest\Auth\OAuth\GenericOAuthClient;
use Tempest\Auth\OAuth\OAuthClient;
use Tempest\Http\GenericRequest;
use Tempest\Http\Method;
use Tempest\Http\Session\Session;
use Tempest\Support\Uri;
use Tests\Tempest\Integration\FrameworkIntegrationTestCase;

final class GenericOAuthClientTest extends FrameworkIntegrationTestCase
Expand Down Expand Up @@ -52,4 +62,63 @@ public function state_is_set_when_redirect_is_created(): void

$this->assertNotNull($session->get($oauth->sessionKey));
}

#[Test]
public function missing_session_state_is_rejected(): void
{
$this->container->config(new GitHubOAuthConfig(
clientId: 'client-id',
clientSecret: 'client-secret', // @mago-expect lint:no-literal-password
redirectTo: '/oauth/callback',
scopes: ['user:email'],
));

/** @var GenericOAuthClient $oauth */
$oauth = $this->container->get(OAuthClient::class);

$reflection = new ReflectionClass($oauth);
$reflection->getProperty('provider')->setValue($oauth, new class extends AbstractProvider {
public function getBaseAuthorizationUrl(): string
{
return 'https://provider.test/authorize';
}

public function getBaseAccessTokenUrl(array $params): string
{
return 'https://provider.test/token';
}

public function getResourceOwnerDetailsUrl(AccessToken $token): string
{
return 'https://provider.test/user';
}

public function getAccessToken($grant, array $options = [])
{
throw new LogicException('Access token should not be requested when state is missing.');
}

protected function getDefaultScopes(): array
{
return [];
}

protected function checkResponse(ResponseInterface $response, $data): void {}

protected function createResourceOwner(array $response, AccessToken $token): ResourceOwnerInterface
{
throw new LogicException('Resource owner should not be created when state is missing.');
}
});

$this->expectException(OAuthStateWasInvalid::class);

$oauth->authenticate(
request: new GenericRequest(
method: Method::GET,
uri: Uri\set_query('/oauth/callback', code: 'auth-code'),
),
map: static fn () => throw new LogicException('User should not be mapped when state is missing.'),
);
}
}
28 changes: 28 additions & 0 deletions tests/Integration/Auth/OAuth/TestingOAuthClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,34 @@ public function state_is_cleared_even_when_validation_fails(): void

$this->assertNull($client->getState());
}

#[Test]
public function missing_state_is_rejected(): void
{
$this->database->reset(migrate: false);
$this->database->migrate(CreateMigrationsTable::class, CreateUsersTable::class);

$this->container->config(new GitHubOAuthConfig(
clientId: 'foo',
clientSecret: 'bar', // @mago-expect lint:no-literal-password
redirectTo: '/oauth/github',
));

$client = $this->oauth->fake($this->user);

$this->expectException(OAuthStateWasInvalid::class);

$client->authenticate(
request: new GenericRequest(
method: Method::GET,
uri: Uri\set_query('/oauth/callback', code: 'auth-code'),
),
map: static fn (OAuthUser $user): User => query(User::class)->updateOrCreate(
['github_id' => $user->id],
['email' => $user->email ?? '', 'full_name' => $user->name ?? '', 'username' => $user->nickname ?? ''],
),
);
}
}

final class User implements Authenticatable
Expand Down
Loading