From 54a6d7d10c3f1b840357dbbfedd026b98cc72b49 Mon Sep 17 00:00:00 2001 From: Tim van Dijen <tim.dijen@minbzk.nl> Date: Tue, 31 Dec 2019 21:31:32 +0100 Subject: [PATCH] Refactor routing (#1178) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Added Kernel class to handle request * Kernel handle request Kernel is loaded in SimpleSAML\Module class Removed Router and ControllerResolver classes * Added route command (squash) * Use symfony application and cache * Updated to today's standards * Fix for Symfony4 & Catch environment from ENV variable * Standardize location of routes/services files * Trying to make slashes work properly in all situations * Convert XML to YML * Fix some template-names and endpoints * Rename Controller-classes * Update dependencies * Fix routing files * TooManyArguments * Fix TypeCoercion * PSR-12 * Fix rebase mistake * Rebase lock-file * Fix Psalm * Add strict_types declaration Co-authored-by: Sergio GĂłmez <decano@gmail.com> --- bin/console | 18 ++ composer.json | 10 +- lib/SimpleSAML/Command/RouterDebugCommand.php | 94 +++++++++ lib/SimpleSAML/Console/Application.php | 25 +++ lib/SimpleSAML/HTTP/Router.php | 139 ------------ lib/SimpleSAML/Kernel.php | 172 +++++++++++++++ lib/SimpleSAML/Module.php | 37 +++- lib/SimpleSAML/Module/ControllerResolver.php | 198 ------------------ .../Resources/config/services/simplesaml.xml | 22 -- lib/SimpleSAML/Utils/XML.php | 8 +- lib/SimpleSAML/XHTML/Template.php | 1 - .../Config.php} | 4 +- .../Federation.php} | 8 +- modules/admin/lib/{ => Controller}/Menu.php | 2 +- .../Test.php} | 5 +- modules/admin/routes.yaml | 24 --- modules/admin/routing/routes/routes.yml | 24 +++ modules/core/lib/Controller/Exception.php | 26 ++- modules/core/lib/Controller/Login.php | 7 +- modules/core/routing/routes/routes.yml | 24 +++ psalm.xml | 7 - routing/routes/routes.yml | 13 ++ routing/services/console.yml | 4 + routing/services/routing.yml | 72 +++++++ routing/services/services.yml | 33 +++ routing/services/simplesaml.yml | 16 ++ routing/services/web.yml | 22 ++ ...{LoginControllerTest.php => LoginTest.php} | 0 28 files changed, 598 insertions(+), 417 deletions(-) create mode 100755 bin/console create mode 100644 lib/SimpleSAML/Command/RouterDebugCommand.php create mode 100644 lib/SimpleSAML/Console/Application.php delete mode 100644 lib/SimpleSAML/HTTP/Router.php create mode 100644 lib/SimpleSAML/Kernel.php delete mode 100644 lib/SimpleSAML/Module/ControllerResolver.php delete mode 100644 lib/SimpleSAML/Resources/config/services/simplesaml.xml rename modules/admin/lib/{ConfigController.php => Controller/Config.php} (99%) rename modules/admin/lib/{FederationController.php => Controller/Federation.php} (99%) rename modules/admin/lib/{ => Controller}/Menu.php (98%) rename modules/admin/lib/{TestController.php => Controller/Test.php} (98%) delete mode 100644 modules/admin/routes.yaml create mode 100644 modules/admin/routing/routes/routes.yml create mode 100644 modules/core/routing/routes/routes.yml create mode 100644 routing/routes/routes.yml create mode 100644 routing/services/console.yml create mode 100644 routing/services/routing.yml create mode 100644 routing/services/services.yml create mode 100644 routing/services/simplesaml.yml create mode 100644 routing/services/web.yml rename tests/modules/core/lib/Controller/{LoginControllerTest.php => LoginTest.php} (100%) diff --git a/bin/console b/bin/console new file mode 100755 index 000000000..a01bc1443 --- /dev/null +++ b/bin/console @@ -0,0 +1,18 @@ +#!/usr/bin/env php +<?php + +use SimpleSAML\Console\Application; +use SimpleSAML\Kernel; +use Symfony\Component\Console\Input\ArgvInput; + +umask(000); +set_time_limit(0); + +require __DIR__.'/../vendor/autoload.php'; + +$input = new ArgvInput(); +$module = $input->getParameterOption(['--modules', '-m'], 'core'); +$kernel = new Kernel($module); + +$application = new Application($kernel); +$application->run($input); diff --git a/composer.json b/composer.json index 5c5ef94f0..36fdec9c5 100644 --- a/composer.json +++ b/composer.json @@ -75,11 +75,15 @@ "simplesamlphp/simplesamlphp-module-statistics": "^0.9", "simplesamlphp/simplesamlphp-module-sqlauth": "^0.9", "simplesamlphp/twig-configurable-i18n": "^2.1", - "symfony/routing": "^3.4 || ^4.0", - "symfony/http-foundation": "^3.4 || ^4.0", + "symfony/cache": "^3.4 || ^4.0", "symfony/config": "^3.4 || ^4.0", - "symfony/http-kernel": "^3.4 || ^4.0", + "symfony/console": "^3.4 || ^4.0", "symfony/dependency-injection": "^3.4 || ^4.0", + "symfony/finder": "^3.4 || ^4.0", + "symfony/framework-bundle": "^3.4 || ^4.0", + "symfony/http-foundation": "^3.4 || ^4.0", + "symfony/http-kernel": "^3.4 || ^4.0", + "symfony/routing": "^3.4 || ^4.0", "symfony/yaml": "^3.4 || ^4.0", "twig/twig": "~1.0 || ~2.0", "webmozart/assert": "~1.4", diff --git a/lib/SimpleSAML/Command/RouterDebugCommand.php b/lib/SimpleSAML/Command/RouterDebugCommand.php new file mode 100644 index 000000000..2fba9f6a3 --- /dev/null +++ b/lib/SimpleSAML/Command/RouterDebugCommand.php @@ -0,0 +1,94 @@ +<?php + +declare(strict_types=1); + +namespace SimpleSAML\Command; + +use Closure; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Helper\Table; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Style\SymfonyStyle; +use Symfony\Component\Routing\RouterInterface; + +class RouterDebugCommand extends Command +{ + /** + * @var string + */ + protected static $defaultName = 'debug:router'; + + /** + * @var RouterInterface + */ + private $router; + + + /** + * {@inheritdoc} + */ + public function __construct(RouterInterface $router) + { + parent::__construct(); + $this->router = $router; + } + + + /** + * {@inheritDoc} + * @return void + */ + protected function configure() + { + $this + ->setDescription('Displays current routes for a module') + ->setHelp( + <<<'EOF' +The <info>%command.name%</info> displays the configured routes for a module: + + <info>php %command.full_name%</info> +EOF + ) + ; + } + + + /** + * {@inheritdoc} + * @psalm-suppress InvalidReturnType + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $io = new SymfonyStyle($input, $output); + $routes = $this->router->getRouteCollection(); + + $tableHeaders = array('Name', 'Method', 'Scheme', 'Host', 'Path', 'Controller'); + + $tableRows = array(); + foreach ($routes->all() as $name => $route) { + $row = [ + $name, + $route->getMethods() ? implode('|', $route->getMethods()) : 'ANY', + $route->getSchemes() ? implode('|', $route->getSchemes()) : 'ANY', + '' !== $route->getHost() ? $route->getHost() : 'ANY', + $route->getPath(), + ]; + + $controller = $route->getDefault('_controller'); + if ($controller instanceof Closure) { + $controller = 'Closure'; + } elseif (is_object($controller)) { + $controller = get_class($controller); + } + $row[] = $controller; + + $tableRows[] = $row; + } + + $table = new Table($io); + $table->setHeaders($tableHeaders)->setRows($tableRows); + $table->setStyle('compact'); + $table->render(); + } +} diff --git a/lib/SimpleSAML/Console/Application.php b/lib/SimpleSAML/Console/Application.php new file mode 100644 index 000000000..d4e5fb8c4 --- /dev/null +++ b/lib/SimpleSAML/Console/Application.php @@ -0,0 +1,25 @@ +<?php + +declare(strict_types=1); + +namespace SimpleSAML\Console; + +use SimpleSAML\Kernel; +use Symfony\Bundle\FrameworkBundle\Console\Application as BaseApplication; +use Symfony\Component\Console\Input\InputOption; + +class Application extends BaseApplication +{ + /** + * @param \SimpleSAML\Kernel $kernel + */ + public function __construct(Kernel $kernel) + { + parent::__construct($kernel); + + $inputDefinition = $this->getDefinition(); + $inputDefinition->addOption( + new InputOption('--module', '-m', InputOption::VALUE_REQUIRED, 'The module name', $kernel->getModule()) + ); + } +} diff --git a/lib/SimpleSAML/HTTP/Router.php b/lib/SimpleSAML/HTTP/Router.php deleted file mode 100644 index 40497af1d..000000000 --- a/lib/SimpleSAML/HTTP/Router.php +++ /dev/null @@ -1,139 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace SimpleSAML\HTTP; - -use SimpleSAML\Configuration; -use SimpleSAML\Module\ControllerResolver; -use SimpleSAML\Session; -use Symfony\Component\EventDispatcher\EventDispatcher; -use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\RequestStack; -use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\HttpKernel\Controller\ArgumentResolver; -use Symfony\Component\HttpKernel\HttpKernel; -use Symfony\Component\Routing\RequestContext; - -/** - * Class that routes requests to responses. - * - * @package SimpleSAML - */ -class Router -{ - /** @var \Symfony\Component\HttpKernel\Controller\ArgumentResolver */ - protected $arguments; - - /** @var \SimpleSAML\Configuration|null */ - protected $config = null; - - /** @var \Symfony\Component\Routing\RequestContext */ - protected $context; - - /** @var \Symfony\Component\EventDispatcher\EventDispatcher */ - protected $dispatcher; - - /** @var \Symfony\Component\HttpFoundation\Request|null */ - protected $request = null; - - /** @var \SimpleSAML\Module\ControllerResolver */ - protected $resolver; - - /** @var \SimpleSAML\Session|null */ - protected $session = null; - - /** @var \Symfony\Component\HttpFoundation\RequestStack|null */ - protected $stack = null; - - - /** - * Router constructor. - * - * @param string $module - */ - public function __construct($module) - { - $this->arguments = new ArgumentResolver(); - $this->context = new RequestContext(); - $this->resolver = new ControllerResolver($module); - $this->dispatcher = new EventDispatcher(); - } - - - /** - * Process a given request. - * - * If no specific arguments are given, the default instances will be used (configuration, session, etc). - * - * @param \Symfony\Component\HttpFoundation\Request|null $request - * The request to process. Defaults to the current one. - * - * @return \Symfony\Component\HttpFoundation\Response A response suitable for the given request. - * - * @throws \Exception If an error occurs. - */ - public function process(Request $request = null) - { - if ($this->config === null) { - $this->setConfiguration(Configuration::getInstance()); - } - if ($this->session === null) { - $this->setSession(Session::getSessionFromRequest()); - } - - if ($request === null) { - $this->request = Request::createFromGlobals(); - } else { - $this->request = $request; - } - - $stack = new RequestStack(); - $stack->push($this->request); - $this->context->fromRequest($this->request); - $kernel = new HttpKernel($this->dispatcher, $this->resolver, $stack, $this->resolver); - return $kernel->handle($this->request); - } - - - /** - * Send a given response to the browser. - * - * @param \Symfony\Component\HttpFoundation\Response $response The response to send. - * @return void - */ - public function send(Response $response) - { - if ($this->request === null) { - throw new \Exception("No request found to respond to"); - } - $response->prepare($this->request); - $response->send(); - } - - - /** - * Set the configuration to use by the controller. - * - * @param \SimpleSAML\Configuration $config - * @return void - */ - public function setConfiguration(Configuration $config) - { - $this->config = $config; - $this->resolver->setConfiguration($config); - } - - - /** - * Set the session to use by the controller. - * - * @param \SimpleSAML\Session $session - * @return void - */ - public function setSession(Session $session) - { - $this->session = $session; - $this->resolver->setSession($session); - } -} diff --git a/lib/SimpleSAML/Kernel.php b/lib/SimpleSAML/Kernel.php new file mode 100644 index 000000000..2127d870d --- /dev/null +++ b/lib/SimpleSAML/Kernel.php @@ -0,0 +1,172 @@ +<?php + +declare(strict_types=1); + +namespace SimpleSAML; + +use Symfony\Bundle\FrameworkBundle\FrameworkBundle; +use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait; +use Symfony\Component\Config\Exception\FileLocatorFileNotFoundException; +use Symfony\Component\Config\FileLocator; +use Symfony\Component\Config\Loader\LoaderInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Loader\DirectoryLoader; +use Symfony\Component\HttpKernel\Kernel as BaseKernel; +use Symfony\Component\Routing\RouteCollectionBuilder; + +/** + * A class to create the container and handle a given request. + */ +class Kernel extends BaseKernel +{ + use MicroKernelTrait; + + const CONFIG_EXTS = '.{php,xml,yaml,yml}'; + + /** + * @var string + */ + private $module; + + + /** + * @param string $module + */ + public function __construct($module) + { + $this->module = $module; + + $env = getenv('APP_ENV') ?: (getenv('SYMFONY_ENV') ?: 'prod'); + + parent::__construct($env, false); + } + + + /** + * @return string + */ + public function getCacheDir() + { + $configuration = Configuration::getInstance(); + $cachePath = $configuration->getString('tempdir') . '/cache/' . $this->module; + + if (0 === strpos($cachePath, '/')) { + return $cachePath; + } + + return $configuration->getBaseDir() . '/' . $cachePath; + } + + + /** + * @return string + */ + public function getLogDir() + { + $configuration = Configuration::getInstance(); + $loggingPath = $configuration->getString('loggingdir'); + + if (0 === strpos($loggingPath, '/')) { + return $loggingPath; + } + + return $configuration->getBaseDir() . '/' . $loggingPath; + } + + + /** + * {@inheritdoc} + */ + public function registerBundles() + { + return [ + new FrameworkBundle(), + ]; + } + + + /** + * Get the module loaded in this kernel. + * + * @return string + */ + public function getModule() + { + return $this->module; + } + + + /** + * Configures the container. + * + * @param ContainerBuilder $container + * @param LoaderInterface $loader + * @return void + */ + protected function configureContainer(ContainerBuilder $container, LoaderInterface $loader) + { + $configuration = Configuration::getInstance(); + $baseDir = $configuration->getBaseDir(); + $loader->load($baseDir . '/routing/services/*' . self::CONFIG_EXTS, 'glob'); + $confDir = Module::getModuleDir($this->module) . '/routing/services'; + if (is_dir($confDir)) { + $loader->load($confDir . '/**/*' . self::CONFIG_EXTS, 'glob'); + } + + $container->loadFromExtension('framework', [ + 'secret' => Configuration::getInstance()->getString('secretsalt'), + ]); + + $this->registerModuleControllers($container); + } + + + /** + * Import routes. + * + * @param RouteCollectionBuilder $routes + * @return void + */ + protected function configureRoutes(RouteCollectionBuilder $routes) + { + $configuration = Configuration::getInstance(); + $baseDir = $configuration->getBaseDir(); + $routes->import($baseDir . '/routing/routes/*' . self::CONFIG_EXTS, '/', 'glob'); + $confDir = Module::getModuleDir($this->module) . '/routing/routes'; + if (is_dir($confDir)) { + $routes->import($confDir . '/**/*' . self::CONFIG_EXTS, $this->module, 'glob'); + } + } + + + /** + * @param ContainerBuilder $container + * @return void + */ + private function registerModuleControllers(ContainerBuilder $container) + { + try { + $definition = new Definition(); + $definition->setAutowired(true); + $definition->setPublic(true); + + $controllerDir = Module::getModuleDir($this->module) . '/lib/Controller'; + + if (!is_dir($controllerDir)) { + return; + } + + $loader = new DirectoryLoader( + $container, + new FileLocator($controllerDir . '/') + ); + $loader->registerClasses( + $definition, + 'SimpleSAML\\Module\\' . $this->module . '\\Controller\\', + $controllerDir . '/*' + ); + } catch (FileLocatorFileNotFoundException $e) { + } + } +} diff --git a/lib/SimpleSAML/Module.php b/lib/SimpleSAML/Module.php index 55f16cfaa..2072fb0cf 100644 --- a/lib/SimpleSAML/Module.php +++ b/lib/SimpleSAML/Module.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace SimpleSAML; -use SimpleSAML\HTTP\Router; +use SimpleSAML\Kernel; use SimpleSAML\Utils; use Symfony\Component\Config\Exception\FileLocatorFileNotFoundException; use Symfony\Component\HttpFoundation\BinaryFileResponse; @@ -145,14 +145,12 @@ class Module $modEnd = strpos($url, '/', 1); if ($modEnd === false) { - // the path must always be on the form /module/ - throw new Error\NotFound('The URL must at least contain a module name followed by a slash.'); - } - - $module = substr($url, 1, $modEnd - 1); - $url = substr($url, $modEnd + 1); - if ($url === false) { + $modEnd = strlen($url); + $module = substr($url, 1); $url = ''; + } else { + $module = substr($url, 1, $modEnd - 1); + $url = substr($url, $modEnd + 1); } if (!self::isModuleEnabled($module)) { @@ -186,9 +184,12 @@ class Module ); if ($config->getBoolean('usenewui', false) === true) { - $router = new Router($module); try { - return $router->process($request); + $kernel = new Kernel($module); + $response = $kernel->handle($request); + $kernel->terminate($request, $response); + + return $response; } catch (FileLocatorFileNotFoundException $e) { // no routes configured for this module, fall back to the old system } catch (NotFoundHttpException $e) { @@ -557,6 +558,22 @@ class Module } + /** + * Handle a valid request for a module that lacks a trailing slash. + * + * This method add the trailing slash and redirects to the resulting URL. + * + * @param Request $request The request to process by this controller method. + * + * @return RedirectResponse A redirection to the URI specified in the request, but with a trailing slash. + */ + public static function addTrailingSlash(Request $request) + { + // Must be of form /{module} - append a slash + return new RedirectResponse($request->getRequestUri() . '/', 308); + } + + /** * Handle a valid request that ends with a trailing slash. * diff --git a/lib/SimpleSAML/Module/ControllerResolver.php b/lib/SimpleSAML/Module/ControllerResolver.php deleted file mode 100644 index 21638217e..000000000 --- a/lib/SimpleSAML/Module/ControllerResolver.php +++ /dev/null @@ -1,198 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace SimpleSAML\Module; - -use SimpleSAML\Auth\AuthenticationFactory; -use SimpleSAML\Configuration; -use SimpleSAML\Error\Exception; -use SimpleSAML\Module; -use SimpleSAML\Session; -use Symfony\Component\Config\Exception\FileLocatorFileNotFoundException; -use Symfony\Component\Config\FileLocator; -use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Reference; -use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Controller\ArgumentResolverInterface; -use Symfony\Component\HttpKernel\Controller\ControllerResolver as SymfonyControllerResolver; -use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; -use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadataFactory; -use Symfony\Component\Routing\Exception\ResourceNotFoundException; -use Symfony\Component\Routing\Loader\YamlFileLoader; -use Symfony\Component\Routing\Matcher\UrlMatcher; -use Symfony\Component\Routing\RequestContext; -use Symfony\Component\Routing\Route; -use Symfony\Component\Routing\RouteCollection; - -/** - * A class to resolve module controllers based on a given request. - * - * This class allows us to find a controller (a callable) that's configured for a given URL. - * - * @package SimpleSAML - */ -class ControllerResolver extends SymfonyControllerResolver implements ArgumentResolverInterface -{ - /** @var ArgumentMetadataFactory */ - protected $argFactory; - - /** @var ContainerBuilder */ - protected $container; - - /** @var string */ - protected $module; - - /** @var array */ - protected $params = []; - - /** @var RouteCollection|null */ - protected $routes; - - - /** - * Build a module controller resolver. - * - * @param string $module The name of the module. - */ - public function __construct($module) - { - parent::__construct(); - $this->module = $module; - - $loader = new YamlFileLoader( - new FileLocator(Module::getModuleDir($this->module)) - ); - - $this->argFactory = new ArgumentMetadataFactory(); - $this->container = new ContainerBuilder(); - $this->container->autowire(AuthenticationFactory::class, AuthenticationFactory::class); - - try { - $this->routes = $loader->load('routes.yaml'); - $redirect = new Route( - '/{url}', - ['_controller' => '\SimpleSAML\Module::removeTrailingSlash'], - ['url' => '.*/$'] - ); - $this->routes->add('trailing-slash', $redirect); - $this->routes->addPrefix('/' . $this->module); - } catch (FileLocatorFileNotFoundException $e) { - } - } - - - /** - * Get the controller associated with a given URL, based on a request. - * - * This method searches for a 'routes.yaml' file in the root of the module, defining valid routes for the module - * and mapping them given controllers. It's input is a Request object with the request that we want to serve. - * - * @param Request $request The request we need to find a controller for. - * - * @return callable|false A controller (as a callable) that can handle the request, or false if we cannot find - * one suitable for the given request. - */ - public function getController(Request $request) - { - if ($this->routes === null) { - return false; - } - $ctxt = new RequestContext(); - $ctxt->fromRequest($request); - - try { - $matcher = new UrlMatcher($this->routes, $ctxt); - $this->params = $matcher->match($ctxt->getPathInfo()); - list($class, $method) = explode('::', $this->params['_controller']); - $this->container->register($class, $class)->setAutowired(true)->setPublic(true); - $this->container->compile(); - return [$this->container->get($class), $method]; - } catch (ResourceNotFoundException $e) { - // no route defined matching this request - } - return false; - } - - - /** - * Get the arguments that should be passed to a controller from a given request. - * - * When the signature of the controller includes arguments with type Request, the given request will be passed to - * those. Otherwise, they'll be matched by name. If no value is available for a given argument, the method will - * try to set a default value or null, if possible. - * - * @param Request $request The request that holds all the information needed by the controller. - * @param callable $controller A controller for the given request. - * - * @return array An array of arguments that should be passed to the controller, in order. - * - * @throws \SimpleSAML\Error\Exception If we don't find anything suitable for an argument in the controller's - * signature. - */ - public function getArguments(Request $request, $controller) - { - $args = []; - $metadata = $this->argFactory->createArgumentMetadata($controller); - - /** @var ArgumentMetadata $argMeta */ - foreach ($metadata as $argMeta) { - if ($argMeta->getType() === Request::class) { - // add request argument - $args[] = $request; - continue; - } - - $argName = $argMeta->getName(); - if (array_key_exists($argName, $this->params)) { - // add argument by name - $args[] = $this->params[$argName]; - continue; - } - - // URL does not contain value for this argument - if ($argMeta->hasDefaultValue()) { - // it has a default value - $args[] = $argMeta->getDefaultValue(); - } - - // no default value - if ($argMeta->isNullable()) { - $args[] = null; - } - - throw new Exception('Missing value for argument ' . $argName . '. This is probably a bug.'); - } - - return $args; - } - - - /** - * Set the configuration to use by the controllers. - * - * @param \SimpleSAML\Configuration $config - * @return void - */ - public function setConfiguration(Configuration $config) - { - $this->container->set(Configuration::class, $config); - $this->container->register(Configuration::class)->setSynthetic(true)->setAutowired(true); - } - - - /** - * Set the session to use by the controllers. - * - * @param \SimpleSAML\Session $session - * @return void - */ - public function setSession(Session $session) - { - $this->container->set(Session::class, $session); - $this->container->register(Session::class) - ->setSynthetic(true) - ->setAutowired(true) - ->addMethodCall('setConfiguration', [new Reference(Configuration::class)]); - } -} diff --git a/lib/SimpleSAML/Resources/config/services/simplesaml.xml b/lib/SimpleSAML/Resources/config/services/simplesaml.xml deleted file mode 100644 index ce6b47efa..000000000 --- a/lib/SimpleSAML/Resources/config/services/simplesaml.xml +++ /dev/null @@ -1,22 +0,0 @@ -<?xml version="1.0" ?> - -<container xmlns="http://symfony.com/schema/dic/services" - xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" - xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - - <services> - <defaults public="false"/> - - <service id="SimpleSAML\Configuration"> - <factory class="SimpleSAML\Configuration" method="getInstance" /> - </service> - - <service id="SimpleSAML\Session"> - <factory class="SimpleSAML\Session" method="getSessionFromRequest" /> - </service> - - <service class="SimpleSAML\Auth\AuthenticationFactory"> - <argument type="service" id="SimpleSAML\Configuration"/> - </service> - </services> -</container> diff --git a/lib/SimpleSAML/Utils/XML.php b/lib/SimpleSAML/Utils/XML.php index 89db4344c..8c178fee1 100644 --- a/lib/SimpleSAML/Utils/XML.php +++ b/lib/SimpleSAML/Utils/XML.php @@ -452,7 +452,13 @@ class XML $schemaFile = $schemaPath . '/' . $schema; libxml_set_external_entity_loader( - function ($public, $system, $context) { + /** + * @param string|null $public + * @param string $system + * @param array $context + * @return string|null + */ + function (string $public = null, string $system, array $context) { if (filter_var($system, FILTER_VALIDATE_URL) === $system) { return null; } diff --git a/lib/SimpleSAML/XHTML/Template.php b/lib/SimpleSAML/XHTML/Template.php index 0a73ea26f..4dd75b68f 100644 --- a/lib/SimpleSAML/XHTML/Template.php +++ b/lib/SimpleSAML/XHTML/Template.php @@ -317,7 +317,6 @@ class Template extends Response $twig->addFunction(new TwigFunction('moduleURL', [Module::class, 'getModuleURL'])); - // initialize some basic context $langParam = $this->configuration->getString('language.parameter.name', 'language'); $twig->addGlobal('languageParameterName', $langParam); diff --git a/modules/admin/lib/ConfigController.php b/modules/admin/lib/Controller/Config.php similarity index 99% rename from modules/admin/lib/ConfigController.php rename to modules/admin/lib/Controller/Config.php index c61474ab4..0961fa9d4 100644 --- a/modules/admin/lib/ConfigController.php +++ b/modules/admin/lib/Controller/Config.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace SimpleSAML\Module\admin; +namespace SimpleSAML\Module\admin\Controller; use SimpleSAML\Configuration; use SimpleSAML\HTTP\RunnableResponse; @@ -20,7 +20,7 @@ use Symfony\Component\HttpFoundation\Request; * * @package SimpleSAML\Module\admin */ -class ConfigController +class Config { const LATEST_VERSION_STATE_KEY = 'core:latest_simplesamlphp_version'; diff --git a/modules/admin/lib/FederationController.php b/modules/admin/lib/Controller/Federation.php similarity index 99% rename from modules/admin/lib/FederationController.php rename to modules/admin/lib/Controller/Federation.php index 9fa0f9651..351e91ec7 100644 --- a/modules/admin/lib/FederationController.php +++ b/modules/admin/lib/Controller/Federation.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace SimpleSAML\Module\admin; +namespace SimpleSAML\Module\admin\Controller; use SimpleSAML\Auth; use SimpleSAML\Configuration; @@ -30,9 +30,8 @@ use Symfony\Component\HttpFoundation\ResponseHeaderBag; * * @package SimpleSAML\Module\admin */ -class FederationController +class Federation { - /** @var \SimpleSAML\Configuration */ protected $config; @@ -385,6 +384,7 @@ class FederationController return $entities; } + /** * Metadata converter * @@ -454,6 +454,7 @@ class FederationController return $this->menu->insert($t); } + /** * Download a certificate for a given entity. * @@ -496,6 +497,7 @@ class FederationController return $response; } + /** * Show remote entity metadata * diff --git a/modules/admin/lib/Menu.php b/modules/admin/lib/Controller/Menu.php similarity index 98% rename from modules/admin/lib/Menu.php rename to modules/admin/lib/Controller/Menu.php index 0384ef2e2..6887fab1a 100644 --- a/modules/admin/lib/Menu.php +++ b/modules/admin/lib/Controller/Menu.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace SimpleSAML\Module\admin; +namespace SimpleSAML\Module\admin\Controller; use SimpleSAML\Locale\Translate; use SimpleSAML\Module; diff --git a/modules/admin/lib/TestController.php b/modules/admin/lib/Controller/Test.php similarity index 98% rename from modules/admin/lib/TestController.php rename to modules/admin/lib/Controller/Test.php index 7b9a1d49e..c03018d56 100644 --- a/modules/admin/lib/TestController.php +++ b/modules/admin/lib/Controller/Test.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace SimpleSAML\Module\admin; +namespace SimpleSAML\Module\admin\Controller; use SAML2\Constants; use SAML2\XML\saml\NameID; @@ -25,7 +25,7 @@ use Webmozart\Assert\Assert; * * @package SimpleSAML\Module\admin */ -class TestController +class Test { /** @var \SimpleSAML\Configuration */ protected $config; @@ -54,6 +54,7 @@ class TestController /** * Display the list of available authsources. * + * @param \Symfony\Component\HttpFoundation\Request $request * @param string|null $as * @return \SimpleSAML\XHTML\Template */ diff --git a/modules/admin/routes.yaml b/modules/admin/routes.yaml deleted file mode 100644 index 34490c3f9..000000000 --- a/modules/admin/routes.yaml +++ /dev/null @@ -1,24 +0,0 @@ -admin-main: - path: / - defaults: { _controller: 'SimpleSAML\Module\admin\ConfigController::main' } -admin-diagnostics: - path: /diagnostics - defaults: { _controller: 'SimpleSAML\Module\admin\ConfigController::diagnostics' } -admin-phpinfo: - path: /phpinfo - defaults: { _controller: 'SimpleSAML\Module\admin\ConfigController::phpinfo' } -admin-test: - path: /test/{as} - defaults: { _controller: 'SimpleSAML\Module\admin\TestController::main', as: null } -admin-fed: - path: /federation - defaults: { _controller: 'SimpleSAML\Module\admin\FederationController::main' } -admin-fed-cert: - path: /federation/cert - defaults: { _controller: 'SimpleSAML\Module\admin\FederationController::downloadCert' } -admin-fed-show: - path: /federation/show - defaults: { _controller: 'SimpleSAML\Module\admin\FederationController::showRemoteEntity' } -admin-fed-converter: - path: /federation/metadata-converter - defaults: { _controller: 'SimpleSAML\Module\admin\FederationController::metadataConverter' } diff --git a/modules/admin/routing/routes/routes.yml b/modules/admin/routing/routes/routes.yml new file mode 100644 index 000000000..3cff6ed4c --- /dev/null +++ b/modules/admin/routing/routes/routes.yml @@ -0,0 +1,24 @@ +admin-main: + path: / + defaults: { _controller: 'SimpleSAML\Module\admin\Controller\Config::main' } +admin-diagnostics: + path: /diagnostics + defaults: { _controller: 'SimpleSAML\Module\admin\Controller\Config::diagnostics' } +admin-phpinfo: + path: /phpinfo + defaults: { _controller: 'SimpleSAML\Module\admin\Controller\Config::phpinfo' } +admin-test: + path: /test/{as} + defaults: { _controller: 'SimpleSAML\Module\admin\Controller\Test::main', as: null } +admin-fed: + path: /federation + defaults: { _controller: 'SimpleSAML\Module\admin\Controller\Federation::main' } +admin-fed-cert: + path: /federation/cert + defaults: { _controller: 'SimpleSAML\Module\admin\Controller\Federation::downloadCert' } +admin-fed-show: + path: /federation/show + defaults: { _controller: 'SimpleSAML\Module\admin\Controller\Federation::showRemoteEntity' } +admin-fed-converter: + path: /federation/metadata-converter + defaults: { _controller: 'SimpleSAML\Module\admin\Controller\Federation::metadataConverter' } diff --git a/modules/core/lib/Controller/Exception.php b/modules/core/lib/Controller/Exception.php index 1bb7834e7..905f8c116 100644 --- a/modules/core/lib/Controller/Exception.php +++ b/modules/core/lib/Controller/Exception.php @@ -82,6 +82,14 @@ class Exception ) . "&logout"; } + $t = new Template($this->config, 'core:cardinality_error.twig'); + $t->data['cardinalityErrorAttributes'] = $state['core:cardinality:errorAttributes']; + if (isset($state['Source']['auth'])) { + $t->data['LogoutURL'] = Module::getModuleURL( + 'core/login/' . urlencode($state['Source']['auth']) + ); + } + $t->setStatusCode(403); return $t; } @@ -101,7 +109,21 @@ class Exception $retryURL = Utils\HTTP::checkURLAllowed(strval($retryURL)); } - $t = new Template($this->config, 'core:no_cookie.tpl.php'); + $t = new Template($this->config, 'core:no_cookie.twig'); + $translator = $t->getTranslator(); + + /** @var string $header */ + $header = $translator->t('{core:no_cookie:header}'); + + /** @var string $desc */ + $desc = $translator->t('{core:no_cookie:description}'); + + /** @var string $retry */ + $retry = $translator->t('{core:no_cookie:retry}'); + + $t->data['header'] = htmlspecialchars($header); + $t->data['description'] = htmlspecialchars($desc); + $t->data['retry'] = htmlspecialchars($retry); $t->data['retryURL'] = $retryURL; return $t; } @@ -134,7 +156,7 @@ class Exception Auth\ProcessingChain::resumeProcessing($state); } - $t = new Template($this->config, 'core:short_sso_interval.tpl.php'); + $t = new Template($this->config, 'core:short_sso_interval.twig'); $translator = $t->getTranslator(); $t->data['params'] = ['StateId' => $stateId]; $t->data['trackId'] = $this->session->getTrackID(); diff --git a/modules/core/lib/Controller/Login.php b/modules/core/lib/Controller/Login.php index b6b5228ab..cc1c16af6 100644 --- a/modules/core/lib/Controller/Login.php +++ b/modules/core/lib/Controller/Login.php @@ -116,7 +116,7 @@ class Login * @param Request $request The request that lead to this login operation. * @param string|null $as The name of the authentication source to use, if any. Optional. * - * @return \SimpleSAML\XHTML\Template|\SimpleSAML\HTTP\RunnableResponse|\Symfony\Component\HttpFoundation\RedirectResponse + * @return \Symfony\Component\HttpFoundation\Response * An HTML template, a redirect or a "runnable" response. * * @throws \SimpleSAML\Error\Exception @@ -186,7 +186,10 @@ class Login public function logout(string $as): Response { $auth = new Auth\Simple($as); - return new RunnableResponse([$auth, 'logout'], [$this->config->getBasePath() . 'core/logout/' . urlencode($as)]); + return new RunnableResponse( + [$auth, 'logout'], + [$this->config->getBasePath() . 'core/logout/' . urlencode($as)] + ); } diff --git a/modules/core/routing/routes/routes.yml b/modules/core/routing/routes/routes.yml new file mode 100644 index 000000000..12884b024 --- /dev/null +++ b/modules/core/routing/routes/routes.yml @@ -0,0 +1,24 @@ +core-account: + path: /account/{as} + defaults: { _controller: 'SimpleSAML\Module\core\Controller\Login:account' } +core-account-disco-clearchoices: + path: /account/disco/clearchoices + defaults: { _controller: 'SimpleSAML\Module\core\Controller\Login:cleardiscochoices' } +core-login: + path: /login/{as} + defaults: { _controller: 'SimpleSAML\Module\core\Controller\Login:login', as: null } +core-logout: + path: /logout/{as} + defaults: { _controller: 'SimpleSAML\Module\core\Controller\Login:logout' } +core-error-nocookie: + path: /error/nocookie + defaults: { _controller: 'SimpleSAML\Module\core\Controller\Exception:nocookie' } +core-cardinality: + path: /error/cardinality + defaults: { _controller: 'SimpleSAML\Module\core\Controller\Exception:cardinality' } +core-warning-shortssointerval: + path: /warning/short_sso_interval + defaults: { _controller: 'SimpleSAML\Module\core\Controller\Exception:shortSsoInterval' } +core-post-redirect: + path: /postredirect + defaults: { _controller: 'SimpleSAML\Module\core\Controller\Redirection:postredirect' } diff --git a/psalm.xml b/psalm.xml index 98c6fd8fa..ff74be3ca 100644 --- a/psalm.xml +++ b/psalm.xml @@ -67,13 +67,6 @@ <DocblockTypeContradiction errorLevel="suppress" /> <RedundantConditionGivenDocblockType errorLevel="suppress" /> - <!-- See #1141 --> - <MissingDependency> - <errorLevel type="suppress"> - <file name="lib/SimpleSAML/HTTP/Router.php" /> - </errorLevel> - </MissingDependency> - <!-- Ignore UnresolvableInclude on CLI-scripts --> <UnresolvableInclude> <errorLevel type="suppress"> diff --git a/routing/routes/routes.yml b/routing/routes/routes.yml new file mode 100644 index 000000000..950cce0bb --- /dev/null +++ b/routing/routes/routes.yml @@ -0,0 +1,13 @@ +remove_trailing_slash: + path: /{module}/{url} + defaults: { _controller: SimpleSAML\Module::removeTrailingSlash } + requirements: + url: ".*/$" + methods: [GET] + +add_trailing_slash: + path: /{url} + defaults: { _controller: SimpleSAML\Module::addTrailingSlash } + requirements: + url: "[a-zA-Z0-9_-]+[^/]$" + methods: [GET] diff --git a/routing/services/console.yml b/routing/services/console.yml new file mode 100644 index 000000000..13df31237 --- /dev/null +++ b/routing/services/console.yml @@ -0,0 +1,4 @@ +services: + # default configuration for services in *this* file + _defaults: + public: false diff --git a/routing/services/routing.yml b/routing/services/routing.yml new file mode 100644 index 000000000..eaec9140b --- /dev/null +++ b/routing/services/routing.yml @@ -0,0 +1,72 @@ +services: + # default configuration for services in *this* file + _defaults: + public: false + + routing.resolver: + class: Symfony\Component\Config\Loader\LoaderResolver + + routing.loader.xml: + class: Symfony\Component\Routing\Loader\XmlFileLoader + arguments: + - '@file_locator' + tags: ['routing.loader'] + + routing.loader.yaml: + class: Symfony\Component\Routing\Loader\YamlFileLoader + arguments: + - '@file_locator' + tags: ['routing.loader'] + + routing.loader.php: + class: Symfony\Component\Routing\Loader\PhpFileLoader + arguments: + - '@file_locator' + tags: ['routing.loader'] + + routing.loader.glob: + class: Symfony\Component\Routing\Loader\GlobFileLoader + arguments: + - '@file_locator' + tags: ['routing.loader'] + + routing.loader.directory: + class: Symfony\Component\Routing\Loader\DirectoryLoader + arguments: + - '@file_locator' + tags: ['routing.loader'] + + routing.loader.service: + class: Symfony\Component\Routing\Loader\DependencyInjection\ServiceRouterLoader + arguments: + - '@service_container' + tags: ['routing.loader'] + + routing.loader: + public: true + class: Symfony\Component\Config\Loader\DelegatingLoader + arguments: + - '@routing.resolver' + - {} + + router.default: + class: Symfony\Component\Routing\Router + arguments: + - '@routing.loader' + - 'kernel:loadRoutes' + - { resource_type: 'service' } + - '@?router.request_context' + + router: + alias: router.default + public: true + + router.request_context: + class: Symfony\Component\Routing\RequestContext + + router.router_listener: + class: Symfony\Component\HttpKernel\EventListener\RouterListener + arguments: + - '@router' + - '@request_stack' + - '@?router.request_context' diff --git a/routing/services/services.yml b/routing/services/services.yml new file mode 100644 index 000000000..bbf898976 --- /dev/null +++ b/routing/services/services.yml @@ -0,0 +1,33 @@ +services: + # default configuration for services in *this* file + _defaults: + public: false + + request_stack: + class: Symfony\Component\HttpFoundation\RequestStack + public: true + + Symfony\Component\HttpFoundation\RequestStack: + alias: request_stack + + file_locator: + class: Symfony\Component\Config\FileLocator + + dispatcher: + class: Symfony\Component\EventDispatcher\EventDispatcher + calls: + - method: addSubscriber + arguments: + - '@router_listener' + - method: addSubscriber + arguments: + - '@response_listener' + + http_kernel: + class: Symfony\Component\HttpKernel\HttpKernel + public: true + arguments: + - '@dispatcher' + - '@controller_resolver' + - '@request_stack' + - '@argument_resolver' diff --git a/routing/services/simplesaml.yml b/routing/services/simplesaml.yml new file mode 100644 index 000000000..8a0c7d2b7 --- /dev/null +++ b/routing/services/simplesaml.yml @@ -0,0 +1,16 @@ +services: + # default configuration for services in *this* file + _defaults: + public: false + + SimpleSAML\Configuration: + factory: ['SimpleSAML\Configuration', 'getInstance'] + + SimpleSAML\Session: + factory: ['SimpleSAML\Session', 'getSessionFromRequest'] + + SimpleSAML\Auth\AuthenticationFactory: + class: SimpleSAML\Auth\AuthenticationFactory + arguments: + - '@SimpleSAML\Configuration' + - '@SimpleSAML\Session' diff --git a/routing/services/web.yml b/routing/services/web.yml new file mode 100644 index 000000000..dcca7e4c3 --- /dev/null +++ b/routing/services/web.yml @@ -0,0 +1,22 @@ +services: + # default configuration for services in *this* file + _defaults: + public: false + + controller_resolver: + class: Symfony\Component\HttpKernel\Controller\ContainerControllerResolver + arguments: + - '@service_container' + + argument_metadata_factory: + class: Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadataFactory + + argument_resolver: + class: Symfony\Component\HttpKernel\Controller\ArgumentResolver + arguments: + - '@argument_metadata_factory' + - '' + response_listener: + class: Symfony\Component\HttpKernel\EventListener\ResponseListener + arguments: + - 'UTF-8' diff --git a/tests/modules/core/lib/Controller/LoginControllerTest.php b/tests/modules/core/lib/Controller/LoginTest.php similarity index 100% rename from tests/modules/core/lib/Controller/LoginControllerTest.php rename to tests/modules/core/lib/Controller/LoginTest.php -- GitLab