diff --git a/DependencyInjection/Compiler/RememberMeServicesDecoratorCompilerPass.php b/DependencyInjection/Compiler/RememberMeServicesDecoratorCompilerPass.php new file mode 100644 index 00000000..c07c0028 --- /dev/null +++ b/DependencyInjection/Compiler/RememberMeServicesDecoratorCompilerPass.php @@ -0,0 +1,44 @@ +getDefinitions() as $definitionId => $definition) { + if (substr($definitionId, 0, $prefixLength) === self::REMEMBER_ME_LISTENER_ID_PREFIX) { + $this->decorateRememberMeServices($container, $definition); + } + } + } + + private function decorateRememberMeServices(ContainerBuilder $container, Definition $authListenerDefinition): void + { + // Get the remember-me services from the listener and decorate it + $rememberMeServicesId = (string) $authListenerDefinition->getArgument(1); + if ($rememberMeServicesId) { + $decoratedServiceId = $rememberMeServicesId.'.two_factor_decorator'; + $container + ->setDefinition($decoratedServiceId, new ChildDefinition('scheb_two_factor.security.rememberme_services_decorator')) + ->setDecoratedService($rememberMeServicesId) + ->replaceArgument(0, new Reference($decoratedServiceId.'.inner')); + } + } +} diff --git a/Resources/config/security.xml b/Resources/config/security.xml index 694b063f..9ea3b2b4 100644 --- a/Resources/config/security.xml +++ b/Resources/config/security.xml @@ -55,6 +55,10 @@ + + + + diff --git a/SchebTwoFactorBundle.php b/SchebTwoFactorBundle.php index 8a8c3c64..b51a0781 100644 --- a/SchebTwoFactorBundle.php +++ b/SchebTwoFactorBundle.php @@ -5,6 +5,7 @@ namespace Scheb\TwoFactorBundle; use Scheb\TwoFactorBundle\DependencyInjection\Compiler\AuthenticationProviderDecoratorCompilerPass; +use Scheb\TwoFactorBundle\DependencyInjection\Compiler\RememberMeServicesDecoratorCompilerPass; use Scheb\TwoFactorBundle\DependencyInjection\Compiler\TwoFactorFirewallConfigCompilerPass; use Scheb\TwoFactorBundle\DependencyInjection\Compiler\TwoFactorProviderCompilerPass; use Scheb\TwoFactorBundle\DependencyInjection\Factory\Security\TwoFactorFactory; @@ -19,6 +20,7 @@ public function build(ContainerBuilder $container) parent::build($container); $container->addCompilerPass(new AuthenticationProviderDecoratorCompilerPass()); + $container->addCompilerPass(new RememberMeServicesDecoratorCompilerPass()); $container->addCompilerPass(new TwoFactorProviderCompilerPass()); $container->addCompilerPass(new TwoFactorFirewallConfigCompilerPass()); diff --git a/Security/Authentication/RememberMe/RememberMeServicesDecorator.php b/Security/Authentication/RememberMe/RememberMeServicesDecorator.php new file mode 100644 index 00000000..9cadb9bc --- /dev/null +++ b/Security/Authentication/RememberMe/RememberMeServicesDecorator.php @@ -0,0 +1,54 @@ +decoratedRememberMeServices = $decoratedRememberMeServices; + } + + public function loginSuccess(Request $request, Response $response, TokenInterface $token) + { + if ($token instanceof TwoFactorTokenInterface) { + // Create a fake response to capture the remember-me cookie but not let it leak to the real response. + $cookieCaptureResponse = new Response(); + $this->decoratedRememberMeServices->loginSuccess($request, $cookieCaptureResponse, $token); + $rememberMeCookies = $cookieCaptureResponse->headers->getCookies(); + $token->setAttribute(TwoFactorTokenInterface::ATTRIBUTE_NAME_REMEMBER_ME_COOKIE, $rememberMeCookies); + } else { + // Not a TwoFactorToken => default behaviour + $this->decoratedRememberMeServices->loginSuccess($request, $response, $token); + } + } + + public function autoLogin(Request $request) + { + return $this->decoratedRememberMeServices->autoLogin($request); + } + + public function loginFail(Request $request, \Exception $exception = null) + { + $this->decoratedRememberMeServices->loginFail($request, $exception); + } + + public function logout(Request $request, Response $response, TokenInterface $token) + { + $this->decoratedRememberMeServices->logout($request, $response, $token); + } +} diff --git a/Security/Authentication/Token/TwoFactorTokenInterface.php b/Security/Authentication/Token/TwoFactorTokenInterface.php index 0e88379a..fc3f48cf 100644 --- a/Security/Authentication/Token/TwoFactorTokenInterface.php +++ b/Security/Authentication/Token/TwoFactorTokenInterface.php @@ -6,6 +6,8 @@ interface TwoFactorTokenInterface extends TokenInterface { + public const ATTRIBUTE_NAME_REMEMBER_ME_COOKIE = 'remember_me_cookie'; + /** * Return the authenticated token. * diff --git a/Security/Http/Firewall/TwoFactorListener.php b/Security/Http/Firewall/TwoFactorListener.php index 03d9ac92..016ca6ae 100644 --- a/Security/Http/Firewall/TwoFactorListener.php +++ b/Security/Http/Firewall/TwoFactorListener.php @@ -207,7 +207,7 @@ private function attemptAuthentication(Request $request, TwoFactorTokenInterface $this->dispatchTwoFactorAuthenticationEvent(TwoFactorAuthenticationEvents::ATTEMPT, $request, $token); $resultToken = $this->authenticationManager->authenticate($token); - return $this->onSuccess($request, $resultToken); + return $this->onSuccess($request, $resultToken, $currentToken); } catch (AuthenticationException $failed) { return $this->onFailure($request, $failed); } @@ -223,7 +223,7 @@ private function onFailure(Request $request, AuthenticationException $failed): R return $this->failureHandler->onAuthenticationFailure($request, $failed); } - private function onSuccess(Request $request, TokenInterface $token): Response + private function onSuccess(Request $request, TokenInterface $token, TwoFactorTokenInterface $previousTwoFactorToken): Response { if ($this->logger) { $this->logger->info('User has been two-factor authenticated successfully.', ['username' => $token->getUsername()]); @@ -244,7 +244,10 @@ private function onSuccess(Request $request, TokenInterface $token): Response $this->trustedDeviceManager->addTrustedDevice($token->getUser(), $this->firewallName); } - return $this->successHandler->onAuthenticationSuccess($request, $token); + $response = $this->successHandler->onAuthenticationSuccess($request, $token); + $this->addRememberMeCookies($previousTwoFactorToken, $response); + + return $response; } private function hasTrustedDeviceParameter(Request $request): bool @@ -263,4 +266,15 @@ private function dispatchTwoFactorAuthenticationEvent(string $eventType, Request $this->eventDispatcher->dispatch($eventType, $event); } } + + private function addRememberMeCookies(TwoFactorTokenInterface $twoFactorToken, Response $response): void + { + // Add the remember-me cookie that was previously suppressed by two-factor authentication + if ($twoFactorToken->hasAttribute(TwoFactorTokenInterface::ATTRIBUTE_NAME_REMEMBER_ME_COOKIE)) { + $rememberMeCookies = $twoFactorToken->getAttribute(TwoFactorTokenInterface::ATTRIBUTE_NAME_REMEMBER_ME_COOKIE); + foreach ($rememberMeCookies as $cookie) { + $response->headers->setCookie($cookie); + } + } + } } diff --git a/Tests/DependencyInjection/Compiler/RememberMeServicesDecoratorCompilerPassTest.php b/Tests/DependencyInjection/Compiler/RememberMeServicesDecoratorCompilerPassTest.php new file mode 100644 index 00000000..a96ad48f --- /dev/null +++ b/Tests/DependencyInjection/Compiler/RememberMeServicesDecoratorCompilerPassTest.php @@ -0,0 +1,76 @@ +container = new ContainerBuilder(); + $this->compilerPass = new RememberMeServicesDecoratorCompilerPass(); + } + + private function stubRememberMeListenersWithServices(array $firewalls): void + { + foreach ($firewalls as $firewallName) { + $rememberMeServicesId = 'rememberme_services.'.$firewallName; + $rememberMeServicesDefinition = new Definition(AbstractRememberMeServices::class); + $this->container->setDefinition($rememberMeServicesId, $rememberMeServicesDefinition); + + $listenerId = 'security.authentication.listener.rememberme.'.$firewallName; + $listenerDefinition = new Definition(RememberMeListener::class); + $listenerDefinition->setArgument(1, new Reference($rememberMeServicesId)); + $this->container->setDefinition($listenerId, $listenerDefinition); + } + } + + private function assertContainerHasDecoratedProvider(string $rememberMeServicesId): void + { + $expectedDecoratorId = $rememberMeServicesId.'.two_factor_decorator'; + $expectedDecoratedId = $expectedDecoratorId.'.inner'; + + $this->assertTrue($this->container->hasDefinition($expectedDecoratorId), 'Must have service "'.$expectedDecoratorId.'" defined.'); + + $decoratorDefinition = $this->container->getDefinition($expectedDecoratorId); + $decoratedServiceReference = $decoratorDefinition->getArgument(0); + $this->assertEquals($expectedDecoratedId, (string) $decoratedServiceReference); + $this->assertEquals($rememberMeServicesId, $decoratorDefinition->getDecoratedService()[0]); + } + + /** + * @test + */ + public function process_hasMultipleRemembermeServices_decorateAll(): void + { + $this->stubRememberMeListenersWithServices([ + 'firewall1', + 'firewall2', + ]); + + $this->compilerPass->process($this->container); + + $this->assertContainerHasDecoratedProvider('rememberme_services.firewall1'); + $this->assertContainerHasDecoratedProvider('rememberme_services.firewall2'); + } +} diff --git a/Tests/SchebTwoFactorBundleTest.php b/Tests/SchebTwoFactorBundleTest.php index 8fcdfc3d..82e3d1fe 100644 --- a/Tests/SchebTwoFactorBundleTest.php +++ b/Tests/SchebTwoFactorBundleTest.php @@ -5,6 +5,7 @@ namespace Scheb\TwoFactorBundle\Tests; use Scheb\TwoFactorBundle\DependencyInjection\Compiler\AuthenticationProviderDecoratorCompilerPass; +use Scheb\TwoFactorBundle\DependencyInjection\Compiler\RememberMeServicesDecoratorCompilerPass; use Scheb\TwoFactorBundle\DependencyInjection\Compiler\TwoFactorFirewallConfigCompilerPass; use Scheb\TwoFactorBundle\DependencyInjection\Compiler\TwoFactorProviderCompilerPass; use Scheb\TwoFactorBundle\DependencyInjection\Factory\Security\TwoFactorFactory; @@ -23,10 +24,11 @@ public function build_initializeBundle_addCompilerPass(): void //Expect compiler pass to be added $containerBuilder - ->expects($this->exactly(3)) + ->expects($this->exactly(4)) ->method('addCompilerPass') ->with($this->logicalOr( $this->isInstanceOf(AuthenticationProviderDecoratorCompilerPass::class), + $this->isInstanceOf(RememberMeServicesDecoratorCompilerPass::class), $this->isInstanceOf(TwoFactorProviderCompilerPass::class), $this->isInstanceOf(TwoFactorFirewallConfigCompilerPass::class) )); diff --git a/Tests/Security/Authentication/RememberMe/RememberMeServicesDecoratorTest.php b/Tests/Security/Authentication/RememberMe/RememberMeServicesDecoratorTest.php new file mode 100644 index 00000000..1ff937c8 --- /dev/null +++ b/Tests/Security/Authentication/RememberMe/RememberMeServicesDecoratorTest.php @@ -0,0 +1,101 @@ +request = $this->createMock(Request::class); + $this->response = $this->createMock(Response::class); + $this->innerRememberMeServices = $this->createMock(RememberMeServicesInterface::class); + $this->decorator = new RememberMeServicesDecorator($this->innerRememberMeServices); + } + + /** + * @test + */ + public function loginSuccess_noATwoFactorToken_forwardCall() + { + $token = $this->createMock(TokenInterface::class); + $this->innerRememberMeServices + ->expects($this->once()) + ->method('loginSuccess') + ->with( + $this->identicalTo($this->request), + $this->identicalTo($this->response), + $this->identicalTo($token) + ); + + $this->decorator->loginSuccess($this->request, $this->response, $token); + } + + /** + * @test + */ + public function loginSuccess_isTwoFactorToken_setRememberMeAttribute() + { + $token = $this->createMock(TwoFactorTokenInterface::class); + + $responseCallback = function ($argument) { + /** @var Response $argument */ + $this->assertInstanceOf(Response::class, $argument); + $this->assertFalse($argument === $this->response, 'Response objects must NOT be identical'); + $argument->headers->setCookie(new Cookie('name', 'value')); + + return true; + }; + + $this->innerRememberMeServices + ->expects($this->once()) + ->method('loginSuccess') + ->with( + $this->identicalTo($this->request), + $this->callback($responseCallback), // 2nd argument is a different Response instance + $this->identicalTo($token) + ); + + $token + ->expects($this->once()) + ->method('setAttribute') + ->with(TwoFactorTokenInterface::ATTRIBUTE_NAME_REMEMBER_ME_COOKIE, $this->callback(function ($argument) { + $this->assertContainsOnlyInstancesOf(Cookie::class, $argument); + + return true; + })); + + $this->decorator->loginSuccess($this->request, $this->response, $token); + } +} diff --git a/Tests/Security/Http/Firewall/TwoFactorListenerTest.php b/Tests/Security/Http/Firewall/TwoFactorListenerTest.php index 6206670a..a33ff919 100644 --- a/Tests/Security/Http/Firewall/TwoFactorListenerTest.php +++ b/Tests/Security/Http/Firewall/TwoFactorListenerTest.php @@ -18,6 +18,7 @@ use Scheb\TwoFactorBundle\Security\TwoFactor\Trusted\TrustedDeviceManagerInterface; use Scheb\TwoFactorBundle\Tests\TestCase; use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -188,7 +189,7 @@ protected function setUp(): void /** * @return MockObject|TokenInterface */ - private function createTwoFactorToken($firewallName = self::FIREWALL_NAME, $authenticatedToken = null): MockObject + private function createTwoFactorToken($firewallName = self::FIREWALL_NAME, $authenticatedToken = null, array $attributes = []): MockObject { $twoFactorToken = $this->createMock(TwoFactorTokenInterface::class); $twoFactorToken @@ -203,6 +204,22 @@ private function createTwoFactorToken($firewallName = self::FIREWALL_NAME, $auth ->expects($this->any()) ->method('getAuthenticatedToken') ->willReturn($authenticatedToken ?? $this->createMock(TokenInterface::class)); + $twoFactorToken + ->expects($this->any()) + ->method('getAttributes') + ->willReturn($attributes); + $twoFactorToken + ->expects($this->any()) + ->method('hasAttribute') + ->willReturnCallback(function ($key) use ($attributes) { + return isset($attributes[$key]); + }); + $twoFactorToken + ->expects($this->any()) + ->method('getAttribute') + ->willReturnCallback(function ($key) use ($attributes) { + return $attributes[$key] ?? null; + }); return $twoFactorToken; } @@ -236,16 +253,19 @@ private function stubRequestHasParameter(string $parameterName, $value): void $this->requestParams[$parameterName] = $value; } - private function stubHandlersReturnResponse(): void + private function stubHandlersReturnResponse(): Response { + $response = new Response(); $this->successHandler ->expects($this->any()) ->method('onAuthenticationSuccess') - ->willReturn($this->createMock(Response::class)); + ->willReturn($response); $this->failureHandler ->expects($this->any()) ->method('onAuthenticationFailure') - ->willReturn($this->createMock(Response::class)); + ->willReturn($response); + + return $response; } private function stubAuthenticationManagerReturnsToken(MockObject $returnedToken): void @@ -653,4 +673,30 @@ public function handle_twoFactorProcessCompleteWithTrustedDisabled_notSetTrusted ($this->listener)($this->getResponseEvent); } + + /** + * @test + */ + public function handle_twoFactorProcessCompleteWithRememberMeEnabled_setRememberMeCookies(): void + { + $authenticatedToken = $this->createMock(TokenInterface::class); + $authenticatedToken + ->expects($this->any()) + ->method('getUser') + ->willReturn('user'); + + $rememberMeCookie = new Cookie('remember_me', 'value'); + $attributes = [TwoFactorTokenInterface::ATTRIBUTE_NAME_REMEMBER_ME_COOKIE => [$rememberMeCookie]]; + $twoFactorToken = $this->createTwoFactorToken(self::FIREWALL_NAME, null, $attributes); + + $this->stubTokenManagerHasToken($twoFactorToken); + $this->stubCurrentPath(self::CHECK_PATH); + $this->stubCsrfTokenValidatorHasValidCsrfTokenReturnsTrue(); + $this->stubAuthenticationManagerReturnsToken($authenticatedToken); // Not a TwoFactorToken + $response = $this->stubHandlersReturnResponse(); + + ($this->listener)($this->getResponseEvent); + + $this->assertContains($rememberMeCookie, $response->headers->getCookies()); + } }