chore: merge branch 'main-patch-6aa1' into 'main'

fix: correct no tokens redirect

See merge request perun-proxy-aai/simplesamlphp/simplesamlphp-module-authswitcher!55

lib/Auth/Process/SwitchAuth.php

@@ -24,7 +24,7 @@ class SwitchAuth extends \SimpleSAML\Auth\ProcessingFilter
 
     private const SETUP_MFA_URL = 'authswitcher/setupMFA.php';
 
-    public const SETUP_MFA_TPL_URL = 'authswitcher/setup-mfa-tpl.php';
+    public const SETUP_MFA_TPL_URL = 'authswitcher:setup-mfa-tpl.php';
 
     public const PARAM_MFA_REDIRECT_URL = 'mfa_redirect_url';
 
@@ -134,41 +134,33 @@ class SwitchAuth extends \SimpleSAML\Auth\ProcessingFilter
 
         $usersCapabilities = $this->getMFAForUid($state);
 
-        if (
-            $mfaEnforced && empty($state['Attributes'][AuthSwitcher::MFA_TOKENS]) &&
-            !in_array($this->entityID, $this->mfa_excluded_sps) && !empty($this->setup_mfa_redirect_url)
-        ) {
-            $url = Module::getModuleURL(self::SETUP_MFA_URL);
-            $params = [];
-            $params[self::PARAM_MFA_REDIRECT_URL] = $this->setup_mfa_redirect_url;
-            HTTP::redirectTrustedURL($url, $params);
-        }
+        $upstreamContext = $this->proxyMode ? ProxyHelper::fetchContextFromUpstreamIdp($state) : null;
 
         self::info('user capabilities: ' . json_encode($usersCapabilities));
 
         self::setErrorHandling($state);
 
         if ($this->proxyMode) {
-            $upstreamContext = ProxyHelper::fetchContextFromUpstreamIdp($state);
             self::info('upstream context: ' . $upstreamContext);
             ProxyHelper::recoverSPRequestedContexts($state);
-        } else {
-            $upstreamContext = null;
         }
 
         $this->supported_requested_contexts = $this->authnContextHelper->getSupportedRequestedContexts(
             $usersCapabilities,
             $state,
             $upstreamContext,
-            !$this->check_entropy || $this->checkSfaEntropy($state['Attributes']),
-            $mfaEnforced
+            !$this->check_entropy || $this->checkSfaEntropy($state['Attributes'])
         );
 
         self::info('supported requested contexts: ' . json_encode($this->supported_requested_contexts));
 
         $shouldPerformMFA = !$this->authnContextHelper->MFAin([
                 $upstreamContext,
-            ]) && ($mfaEnforced || $this->authnContextHelper->isMFAprefered($this->supported_requested_contexts));
+            ]) && (
+                $mfaEnforced
+                || empty($this->supported_requested_contexts)
+                || $this->authnContextHelper->isMFAprefered($this->supported_requested_contexts)
+            );
 
         if (
             $this->mfa_preferred_privacyidea_fail
@@ -178,6 +170,26 @@ class SwitchAuth extends \SimpleSAML\Auth\ProcessingFilter
             throw new Exception(self::DEBUG_PREFIX . 'MFA should be performed but connection to privacyidea failed.');
         }
 
+        if (
+            $shouldPerformMFA
+            && empty($state['Attributes'][AuthSwitcher::MFA_TOKENS])
+            && !empty($this->setup_mfa_redirect_url)
+            && !in_array($this->entityID, $this->mfa_excluded_sps)
+        ) {
+            self::info('user must perform MFA but has no tokens, redirect to setup');
+            $url = Module::getModuleURL(self::SETUP_MFA_URL);
+            $state[self::PARAM_MFA_REDIRECT_URL] = $this->setup_mfa_redirect_url;
+            $stateId = State::saveState($state, 'authswitcher:authswitcher');
+            HTTP::redirectTrustedURL($url, ['stateId' => $stateId]);
+            exit;
+        }
+        if (empty($this->supported_requested_contexts)) {
+            Logger::info(
+                'authswitcher: no requested AuthnContext can be fulfilled: ' . json_encode($requestedContexts)
+            );
+            self::noAuthnContextResponder($state);
+        }
+
         // switch to MFA if enforced or preferred but not already done if we handle the proxy mode
         $performMFA = $this->authnContextHelper->MFAin($usersCapabilities) && $shouldPerformMFA;
 

lib/AuthnContextHelper.php

@@ -46,8 +46,7 @@ class AuthnContextHelper
         $usersCapabilities,
         $state,
         $upstreamContext,
-        $sfaEntropy,
-        $mfaEnforced = false
+        $sfaEntropy
     ) {
         $requestedContexts = $state['saml:RequestedAuthnContext']['AuthnContextClassRef'] ?? null;
         if (empty($requestedContexts)) {
@@ -67,31 +66,30 @@ class AuthnContextHelper
         }
 
         if (
-            !empty($requestedContexts) // sp has requested something
-            && empty($supportedRequestedContexts) // nothing of that is supported by authswitcher
-            && empty($upstreamContext) // it was neither filled from upstream IdP
+            // no requested context is supported by authswitcher (or this module is misconfigured)
+            empty($supportedRequestedContexts)
+            && empty($upstreamContext) // nor it was filled from upstream IdP
         ) {
             Logger::info('authswitcher: no requested AuthnContext is supported: ' . json_encode($requestedContexts));
             self::noAuthnContextRequester($state);
         }
 
         // check for unsatisfiable combinations
-        if (
-            !$this->testComparison(
-                $usersCapabilities,
-                $supportedRequestedContexts,
-                $state['saml:RequestedAuthnContext']['Comparison'] ?? Constants::COMPARISON_EXACT,
-                $upstreamContext,
-                $mfaEnforced
-            )
-        ) {
-            Logger::info(
-                'authswitcher: no requested AuthnContext can be fulfilled: ' . json_encode($requestedContexts)
-            );
-            self::noAuthnContextResponder($state);
+        $requestResult = $this->testComparison(
+            $usersCapabilities,
+            $supportedRequestedContexts,
+            $state['saml:RequestedAuthnContext']['Comparison'] ?? Constants::COMPARISON_EXACT,
+            $upstreamContext,
+            $mfaEnforced
+        );
+        if ($requestResult === null) {
+            Logger::info('authswitcher: comparsion type maxium is not supported');
+            self::noAuthnContextRequester($state);
+        } elseif ($requestResult === false) {
+            return [];
+        } else {
+            return $supportedRequestedContexts;
         }
-
-        return $supportedRequestedContexts;
     }
 
     public static function noAuthnContextResponder($state)
@@ -127,6 +125,9 @@ class AuthnContextHelper
     /**
      * If the Comparison attribute is set to “better”, “minimum”, or “maximum”, the method of authentication
      * must be stronger than, at least as strong as, or no stronger than one of the specified authentication classes.
+     * It is assumed that MFA always includes a password.
+     * Returns null for unsupported maximum comparison, false if MFA should be performed but user has no tokens,
+     * true otherwise.
      *
      * @param mixed      $usersCapabilities
      * @param mixed      $supportedRequestedContexts
@@ -138,8 +139,7 @@ class AuthnContextHelper
         $usersCapabilities,
         $supportedRequestedContexts,
         $comparison,
-        $upstreamContext = null,
-        $mfaEnforced = false
+        $upstreamContext = null
     ) {
         $upstreamMFA = $upstreamContext === null ? false : $this->MFAin([$upstreamContext]);
         $upstreamSFA = $upstreamContext === null ? false : $this->SFAin([$upstreamContext]);
@@ -162,9 +162,7 @@ class AuthnContextHelper
                 }
                 break;
             case Constants::COMPARISON_MAXIMUM:
-                if ($mfaEnforced && $requestedSFA) {
-                    return false;
-                }
+                return null;
                 break;
             case Constants::COMPARISON_EXACT:
             default:

www/setupMFA.php

@@ -2,12 +2,19 @@
 
 declare(strict_types=1);
 
+use SimpleSAML\Auth\State;
 use SimpleSAML\Configuration;
 use SimpleSAML\Module\authswitcher\Auth\Process\SwitchAuth;
 use SimpleSAML\XHTML\Template;
 
 $config = Configuration::getInstance();
 $t = new Template($config, SwitchAuth::SETUP_MFA_TPL_URL);
-$t->data[SwitchAuth::PARAM_MFA_REDIRECT_URL] = $_REQUEST[SwitchAuth::PARAM_MFA_REDIRECT_URL];
+
+if (empty($_GET['stateId'])) {
+    exit;
+}
+
+$state = State::loadState($_GET['stateId'], 'authswitcher:authswitcher', true);
+$t->data[SwitchAuth::PARAM_MFA_REDIRECT_URL] = $state[SwitchAuth::PARAM_MFA_REDIRECT_URL];
 
 $t->show();