From 8d9af3e26214f07e833a6b8d8b60329fc71916a7 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Mon, 23 Jan 2023 17:11:39 +0100 Subject: [PATCH] feat(app-framework): Add support for global middlewares This allows apps to register middlewares that always register, not just for the app's own requests Signed-off-by: Christoph Wurst --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + .../Bootstrap/MiddlewareRegistration.php | 45 +++++++++++++++++++ .../Bootstrap/RegistrationContext.php | 13 +++--- .../DependencyInjection/DIContainer.php | 3 +- .../Bootstrap/IRegistrationContext.php | 4 +- .../Bootstrap/RegistrationContextTest.php | 2 +- .../DependencyInjection/DIContainerTest.php | 38 ++++++++++++++-- 8 files changed, 95 insertions(+), 12 deletions(-) create mode 100644 lib/private/AppFramework/Bootstrap/MiddlewareRegistration.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index a92b208ba7a..853d97f4a2e 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -664,6 +664,7 @@ return array( 'OC\\AppFramework\\Bootstrap\\Coordinator' => $baseDir . '/lib/private/AppFramework/Bootstrap/Coordinator.php', 'OC\\AppFramework\\Bootstrap\\EventListenerRegistration' => $baseDir . '/lib/private/AppFramework/Bootstrap/EventListenerRegistration.php', 'OC\\AppFramework\\Bootstrap\\FunctionInjector' => $baseDir . '/lib/private/AppFramework/Bootstrap/FunctionInjector.php', + 'OC\\AppFramework\\Bootstrap\\MiddlewareRegistration' => $baseDir . '/lib/private/AppFramework/Bootstrap/MiddlewareRegistration.php', 'OC\\AppFramework\\Bootstrap\\ParameterRegistration' => $baseDir . '/lib/private/AppFramework/Bootstrap/ParameterRegistration.php', 'OC\\AppFramework\\Bootstrap\\PreviewProviderRegistration' => $baseDir . '/lib/private/AppFramework/Bootstrap/PreviewProviderRegistration.php', 'OC\\AppFramework\\Bootstrap\\RegistrationContext' => $baseDir . '/lib/private/AppFramework/Bootstrap/RegistrationContext.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index c0ff8e6d9a4..455eccc45fd 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -697,6 +697,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\Bootstrap\\Coordinator' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Bootstrap/Coordinator.php', 'OC\\AppFramework\\Bootstrap\\EventListenerRegistration' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Bootstrap/EventListenerRegistration.php', 'OC\\AppFramework\\Bootstrap\\FunctionInjector' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Bootstrap/FunctionInjector.php', + 'OC\\AppFramework\\Bootstrap\\MiddlewareRegistration' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Bootstrap/MiddlewareRegistration.php', 'OC\\AppFramework\\Bootstrap\\ParameterRegistration' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Bootstrap/ParameterRegistration.php', 'OC\\AppFramework\\Bootstrap\\PreviewProviderRegistration' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Bootstrap/PreviewProviderRegistration.php', 'OC\\AppFramework\\Bootstrap\\RegistrationContext' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Bootstrap/RegistrationContext.php', diff --git a/lib/private/AppFramework/Bootstrap/MiddlewareRegistration.php b/lib/private/AppFramework/Bootstrap/MiddlewareRegistration.php new file mode 100644 index 00000000000..da226b311e0 --- /dev/null +++ b/lib/private/AppFramework/Bootstrap/MiddlewareRegistration.php @@ -0,0 +1,45 @@ + + * + * @author 2023 Christoph Wurst + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OC\AppFramework\Bootstrap; + +use OCP\AppFramework\Middleware; + +/** + * @psalm-immutable + * @template-extends ServiceRegistration + */ +class MiddlewareRegistration extends ServiceRegistration { + private bool $global; + + public function __construct(string $appId, string $service, bool $global) { + parent::__construct($appId, $service); + $this->global = $global; + } + + public function isGlobal(): bool { + return $this->global; + } +} diff --git a/lib/private/AppFramework/Bootstrap/RegistrationContext.php b/lib/private/AppFramework/Bootstrap/RegistrationContext.php index ac3f42ff2af..236aa106d02 100644 --- a/lib/private/AppFramework/Bootstrap/RegistrationContext.php +++ b/lib/private/AppFramework/Bootstrap/RegistrationContext.php @@ -94,7 +94,7 @@ class RegistrationContext { /** @var EventListenerRegistration[] */ private $eventListeners = []; - /** @var ServiceRegistration[] */ + /** @var MiddlewareRegistration[] */ private $middlewares = []; /** @var ServiceRegistration[] */ @@ -205,10 +205,11 @@ class RegistrationContext { ); } - public function registerMiddleware(string $class): void { + public function registerMiddleware(string $class, bool $global = false): void { $this->context->registerMiddleware( $this->appId, - $class + $class, + $global, ); } @@ -368,8 +369,8 @@ class RegistrationContext { /** * @psalm-param class-string $class */ - public function registerMiddleware(string $appId, string $class): void { - $this->middlewares[] = new ServiceRegistration($appId, $class); + public function registerMiddleware(string $appId, string $class, bool $global): void { + $this->middlewares[] = new MiddlewareRegistration($appId, $class, $global); } public function registerSearchProvider(string $appId, string $class) { @@ -617,7 +618,7 @@ class RegistrationContext { } /** - * @return ServiceRegistration[] + * @return MiddlewareRegistration[] */ public function getMiddlewareRegistrations(): array { return $this->middlewares; diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 6e7f054a80b..9b202f07fbf 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -321,7 +321,8 @@ class DIContainer extends SimpleContainer implements IAppContainer { if ($registrationContext !== null) { $appId = $this->getAppName(); foreach ($registrationContext->getMiddlewareRegistrations() as $middlewareRegistration) { - if ($middlewareRegistration->getAppId() === $appId) { + if ($middlewareRegistration->getAppId() === $appId + || $middlewareRegistration->isGlobal()) { $dispatcher->registerMiddleware($c->get($middlewareRegistration->getService())); } } diff --git a/lib/public/AppFramework/Bootstrap/IRegistrationContext.php b/lib/public/AppFramework/Bootstrap/IRegistrationContext.php index 7ff2bdca52e..3748714bb79 100644 --- a/lib/public/AppFramework/Bootstrap/IRegistrationContext.php +++ b/lib/public/AppFramework/Bootstrap/IRegistrationContext.php @@ -138,14 +138,16 @@ interface IRegistrationContext { /** * @param string $class + * @param bool $global load this middleware also for requests of other apps? Added in Nextcloud 26 * @psalm-param class-string<\OCP\AppFramework\Middleware> $class * * @return void * @see IAppContainer::registerMiddleWare() * * @since 20.0.0 + * @since 26.0.0 Added optional argument $global */ - public function registerMiddleware(string $class): void; + public function registerMiddleware(string $class, bool $global = false): void; /** * Register a search provider for the unified search diff --git a/tests/lib/AppFramework/Bootstrap/RegistrationContextTest.php b/tests/lib/AppFramework/Bootstrap/RegistrationContextTest.php index cee30b665fb..55f30a895f5 100644 --- a/tests/lib/AppFramework/Bootstrap/RegistrationContextTest.php +++ b/tests/lib/AppFramework/Bootstrap/RegistrationContextTest.php @@ -180,7 +180,7 @@ class RegistrationContextTest extends TestCase { } public function testGetMiddlewareRegistrations(): void { - $this->context->registerMiddleware('core', TwoFactorMiddleware::class); + $this->context->registerMiddleware('core', TwoFactorMiddleware::class, false); $registrations = $this->context->getMiddlewareRegistrations(); diff --git a/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php b/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php index bcf4e0a2771..04422643121 100644 --- a/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php +++ b/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php @@ -26,8 +26,8 @@ namespace Test\AppFramework\DependencyInjection; use OC\AppFramework\Bootstrap\Coordinator; +use OC\AppFramework\Bootstrap\MiddlewareRegistration; use OC\AppFramework\Bootstrap\RegistrationContext; -use OC\AppFramework\Bootstrap\ServiceRegistration; use OC\AppFramework\DependencyInjection\DIContainer; use OC\AppFramework\Http\Request; use OC\AppFramework\Middleware\Security\SecurityMiddleware; @@ -95,8 +95,40 @@ class DIContainerTest extends \Test\TestCase { $registrationContext = $this->createMock(RegistrationContext::class); $registrationContext->method('getMiddlewareRegistrations') ->willReturn([ - new ServiceRegistration($this->container['appName'], 'foo'), - new ServiceRegistration('otherapp', 'bar'), + new MiddlewareRegistration($this->container['appName'], 'foo', false), + new MiddlewareRegistration('otherapp', 'bar', false), + ]); + $this->container['foo'] = new class extends Middleware { + }; + $this->container['bar'] = new class extends Middleware { + }; + $coordinator->method('getRegistrationContext')->willReturn($registrationContext); + + $dispatcher = $this->container['MiddlewareDispatcher']; + + $middlewares = $dispatcher->getMiddlewares(); + self::assertNotEmpty($middlewares); + foreach ($middlewares as $middleware) { + if ($middleware === $this->container['bar']) { + $this->fail('Container must not register this middleware'); + } + if ($middleware === $this->container['foo']) { + // It is done + return; + } + } + $this->fail('Bootstrap registered middleware not found'); + } + + public function testMiddlewareDispatcherIncludesGlobalBootstrapMiddlewares(): void { + $coordinator = $this->createMock(Coordinator::class); + $this->container[Coordinator::class] = $coordinator; + $this->container['Request'] = $this->createMock(Request::class); + $registrationContext = $this->createMock(RegistrationContext::class); + $registrationContext->method('getMiddlewareRegistrations') + ->willReturn([ + new MiddlewareRegistration('otherapp', 'foo', true), + new MiddlewareRegistration('otherapp', 'bar', false), ]); $this->container['foo'] = new class extends Middleware { }; -- 2.39.5