Verified Commit 14990e5a authored by Pavel Břoušek's avatar Pavel Břoušek
Browse files

fix: correct no tokens redirect

* drop support for maximum comparison (does not make sense)
* handle no tokens redirect correctly for any comparison
* fix open redirect
* refactoring
parent 292f0c7a
Loading
Loading
Loading
Loading
Loading
+28 −16
Original line number Diff line number Diff line
@@ -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;

+23 −25
Original line number Diff line number Diff line
@@ -46,8 +46,7 @@ class AuthnContextHelper
        $usersCapabilities,
        $state,
        $upstreamContext,
        $sfaEntropy,
        $mfaEnforced = false
        $sfaEntropy
    ) {
        $requestedContexts = $state['saml:RequestedAuthnContext']['AuthnContextClassRef'] ?? null;
        if (empty($requestedContexts)) {
@@ -67,32 +66,31 @@ 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(
        $requestResult = $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);
        }

        if ($requestResult === null) {
            Logger::info('authswitcher: comparsion type maxium is not supported');
            self::noAuthnContextRequester($state);
        } elseif ($requestResult === false) {
            return [];
        } else {
            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:
+8 −1
Original line number Diff line number Diff line
@@ -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();