diff --git a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php index 34e8b3ae7f2bf..38d5f7970ecdc 100644 --- a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php +++ b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php @@ -64,6 +64,7 @@ use Symfony\Component\Routing\DependencyInjection\RoutingResolverPass; use Symfony\Component\Runtime\SymfonyRuntime; use Symfony\Component\Scheduler\DependencyInjection\AddScheduleMessengerPass; +use Symfony\Component\Security\Http\DependencyInjection\UriSignerLocatorPass; use Symfony\Component\Serializer\DependencyInjection\SerializerPass; use Symfony\Component\Translation\DependencyInjection\DataCollectorTranslatorPass; use Symfony\Component\Translation\DependencyInjection\LoggingTranslatorPass; @@ -192,6 +193,7 @@ public function build(ContainerBuilder $container): void $container->addCompilerPass(new VirtualRequestStackPass()); $container->addCompilerPass(new TranslationUpdateCommandPass(), PassConfig::TYPE_BEFORE_REMOVING); $this->addCompilerPassIfExists($container, StreamablePass::class); + $this->addCompilerPassIfExists($container, UriSignerLocatorPass::class); if ($container->getParameter('kernel.debug')) { $container->addCompilerPass(new AddDebugLogProcessorPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 2); diff --git a/src/Symfony/Component/HttpFoundation/CHANGELOG.md b/src/Symfony/Component/HttpFoundation/CHANGELOG.md index ca58a4032d8b8..5283586fd0e6b 100644 --- a/src/Symfony/Component/HttpFoundation/CHANGELOG.md +++ b/src/Symfony/Component/HttpFoundation/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG --- * Deprecate using `Request::sendHeaders()` after headers have already been sent; use a `StreamedResponse` instead + * Add `#[WithHttpStatus]` to define status codes: 404 for `SignedUriException` and 403 for `ExpiredSignedUriException` 7.3 --- diff --git a/src/Symfony/Component/HttpFoundation/Exception/ExpiredSignedUriException.php b/src/Symfony/Component/HttpFoundation/Exception/ExpiredSignedUriException.php index 613e08ef46c63..8264c1b5a67ee 100644 --- a/src/Symfony/Component/HttpFoundation/Exception/ExpiredSignedUriException.php +++ b/src/Symfony/Component/HttpFoundation/Exception/ExpiredSignedUriException.php @@ -11,9 +11,13 @@ namespace Symfony\Component\HttpFoundation\Exception; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Attribute\WithHttpStatus; + /** * @author Kevin Bond */ +#[WithHttpStatus(Response::HTTP_FORBIDDEN)] final class ExpiredSignedUriException extends SignedUriException { /** diff --git a/src/Symfony/Component/HttpFoundation/Exception/SignedUriException.php b/src/Symfony/Component/HttpFoundation/Exception/SignedUriException.php index 17b729d315d70..8b4a28be94919 100644 --- a/src/Symfony/Component/HttpFoundation/Exception/SignedUriException.php +++ b/src/Symfony/Component/HttpFoundation/Exception/SignedUriException.php @@ -11,9 +11,13 @@ namespace Symfony\Component\HttpFoundation\Exception; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Attribute\WithHttpStatus; + /** * @author Kevin Bond */ +#[WithHttpStatus(Response::HTTP_NOT_FOUND)] abstract class SignedUriException extends \RuntimeException implements ExceptionInterface { } diff --git a/src/Symfony/Component/Security/Http/Attribute/IsSignatureValid.php b/src/Symfony/Component/Security/Http/Attribute/IsSignatureValid.php new file mode 100644 index 0000000000000..2494a3f7ca0d7 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Attribute/IsSignatureValid.php @@ -0,0 +1,50 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Attribute; + +use Symfony\Component\HttpFoundation\UriSigner; + +/** + * Validates the request signature for specific HTTP methods. + * + * This class determines whether a request's signature should be validated + * based on the configured HTTP methods. If the request method matches one + * of the specified methods (or if no methods are specified), the signature + * is checked. + * + * If the signature is invalid, a {@see \Symfony\Component\HttpFoundation\Exception\SignedUriException} + * is thrown during validation. + * + * @author Santiago San Martin + */ +#[\Attribute(\Attribute::IS_REPEATABLE | \Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD | \Attribute::TARGET_FUNCTION)] +final class IsSignatureValid +{ + public readonly array $methods; + public readonly string $signer; + + /** + * @param string[]|string $methods HTTP methods that require signature validation. An empty array means that no method filtering is done + * @param string $signer The ID of the UriSigner service to use for signature validation. Defaults to 'uri_signer' + */ + public function __construct( + array|string $methods = [], + string $signer = 'uri_signer', + ) { + if (!method_exists(UriSigner::class, 'verify')) { + throw new \LogicException('The `IsSignatureValid` attribute requires symfony/http-foundation >= 7.3.'); + } + + $this->methods = (array) $methods; + $this->signer = $signer; + } +} diff --git a/src/Symfony/Component/Security/Http/CHANGELOG.md b/src/Symfony/Component/Security/Http/CHANGELOG.md index 6c485dc6e5450..45cb9a6b7b51e 100644 --- a/src/Symfony/Component/Security/Http/CHANGELOG.md +++ b/src/Symfony/Component/Security/Http/CHANGELOG.md @@ -6,6 +6,7 @@ CHANGELOG * Deprecate callable firewall listeners, extend `AbstractListener` or implement `FirewallListenerInterface` instead * Deprecate `AbstractListener::__invoke` + * Add `#[IsSignatureValid]` attribute to validate URI signatures 7.3 --- diff --git a/src/Symfony/Component/Security/Http/DependencyInjection/UriSignerLocatorPass.php b/src/Symfony/Component/Security/Http/DependencyInjection/UriSignerLocatorPass.php new file mode 100644 index 0000000000000..3f8d37836e192 --- /dev/null +++ b/src/Symfony/Component/Security/Http/DependencyInjection/UriSignerLocatorPass.php @@ -0,0 +1,45 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\DependencyInjection; + +use Psr\Container\ContainerInterface; +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\HttpFoundation\UriSigner; +use Symfony\Component\Security\Http\EventListener\IsSignatureValidAttributeListener; + +/** + * Registers all UriSigner services in a service locator and injects it into the IsSignatureValidAttributeListener for dynamic signer resolution. + * + * @author Santiago San Martin + */ +class UriSignerLocatorPass implements CompilerPassInterface +{ + public function process(ContainerBuilder $container): void + { + $locateableServices = []; + foreach ($container->getDefinitions() as $id => $definition) { + if (UriSigner::class === $definition->getClass()) { + $locateableServices[$id] = new Reference($id); + } + } + + $container + ->register('controller.is_signature_valid_attribute_listener', IsSignatureValidAttributeListener::class) + ->addTag('kernel.event_subscriber') + ->setBindings([ + ContainerInterface::class => ServiceLocatorTagPass::register($container, $locateableServices), + ]); + } +} diff --git a/src/Symfony/Component/Security/Http/EventListener/IsSignatureValidAttributeListener.php b/src/Symfony/Component/Security/Http/EventListener/IsSignatureValidAttributeListener.php new file mode 100644 index 0000000000000..02ced770c960f --- /dev/null +++ b/src/Symfony/Component/Security/Http/EventListener/IsSignatureValidAttributeListener.php @@ -0,0 +1,59 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\EventListener; + +use Psr\Container\ContainerInterface; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\HttpFoundation\UriSigner; +use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent; +use Symfony\Component\HttpKernel\KernelEvents; +use Symfony\Component\Security\Http\Attribute\IsSignatureValid; + +/** + * Handles the IsSignatureValid attribute. + * + * @author Santiago San Martin + */ +class IsSignatureValidAttributeListener implements EventSubscriberInterface +{ + public function __construct( + private readonly ContainerInterface $container, + ) { + } + + public function onKernelControllerArguments(ControllerArgumentsEvent $event): void + { + if (!\is_array($attributes = $event->getAttributes(IsSignatureValid::class) ?? null)) { + return; + } + + $request = $event->getRequest(); + foreach ($attributes as $attribute) { + $methods = array_map('strtoupper', $attribute->methods); + if ($methods && !\in_array($request->getMethod(), $methods, true)) { + continue; + } + + $signer = $this->container->get($attribute->signer); + if (!$signer instanceof UriSigner) { + throw new \LogicException(\sprintf('The service "%s" is not an instance of "%s".', $attribute->signer, UriSigner::class)); + } + + $signer->verify($request); + } + } + + public static function getSubscribedEvents(): array + { + return [KernelEvents::CONTROLLER_ARGUMENTS => ['onKernelControllerArguments', 30]]; + } +} diff --git a/src/Symfony/Component/Security/Http/Tests/DependencyInjection/UriSignerLocatorPassTest.php b/src/Symfony/Component/Security/Http/Tests/DependencyInjection/UriSignerLocatorPassTest.php new file mode 100644 index 0000000000000..3ae1991bd55cb --- /dev/null +++ b/src/Symfony/Component/Security/Http/Tests/DependencyInjection/UriSignerLocatorPassTest.php @@ -0,0 +1,46 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\DependencyInjection; + +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\HttpFoundation\UriSigner; +use Symfony\Component\Security\Http\DependencyInjection\UriSignerLocatorPass; + +class UriSignerLocatorPassTest extends TestCase +{ + public function testSetUriSignerLocator() + { + $container = new ContainerBuilder(); + + $container->register('controller.is_signature_valid_attribute_listener'); + + $container->register('UriSignerOne')->setClass(UriSigner::class); + $container->register('UriSignerTwo')->setClass(UriSigner::class); + $container->register('UriSignerThree')->setClass(UriSigner::class); + $container->register('One')->setClass('Foo'); + $container->register('Two')->setClass('Bar'); + $container->register('Three')->setClass('Baz'); + + $pass = new UriSignerLocatorPass(); + $pass->process($container); + + $id = (string) $container->getDefinition('controller.is_signature_valid_attribute_listener')->getBindings()[ContainerInterface::class]->getValues()[0]; + $arguments = $container->getDefinition($id)->getArguments()[0]; + + $this->assertCount(3, $arguments); + $this->assertArrayHasKey('UriSignerOne', $arguments); + $this->assertArrayHasKey('UriSignerTwo', $arguments); + $this->assertArrayHasKey('UriSignerThree', $arguments); + } +} diff --git a/src/Symfony/Component/Security/Http/Tests/EventListener/IsSignatureValidAttributeListenerTest.php b/src/Symfony/Component/Security/Http/Tests/EventListener/IsSignatureValidAttributeListenerTest.php new file mode 100644 index 0000000000000..7db79ed95bc10 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Tests/EventListener/IsSignatureValidAttributeListenerTest.php @@ -0,0 +1,220 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\EventListener; + +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use Symfony\Component\HttpFoundation\Exception\UnsignedUriException; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\UriSigner; +use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent; +use Symfony\Component\HttpKernel\HttpKernelInterface; +use Symfony\Component\Security\Http\EventListener\IsSignatureValidAttributeListener; +use Symfony\Component\Security\Http\Tests\Fixtures\IsSignatureValidAttributeController; +use Symfony\Component\Security\Http\Tests\Fixtures\IsSignatureValidAttributeMethodsController; + +/** + * @requires function \Symfony\Component\HttpFoundation\UriSigner::verify + */ +class IsSignatureValidAttributeListenerTest extends TestCase +{ + public function testInvokableControllerWithValidSignature() + { + $request = new Request(); + + $signer = $this->createMock(UriSigner::class); + $signer->expects($this->once())->method('verify')->with($request); + $kernel = $this->createMock(HttpKernelInterface::class); + $container = $this->createMock(ContainerInterface::class); + $container->expects($this->once())->method('get')->with('uri_signer')->willReturn($signer); + + $event = new ControllerArgumentsEvent( + $kernel, + new IsSignatureValidAttributeController(), + [], + $request, + null + ); + + $listener = new IsSignatureValidAttributeListener($container); + $listener->onKernelControllerArguments($event); + } + + public function testNoAttributeSkipsValidation() + { + $kernel = $this->createMock(HttpKernelInterface::class); + $container = $this->createMock(ContainerInterface::class); + $container->expects($this->never())->method('get'); + + $event = new ControllerArgumentsEvent( + $kernel, + [new IsSignatureValidAttributeMethodsController(), 'noAttribute'], + [], + new Request(), + null + ); + + $listener = new IsSignatureValidAttributeListener($container); + $listener->onKernelControllerArguments($event); + } + + public function testDefaultCheckRequestSucceeds() + { + $request = new Request(); + $signer = $this->createMock(UriSigner::class); + $signer->expects($this->once())->method('verify')->with($request); + $kernel = $this->createMock(HttpKernelInterface::class); + $container = $this->createMock(ContainerInterface::class); + $container->expects($this->once())->method('get')->with('uri_signer')->willReturn($signer); + + $event = new ControllerArgumentsEvent( + $kernel, + [new IsSignatureValidAttributeMethodsController(), 'withDefaultBehavior'], + [], + $request, + null + ); + + $listener = new IsSignatureValidAttributeListener($container); + $listener->onKernelControllerArguments($event); + } + + public function testCheckRequestFailsThrowsHttpException() + { + $request = new Request(); + $signer = $this->createMock(UriSigner::class); + $signer->expects($this->once())->method('verify')->willThrowException(new UnsignedUriException()); + $kernel = $this->createMock(HttpKernelInterface::class); + $container = $this->createMock(ContainerInterface::class); + $container->expects($this->once())->method('get')->with('uri_signer')->willReturn($signer); + + $event = new ControllerArgumentsEvent( + $kernel, + [new IsSignatureValidAttributeMethodsController(), 'withDefaultBehavior'], + [], + $request, + null + ); + + $listener = new IsSignatureValidAttributeListener($container); + + $this->expectException(UnsignedUriException::class); + $listener->onKernelControllerArguments($event); + } + + public function testMultipleAttributesAllValid() + { + $request = new Request(); + + $signer = $this->createMock(UriSigner::class); + $signer->expects($this->exactly(2))->method('verify')->with($request); + $kernel = $this->createMock(HttpKernelInterface::class); + $container = $this->createMock(ContainerInterface::class); + $container->expects($this->exactly(2))->method('get')->with('uri_signer')->willReturn($signer); + + $event = new ControllerArgumentsEvent( + $kernel, + [new IsSignatureValidAttributeMethodsController(), 'withMultiple'], + [], + $request, + null + ); + + $listener = new IsSignatureValidAttributeListener($container); + $listener->onKernelControllerArguments($event); + } + + public function testValidationWithStringMethod() + { + $request = new Request([], [], [], [], [], ['REQUEST_METHOD' => 'POST']); + + $signer = $this->createMock(UriSigner::class); + $signer->expects($this->once())->method('verify')->with($request); + $kernel = $this->createMock(HttpKernelInterface::class); + $container = $this->createMock(ContainerInterface::class); + $container->expects($this->once())->method('get')->with('uri_signer')->willReturn($signer); + + $event = new ControllerArgumentsEvent( + $kernel, + [new IsSignatureValidAttributeMethodsController(), 'withPostOnly'], + [], + $request, + null + ); + + $listener = new IsSignatureValidAttributeListener($container); + $listener->onKernelControllerArguments($event); + } + + public function testValidationWithArrayMethods() + { + $request = new Request([], [], [], [], [], ['REQUEST_METHOD' => 'POST']); + + $signer = $this->createMock(UriSigner::class); + $signer->expects($this->once())->method('verify')->with($request); + $kernel = $this->createMock(HttpKernelInterface::class); + $container = $this->createMock(ContainerInterface::class); + $container->expects($this->once())->method('get')->with('uri_signer')->willReturn($signer); + + $event = new ControllerArgumentsEvent( + $kernel, + [new IsSignatureValidAttributeMethodsController(), 'withGetAndPost'], + [], + $request, + null + ); + + $listener = new IsSignatureValidAttributeListener($container); + $listener->onKernelControllerArguments($event); + } + + public function testValidationSkippedForNonMatchingMethod() + { + $request = new Request([], [], [], [], [], ['REQUEST_METHOD' => 'GET']); + + $kernel = $this->createMock(HttpKernelInterface::class); + $container = $this->createMock(ContainerInterface::class); + $container->expects($this->never())->method('get'); + + $event = new ControllerArgumentsEvent( + $kernel, + [new IsSignatureValidAttributeMethodsController(), 'withPostOnly'], + [], + $request, + null + ); + + $listener = new IsSignatureValidAttributeListener($container); + $listener->onKernelControllerArguments($event); + } + + public function testValidationWithSigner() + { + $request = new Request(); + $customSigner = $this->createMock(UriSigner::class); + $customSigner->expects($this->once())->method('verify')->with($request); + $kernel = $this->createMock(HttpKernelInterface::class); + $container = $this->createMock(ContainerInterface::class); + $container->expects($this->once())->method('get')->with('app.test.signer')->willReturn($customSigner); + + $event = new ControllerArgumentsEvent( + $kernel, + [new IsSignatureValidAttributeMethodsController(), 'withCustomSigner'], + [], + $request, + null + ); + + $listener = new IsSignatureValidAttributeListener($container); + $listener->onKernelControllerArguments($event); + } +} diff --git a/src/Symfony/Component/Security/Http/Tests/Fixtures/IsSignatureValidAttributeController.php b/src/Symfony/Component/Security/Http/Tests/Fixtures/IsSignatureValidAttributeController.php new file mode 100644 index 0000000000000..c5dd5ac87c671 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Tests/Fixtures/IsSignatureValidAttributeController.php @@ -0,0 +1,22 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\Fixtures; + +use Symfony\Component\Security\Http\Attribute\IsSignatureValid; + +#[IsSignatureValid] +class IsSignatureValidAttributeController +{ + public function __invoke() + { + } +} diff --git a/src/Symfony/Component/Security/Http/Tests/Fixtures/IsSignatureValidAttributeMethodsController.php b/src/Symfony/Component/Security/Http/Tests/Fixtures/IsSignatureValidAttributeMethodsController.php new file mode 100644 index 0000000000000..b99bd274545e9 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Tests/Fixtures/IsSignatureValidAttributeMethodsController.php @@ -0,0 +1,47 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\Fixtures; + +use Symfony\Component\Security\Http\Attribute\IsSignatureValid; + +class IsSignatureValidAttributeMethodsController +{ + public function noAttribute() + { + } + + #[IsSignatureValid] + public function withDefaultBehavior() + { + } + + #[IsSignatureValid] + #[IsSignatureValid] + public function withMultiple() + { + } + + #[IsSignatureValid(methods: 'POST')] + public function withPostOnly() + { + } + + #[IsSignatureValid(methods: ['GET', 'POST'])] + public function withGetAndPost() + { + } + + #[IsSignatureValid(signer: 'app.test.signer')] + public function withCustomSigner() + { + } +} diff --git a/src/Symfony/Component/Security/Http/composer.json b/src/Symfony/Component/Security/Http/composer.json index 2d5ed369a7f57..faf3e91ceec92 100644 --- a/src/Symfony/Component/Security/Http/composer.json +++ b/src/Symfony/Component/Security/Http/composer.json @@ -37,7 +37,8 @@ "symfony/security-csrf": "^6.4|^7.0|^8.0", "symfony/translation": "^6.4|^7.0|^8.0", "psr/log": "^1|^2|^3", - "web-token/jwt-library": "^3.3.2|^4.0" + "web-token/jwt-library": "^3.3.2|^4.0", + "symfony/dependency-injection": "^6.4|^7.0|^8.0" }, "conflict": { "symfony/clock": "<6.4",