SSP-2030_OIDC_module_switch_to_ProcessingChain_for_authproc_support_c…

…onsent_mod (#228)

* Switch to ProcessingChain for authproc filters

* Add more tests.

* Fix psalm errors

* Add some manual testing tips for authproc testing; run an authproc as part of conformance tests

* Update documentation

---------

Co-authored-by: Marko Ivančić <[email protected]>
Co-authored-by: Patrick <[email protected]>

README.md

@@ -223,8 +223,8 @@ In addition to that, the following OIDC related data will be available in the st
 * \['Oidc'\]\['RelyingPartyMetadata'\] - contains information about the OIDC client making the authN request.
 * \['Oidc'\]\['AuthorizationRequestParameters'\] - contains relevant authorization request query parameters.
 
-Note: at the moment there is no support for showing a page to the user in a filter, and then resuming the filtering.
-Only the common filter use cases are supported like attribute handling, logging, or similar. 
+Auth Proc processing has been tested with a variety of modules including ones that adjust attributes, log
+and redirect for user interaction.
 
 You can add Auth Proc filters in the 'authproc.oidc' config option in the same manner as described in the [Auth Proc 
 documentation](https://simplesamlphp.org/docs/stable/simplesamlphp-authproc).
@@ -316,6 +316,16 @@ and you can add a client.
 
 You may view the OIDC configuration endpoint at `https://localhost/.well-known/openid-configuration`
 
+#### Testing AuthProc filters
+
+To perform manual testing of authproc filters, enable the authprocs in `module_oidc.php` that set firstname, sn and performs
+a redirect for preprod warning. This setup shows that an authproc can do a redirect and then processing resumes.
+Once adjusted, run docker while change the `COMPOSER_REQUIRE` line to
+
+    `-e COMPOSER_REQUIRE="simplesamlphp/simplesamlphp-module-oidc:@dev simplesamlphp/simplesamlphp-module-preprodwarning" \`
+
+You can register a client from https://oidcdebugger.com/ to test.
+
 ### Build Image to Deploy for Conformance Tests
 
 Build an image that contains a pre-configured sqlite database.

UPGRADE.md

@@ -4,7 +4,6 @@
 - token introspection
 - implement store for different entities?: i.e. client data can use RDB like mysql, whilst short term data
   like tokens can utilize faster stores like memcache, redis...
-- move to SimpleSAMLphp ProcessingChain
 - move checkers to templates (generics) for proper static type handling
 - remove dependency on laminas/laminas-httphandlerrunner
 - create a bridge towards SSP utility classes, so they can be easily mocked
@@ -21,6 +20,11 @@
   - Clients can now be configured with new properties:
     - Entity Identifier
     - Registration Types
+- Improved AuthProc filter support
+  - Support authproc filters that need to redirect and later resume processing
+    - `consent` and `preprodwarning` are two authprocs that redirect for user interaction and are now supported
+  - Uses SSP's ProcessingChain class for closer alignment with SAML IdP configuration.
+    - Allows additional configuration of authprocs in the main `config.php` under key `authproc.oidc`
 
 ## New configuration options
 
@@ -58,6 +62,7 @@ removed in version 7.
 Apache to preserve Authorization HTTP headers with Bearer token scheme (stripping of this header in Apache is a
 known 'issue': https://github.com/symfony/symfony/issues/19693). If you don't set this config, you'll now get warnings
 about this situation in your logs.
+- The new authproc filter processing will look in an additional location for filters, in the main `config.php` under key `authproc.oidc`
 
 ## Low impact changes
 

docker/ssp/config-override.php

@@ -4,6 +4,8 @@
 
 $config['module.enable']['exampleauth'] = true;
 $config['module.enable']['oidc'] = true;
+// Have preprod warning enabled (though it may not be installed) to ease authproc redirect testing
+$config['module.enable']['preprodwarning'] = true;
 $config = [
         'secretsalt' => 'testsalt',
         'database.dsn' => 'sqlite:/var/simplesamlphp/data/mydb.sq3',

docker/ssp/module_oidc.php

@@ -32,8 +32,31 @@
     ],
 
     ModuleConfig::OPTION_AUTH_PROCESSING_FILTERS => [
+        // For conformance tests we always have an authproc run just to confirm nothing is broken
+        // with the integration to ProcessingChain
+        5 => [
+            'class' => 'core:AttributeAdd',
+            'someUnusedAttribute' => 'Some value',
+        ]
     ],
 
+    // Use the below auth processing config to test authprocs with a redirect
+/*    ModuleConfig::OPTION_AUTH_PROCESSING_FILTERS => [
+        5 => [
+            'class' => 'core:AttributeAdd',
+            '%replace',
+            'givenName' => 'First AuthProc',
+        ],
+        10 => [
+            'class' => 'preprodwarning:Warning',
+        ],
+        15 => [
+            'class' => 'core:AttributeAdd',
+            '%replace',
+            'sn' => 'SN AuthProc',
+        ]
+    ],*/
+
     ModuleConfig::OPTION_AUTH_CUSTOM_SCOPES => [
     ],
     ModuleConfig::OPTION_AUTH_SAML_TO_OIDC_TRANSLATE_TABLE => [

src/Controller/AuthorizationController.php

@@ -19,6 +19,7 @@
 use League\OAuth2\Server\Exception\OAuthServerException;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
+use SimpleSAML\Auth\ProcessingChain;
 use SimpleSAML\Module\oidc\Bridges\PsrHttpBridge;
 use SimpleSAML\Module\oidc\ModuleConfig;
 use SimpleSAML\Module\oidc\Server\AuthorizationServer;
@@ -56,9 +57,19 @@ public function __construct(
      */
     public function __invoke(ServerRequestInterface $request): ResponseInterface
     {
-        $authorizationRequest = $this->authorizationServer->validateAuthorizationRequest($request);
+        $queryParameters = $request->getQueryParams();
+        $state = null;
 
-        $user = $this->authenticationService->getAuthenticateUser($request);
+        if (!isset($queryParameters[ProcessingChain::AUTHPARAM])) {
+            $authorizationRequest = $this->authorizationServer->validateAuthorizationRequest($request);
+            $state = $this->authenticationService->processRequest($request, $authorizationRequest);
+            // processState will trigger a redirect
+        }
+
+        $state ??= $this->authenticationService->manageState($queryParameters);
+        $authorizationRequest = $this->authenticationService->getAuthorizationRequestFromState($state);
+
+        $user = $this->authenticationService->getAuthenticateUser($state);
 
         $authorizationRequest->setUser($user);
         $authorizationRequest->setAuthorizationApproved(true);
@@ -77,6 +88,17 @@ public function __invoke(ServerRequestInterface $request): ResponseInterface
         );
     }
 
+    /**
+     * @param   Request  $request
+     *
+     * @return Response
+     * @throws \SimpleSAML\Error\AuthSource
+     * @throws \SimpleSAML\Error\BadRequest
+     * @throws \SimpleSAML\Error\Error
+     * @throws \SimpleSAML\Error\Exception
+     * @throws \SimpleSAML\Error\NotFound
+     * @throws \Throwable
+     */
     public function authorization(Request $request): Response
     {
         try {

src/Factories/ProcessingChainFactory.php

@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the simplesamlphp-module-oidc.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace SimpleSAML\Module\oidc\Factories;
+
+use SimpleSAML\Auth\ProcessingChain;
+use SimpleSAML\Module\oidc\ModuleConfig;
+
+class ProcessingChainFactory
+{
+    public function __construct(
+        private readonly ModuleConfig $moduleConfig,
+    ) {
+    }
+
+    /**
+     * @codeCoverageIgnore
+     * @throws \Exception
+     */
+    public function build(array $state): ProcessingChain
+    {
+        $idpMetadata = [
+            'entityid' => $state['Source']['entityid'] ?? '',
+            // ProcessChain needs to know the list of authproc filters we defined in module_oidc configuration
+            'authproc' => $this->moduleConfig->getAuthProcFilters(),
+        ];
+        $spMetadata = [
+            'entityid' => $state['Destination']['entityid'] ?? '',
+        ];
+
+        return new ProcessingChain(
+            $idpMetadata,
+            $spMetadata,
+            'oidc',
+        );
+    }
+}