diff --git a/lib/Auth/Process/SwitchAuth.php b/lib/Auth/Process/SwitchAuth.php index 73427a88d7d08c7cb3da2e5fa29b14097cf24371..07122ade8ff43d2c96b09db668adea92c0e45bd0 100644 --- a/lib/Auth/Process/SwitchAuth.php +++ b/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; diff --git a/lib/AuthnContextHelper.php b/lib/AuthnContextHelper.php index ab0cfdad049e55d608f790d9151e7827fe07a7d9..978466c033a0bc3805d27a4e9984d406cd08cadc 100644 --- a/lib/AuthnContextHelper.php +++ b/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: diff --git a/www/setupMFA.php b/www/setupMFA.php index 3c50ec01357d1575bb99314523d1fc826782bb3f..52e60e9d248d7379e22bd492e0daf43d7bbd62ec 100644 --- a/www/setupMFA.php +++ b/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();