Merge 3e36641 into fd19d5d

simplesamlphp · Mar 8, 2024 · 37b7f58 · 37b7f58
2 parents fd19d5d + 3e36641
commit 37b7f58
Show file tree

Hide file tree

Showing 19 changed files with 1,956 additions and 110 deletions.
diff --git a/modules/admin/src/Controller/Config.php b/modules/admin/src/Controller/Config.php
@@ -34,15 +34,11 @@ class Config
 
     public const RELEASES_API = 'https://api.github.com/repos/simplesamlphp/simplesamlphp/releases/latest';
 
-    /** @var \SimpleSAML\Utils\Auth */
-    protected Utils\Auth $authUtils;
-
-    /** @var \SimpleSAML\Utils\HTTP */
-    protected Utils\HTTP $httpUtils;
-
     /** @var \SimpleSAML\Module\admin\Controller\Menu */
     protected Menu $menu;
 
+    protected Utils $utils;
+
 
     /**
      * ConfigController constructor.
@@ -52,22 +48,12 @@ class Config
      */
     public function __construct(
         protected Configuration $config,
-        protected Session $session
+        protected Session $session,
+        Utils $utils = null,
+        Menu $menu = null
     ) {
-        $this->menu = new Menu();
-        $this->authUtils = new Utils\Auth();
-        $this->httpUtils = new Utils\HTTP();
-    }
-
-
-    /**
-     * Inject the \SimpleSAML\Utils\Auth dependency.
-     *
-     * @param \SimpleSAML\Utils\Auth $authUtils
-     */
-    public function setAuthUtils(Utils\Auth $authUtils): void
-    {
-        $this->authUtils = $authUtils;
+        $this->utils = $utils ?? new Utils($this->config, $this->session);
+        $this->menu = $menu ?? new Menu();
     }
 
 
@@ -80,26 +66,27 @@ public function setAuthUtils(Utils\Auth $authUtils): void
      */
     public function diagnostics(Request $request): Response
     {
-        $response = $this->authUtils->requireAdmin();
+        $response = $this->utils->auth()->requireAdmin();
         if ($response instanceof Response) {
             return $response;
         }
 
+        // TODO Consider moving to Template factory.
         $t = new Template($this->config, 'admin:diagnostics.twig');
         $t->data = [
             'remaining' => $this->session->getAuthData('admin', 'Expire') - time(),
-            'logouturl' => $this->authUtils->getAdminLogoutURL(),
+            'logouturl' => $this->utils->auth()->getAdminLogoutURL(),
             'items' => [
                 'HTTP_HOST' => [$request->getHost()],
                 'HTTPS' => $request->isSecure() ? ['on'] : [],
                 'SERVER_PROTOCOL' => [$request->getProtocolVersion()],
-                'getBaseURL()' => [$this->httpUtils->getBaseURL()],
-                'getSelfHost()' => [$this->httpUtils->getSelfHost()],
-                'getSelfHostWithNonStandardPort()' => [$this->httpUtils->getSelfHostWithNonStandardPort()],
-                'getSelfURLHost()' => [$this->httpUtils->getSelfURLHost()],
-                'getSelfURLNoQuery()' => [$this->httpUtils->getSelfURLNoQuery()],
-                'getSelfHostWithPath()' => [$this->httpUtils->getSelfHostWithPath()],
-                'getSelfURL()' => [$this->httpUtils->getSelfURL()],
+                'getBaseURL()' => [$this->utils->http()->getBaseURL()],
+                'getSelfHost()' => [$this->utils->http()->getSelfHost()],
+                'getSelfHostWithNonStandardPort()' => [$this->utils->http()->getSelfHostWithNonStandardPort()],
+                'getSelfURLHost()' => [$this->utils->http()->getSelfURLHost()],
+                'getSelfURLNoQuery()' => [$this->utils->http()->getSelfURLNoQuery()],
+                'getSelfHostWithPath()' => [$this->utils->http()->getSelfHostWithPath()],
+                'getSelfURL()' => [$this->utils->http()->getSelfURL()],
             ],
         ];
 
@@ -117,7 +104,7 @@ public function diagnostics(Request $request): Response
      */
     public function main(/** @scrutinizer ignore-unused */ Request $request): Response
     {
-        $response = $this->authUtils->requireAdmin();
+        $response = $this->utils->auth()->requireAdmin();
         if ($response instanceof Response) {
             return $response;
         }
@@ -141,12 +128,12 @@ public function main(/** @scrutinizer ignore-unused */ Request $request): Respon
                 'saml20idp' => $this->config->getOptionalBoolean('enable.saml20-idp', false),
             ],
             'funcmatrix' => $this->getPrerequisiteChecks(),
-            'logouturl' => $this->authUtils->getAdminLogoutURL(),
+            'logouturl' => $this->utils->auth()->getAdminLogoutURL(),
             'modulelist' => $this->getModuleList(),
         ];
 
         Module::callHooks('configpage', $t);
-        $this->menu->addOption('logout', $this->authUtils->getAdminLogoutURL(), Translate::noop('Log out'));
+        $this->menu->addOption('logout', $this->utils->auth()->getAdminLogoutURL(), Translate::noop('Log out'));
         return $this->menu->insert($t);
     }
 
@@ -175,7 +162,7 @@ protected function getModuleList(): array
      */
     public function phpinfo(/** @scrutinizer ignore-unused */ Request $request): Response
     {
-        $response = $this->authUtils->requireAdmin();
+        $response = $this->utils->auth()->requireAdmin();
         if ($response instanceof Response) {
             return $response;
         }
@@ -402,7 +389,7 @@ protected function getWarnings(): array
         $warnings = [];
 
         // make sure we're using HTTPS
-        if (!$this->httpUtils->isHTTPS()) {
+        if (!$this->utils->http()->isHTTPS()) {
             $warnings[] = Translate::noop(
                 '<strong>You are not using HTTPS</strong> to protect communications with your users. HTTP works fine ' .
                 'for testing purposes, but in a production environment you should use HTTPS. <a ' .

diff --git a/modules/admin/src/Controller/Menu.php b/modules/admin/src/Controller/Menu.php
@@ -7,6 +7,7 @@
 use SimpleSAML\Assert\Assert;
 use SimpleSAML\Locale\Translate;
 use SimpleSAML\Module;
+use SimpleSAML\Utils;
 use SimpleSAML\XHTML\Template;
 
 /**
@@ -18,17 +19,20 @@ final class Menu
 {
     /** @var array */
     private array $options;
-
+    private Module $module;
 
     /**
      * Menu constructor.
      *
      * Initialize the menu with some default admin options, and call a hook for anyone willing to extend it.
      */
-    public function __construct()
+    public function __construct(Module $module = null)
     {
+        $this->module = $module ?? new Module();
+
         $this->options = [
             'main' => [
+                // TODO mivanci Get module from utils
                 'url' => Module::getModuleURL('admin/'),
                 'name' => Translate::noop('Configuration'),
             ],

diff --git a/phpunit.xml b/phpunit.xml
@@ -12,6 +12,9 @@
     </testsuite>
   </testsuites>
   <logging/>
+  <php>
+    <env name="REDIRECT_SIMPLESAMLPHP_CONFIG_DIR" value="tests/config" force="true"/>
+  </php>
   <source>
     <include>
       <directory suffix=".php">./modules</directory>

diff --git a/psalm-dev.xml b/psalm-dev.xml
@@ -52,12 +52,19 @@
                 <directory name="tests" />
             </errorLevel>
         </UnsafeInstantiation>
+
+        <InvalidArgument>
+            <!-- Suppress InvalidArgument in tests, since we are to use a lot of mocks. -->
+            <errorLevel type="suppress">
+                <directory name="tests" />
+            </errorLevel>
+        </InvalidArgument>
     </issueHandlers>
 
-    <stubs>
-        <file name="vendor/simplesamlphp/simplesamlphp-test-framework/stubs/krb5.php" />
-        <file name="vendor/simplesamlphp/simplesamlphp-test-framework/stubs/memcache.php" />
-        <file name="vendor/simplesamlphp/simplesamlphp-test-framework/stubs/memcached.php" />
-        <file name="vendor/simplesamlphp/simplesamlphp-test-framework/stubs/predis.php" />
-    </stubs>
-</psalm>
+        <stubs>
+            <file name="vendor/simplesamlphp/simplesamlphp-test-framework/stubs/krb5.php" />
+            <file name="vendor/simplesamlphp/simplesamlphp-test-framework/stubs/memcache.php" />
+            <file name="vendor/simplesamlphp/simplesamlphp-test-framework/stubs/memcached.php" />
+            <file name="vendor/simplesamlphp/simplesamlphp-test-framework/stubs/predis.php" />
+        </stubs>
+    </psalm>
diff --git a/routing/services/simplesamlphp.yml b/routing/services/simplesamlphp.yml
@@ -3,16 +3,31 @@
 services:
   # default configuration for services in *this* file
   _defaults:
+    autowire: true
+    autoconfigure: true
     public: false
+    bind:
+      Psr\Log\LoggerInterface: '@SimpleSAML\Compat\Logger'
 
+  # Main SSP services
   SimpleSAML\Configuration:
     factory: ['SimpleSAML\Configuration', 'getInstance']
 
   SimpleSAML\Session:
     factory: ['SimpleSAML\Session', 'getSessionFromRequest']
 
-  SimpleSAML\Auth\AuthenticationFactory:
-    class: SimpleSAML\Auth\AuthenticationFactory
+  ssp.auth_sources_config:
+    class: SimpleSAML\Configuration
+    factory: ['SimpleSAML\Configuration', 'getConfig']
     arguments:
-      - '@SimpleSAML\Configuration'
-      - '@SimpleSAML\Session'
+      - 'authsources.php'
+
+  SimpleSAML\Utils:
+    class: SimpleSAML\Utils
+    arguments:
+      $authSourcesConfig: '@ssp.auth_sources_config'
+
+  # Common SSP services
+  SimpleSAML\Auth\AuthenticationFactory: ~
+  SimpleSAML\Compat\Logger: ~
+  SimpleSAML\Module: ~
diff --git a/src/SimpleSAML/Auth/Simple.php b/src/SimpleSAML/Auth/Simple.php
@@ -7,6 +7,7 @@
 use SimpleSAML\{Configuration, Error, Module, Session, Utils};
 use SimpleSAML\Assert\Assert;
 use Symfony\Component\HttpFoundation\Response;
+use Throwable;
 
 use function array_key_exists;
 use function call_user_func;
@@ -24,34 +25,29 @@
 
 class Simple
 {
-    /** @var \SimpleSAML\Configuration */
     protected Configuration $app_config;
 
-    /** @var \SimpleSAML\Session */
     protected Session $session;
+    protected Utils $utils;
 
 
     /**
      * Create an instance with the specified authsource.
      *
      * @param string $authSource The id of the authentication source.
-     * @param \SimpleSAML\Configuration|null $config Optional configuration to use.
-     * @param \SimpleSAML\Session|null $session Optional session to use.
+     * @param Configuration|null $config Optional configuration to use.
+     * @param Session|null $session Optional session to use.
+     * @throws \Exception
      */
     public function __construct(
         protected string $authSource,
         Configuration $config = null,
-        Session $session = null
+        Session $session = null,
+        Utils $utils = null
     ) {
-        if ($config === null) {
-            $config = Configuration::getInstance();
-        }
-        $this->app_config = $config->getOptionalConfigItem('application', []);
-
-        if ($session === null) {
-            $session = Session::getSessionFromRequest();
-        }
-        $this->session = $session;
+        $this->app_config = $config ?? (Configuration::getInstance())->getOptionalConfigItem('application', []);
+        $this->session = $session ?? Session::getSessionFromRequest();
+        $this->utils = $utils ?? new Utils();
     }
 
 
@@ -64,7 +60,12 @@ public function __construct(
      */
     public function getAuthSource(): Source
     {
-        $as = Source::getById($this->authSource);
+        try {
+            $as = $this->utils->authSource()->getById($this->authSource);
+        } catch (Throwable $exception) {
+            throw new Error\AuthSource($this->authSource, 'Error getting authentication source.', $exception);
+        }
+
         if ($as === null) {
             throw new Error\AuthSource($this->authSource, 'Unknown authentication source.');
         }

diff --git a/src/SimpleSAML/Auth/Source.php b/src/SimpleSAML/Auth/Source.php
@@ -346,6 +346,10 @@ private static function parseAuthSource(string $authId, array $config): Source
      * @return \SimpleSAML\Auth\Source|null The AuthSource object, or NULL if no authentication
      *     source with the given identifier is found.
      * @throws \SimpleSAML\Error\Exception If no such authentication source is found or it is invalid.
+     *
+     * @deprecated Static method access is deprecated. Move to instance method.
+     * @see Utils\AuthSource::getById()
+     * TODO NextMajorRelease Move content to substitute method and remove.
      */
     public static function getById(string $authId, ?string $type = null): ?Source
     {