From 4f1675d19047d5e9f8dd4a6b9253e0b4241609b3 Mon Sep 17 00:00:00 2001 From: Joseph Bielawski Date: Mon, 6 Dec 2021 17:14:17 +0100 Subject: [PATCH] Bugfix | Adjust `OAuthAuthenticator` to be compatible with new Symfony Security --- .../Factory/OAuthAuthenticatorFactory.php | 1 + .../Security/Factory/OAuthFactory.php | 17 +++++++++++++++++ .../Http/Authenticator/OAuthAuthenticator.php | 15 ++++++++++++--- .../Controller/ConnectControllerTest.php | 5 ----- Tests/Functional/IntegrationTest.php | 11 ++++++++--- .../Authenticator/OAuthAuthenticatorTest.php | 12 ++++++++---- 6 files changed, 46 insertions(+), 15 deletions(-) diff --git a/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php b/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php index ef1fb3ebc..e180d459a 100644 --- a/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php +++ b/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php @@ -67,6 +67,7 @@ public function createAuthenticator( ->addArgument($config['resource_owners']) ->addArgument(new Reference($this->createAuthenticationSuccessHandler($container, $firewallName, $config))) ->addArgument(new Reference($this->createAuthenticationFailureHandler($container, $firewallName, $config))) + ->addArgument(array_intersect_key($config, $this->options)) ; return $authenticatorId; diff --git a/DependencyInjection/Security/Factory/OAuthFactory.php b/DependencyInjection/Security/Factory/OAuthFactory.php index 45e18b160..bc3d015fc 100644 --- a/DependencyInjection/Security/Factory/OAuthFactory.php +++ b/DependencyInjection/Security/Factory/OAuthFactory.php @@ -59,6 +59,23 @@ public function getPosition(): string return 'http'; } + public function getPriority(): int + { + return 1; + } + + /** + * {@inheritDoc} + */ + public function createAuthenticator( + ContainerBuilder $container, + string $firewallName, + array $config, + string $userProviderId + ): string { + throw new \RuntimeException('Deprecated "OAuthFactory" cannot create new Symfony Authenticator!'); + } + /** * Gets a reference to the resource owner map. */ diff --git a/Security/Http/Authenticator/OAuthAuthenticator.php b/Security/Http/Authenticator/OAuthAuthenticator.php index 5592a36c0..f9ac5ed1b 100644 --- a/Security/Http/Authenticator/OAuthAuthenticator.php +++ b/Security/Http/Authenticator/OAuthAuthenticator.php @@ -31,12 +31,13 @@ use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Passport; use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport; +use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface; use Symfony\Component\Security\Http\HttpUtils; /** * @author Vadim Borodavko */ -final class OAuthAuthenticator implements AuthenticatorInterface +final class OAuthAuthenticator implements AuthenticatorInterface, AuthenticationEntryPointInterface { private HttpUtils $httpUtils; private OAuthAwareUserProviderInterface $userProvider; @@ -53,6 +54,7 @@ final class OAuthAuthenticator implements AuthenticatorInterface private ?string $resourceOwnerName = null; private ?string $refreshToken = null; private ?int $createdAt = null; + private array $options; public function __construct( HttpUtils $httpUtils, @@ -60,7 +62,8 @@ public function __construct( ResourceOwnerMapInterface $resourceOwnerMap, array $checkPaths, AuthenticationSuccessHandlerInterface $successHandler, - AuthenticationFailureHandlerInterface $failureHandler + AuthenticationFailureHandlerInterface $failureHandler, + array $options ) { $this->failureHandler = $failureHandler; $this->successHandler = $successHandler; @@ -68,6 +71,7 @@ public function __construct( $this->resourceOwnerMap = $resourceOwnerMap; $this->userProvider = $userProvider; $this->httpUtils = $httpUtils; + $this->options = $options; } public function supports(Request $request): bool @@ -81,6 +85,11 @@ public function supports(Request $request): bool return false; } + public function start(Request $request, AuthenticationException $authException = null): Response + { + return new RedirectResponse($this->httpUtils->generateUri($request, $this->options['login_path'])); + } + /** * @throws AuthenticationException * @throws LazyResponseException @@ -170,7 +179,7 @@ public function onAuthenticationSuccess(Request $request, TokenInterface $token, return $this->successHandler->onAuthenticationSuccess($request, $token); } - public function onAuthenticationFailure(Request $request, AuthenticationException $exception): ?Response + public function onAuthenticationFailure(Request $request, AuthenticationException $exception): Response { return $this->failureHandler->onAuthenticationFailure($request, $exception); } diff --git a/Tests/Functional/Controller/ConnectControllerTest.php b/Tests/Functional/Controller/ConnectControllerTest.php index 71570ed20..15746136a 100644 --- a/Tests/Functional/Controller/ConnectControllerTest.php +++ b/Tests/Functional/Controller/ConnectControllerTest.php @@ -28,11 +28,6 @@ final class ConnectControllerTest extends WebTestCase { use AuthenticationHelperTrait; - protected function setUp(): void - { - static::$class = AppKernel::class; - } - public static function getKernelClass(): string { return AppKernel::class; diff --git a/Tests/Functional/IntegrationTest.php b/Tests/Functional/IntegrationTest.php index 0779a4ff5..198a10cb5 100644 --- a/Tests/Functional/IntegrationTest.php +++ b/Tests/Functional/IntegrationTest.php @@ -13,6 +13,7 @@ namespace HWI\Bundle\OAuthBundle\Tests\Functional; +use HWI\Bundle\OAuthBundle\Tests\App\AppKernel; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\HttpClient\Response\MockResponse; @@ -20,9 +21,16 @@ final class IntegrationTest extends WebTestCase { + public static function getKernelClass(): string + { + return AppKernel::class; + } + public function testRequestRedirect(): void { $client = static::createClient(); + $client->disableReboot(); + $client->getContainer()->set('hwi_oauth.http_client', new MockHttpClient()); $client->request('GET', '/'); $response = $client->getResponse(); @@ -37,9 +45,6 @@ public function testRequestRedirect(): void $this->assertInstanceOf(Response::class, $response); $this->assertSame(200, $response->getStatusCode(), 'No landing, got redirect to '.$response->headers->get('Location')); - $client->disableReboot(); - $client->getContainer()->set('hwi_oauth.http_client', new MockHttpClient()); - $client->click($crawler->selectLink('Login')->link()); $response = $client->getResponse(); diff --git a/Tests/Security/Http/Authenticator/OAuthAuthenticatorTest.php b/Tests/Security/Http/Authenticator/OAuthAuthenticatorTest.php index 2c18df606..73930c2e2 100644 --- a/Tests/Security/Http/Authenticator/OAuthAuthenticatorTest.php +++ b/Tests/Security/Http/Authenticator/OAuthAuthenticatorTest.php @@ -68,7 +68,8 @@ public function testSupports(): void $this->getResourceOwnerMap(), ['/a', '/b'], $this->getAuthenticationSuccessHandlerMock(), - $this->getAuthenticationFailureHandlerMock() + $this->getAuthenticationFailureHandlerMock(), + [] ); $this->assertTrue($authenticator->supports($request)); @@ -148,7 +149,8 @@ public function testAuthenticate(): void $resourceOwnerMap, [], $this->getAuthenticationSuccessHandlerMock(), - $this->getAuthenticationFailureHandlerMock() + $this->getAuthenticationFailureHandlerMock(), + [] ); $passport = $authenticator->authenticate($request); @@ -187,7 +189,8 @@ public function testOnAuthenticationSuccess(): void $this->getResourceOwnerMap(), [], $successHandlerMock, - $this->getAuthenticationFailureHandlerMock() + $this->getAuthenticationFailureHandlerMock(), + [] ); $this->assertSame($response, $authenticator->onAuthenticationSuccess($request, $token, 'main')); @@ -212,7 +215,8 @@ public function testOnAuthenticationFailure(): void $this->getResourceOwnerMap(), [], $this->getAuthenticationSuccessHandlerMock(), - $failureHandlerMock + $failureHandlerMock, + [] ); $this->assertSame($response, $authenticator->onAuthenticationFailure($request, $exception));