~fkooman/php-saml-sp

78601c94868fe4879ec9ecd39b747cf2d1b565b7 — Fran├žois Kooman 2 months ago 8c4cdf4
fix handling query parameters with internal discovery

References: https://codeberg.org/fkooman/php-saml-sp/issues/4
M src/Api/SamlAuth.php => src/Api/SamlAuth.php +1 -1
@@ 55,7 55,7 @@ class SamlAuth
    public function __construct()
    {
        $this->config = Config::fromFile(\dirname(__DIR__, 2) . '/config/config.php');
        $this->request = new Request($_SERVER, $_GET, $_POST);
        $this->request = new Request($_SERVER, $_POST);
        $this->session = new SeSession($this->config);
        $this->dateTime = new DateTimeImmutable();
    }

M src/LogoutResponse.php => src/LogoutResponse.php +9 -8
@@ 27,6 27,7 @@ declare(strict_types=1);
namespace fkooman\SAML\SP;

use fkooman\SAML\SP\Exception\ResponseException;
use fkooman\SAML\SP\Web\Request;

class LogoutResponse
{


@@ 36,12 37,12 @@ class LogoutResponse
     *
     * @throws \fkooman\SAML\SP\Exception\ResponseException
     */
    public function verify(QueryParameters $queryParameters, $expectedInResponseTo, $expectedSloUrl, IdpInfo $idpInfo): void
    public function verify(Request $httpRequest, $expectedInResponseTo, $expectedSloUrl, IdpInfo $idpInfo): void
    {
        $queryString = self::prepareQueryString($queryParameters);
        Crypto::verify($queryString, Base64::decode($queryParameters->requireQueryParameter('Signature')), $idpInfo->getPublicKeys(), $queryParameters->requireQueryParameter('SigAlg'));
        $queryString = self::prepareQueryString($httpRequest);
        Crypto::verify($queryString, Base64::decode($httpRequest->requireQueryParameter('Signature')), $idpInfo->getPublicKeys(), $httpRequest->requireQueryParameter('SigAlg'));

        if (false === $inflatedProtocolMessage = \gzinflate(Base64::decode($queryParameters->requireQueryParameter('SAMLResponse')))) {
        if (false === $inflatedProtocolMessage = \gzinflate(Base64::decode($httpRequest->requireQueryParameter('SAMLResponse')))) {
            throw new ResponseException('unable to "inflate" SAMLResponse');
        }



@@ 72,11 73,11 @@ class LogoutResponse
    /**
     * @return string
     */
    private static function prepareQueryString(QueryParameters $queryParameters)
    private static function prepareQueryString(Request $httpRequest)
    {
        $samlResponse = $queryParameters->requireQueryParameter('SAMLResponse', true);
        $relayState = $queryParameters->optionalQueryParameter('RelayState', true);
        $sigAlg = $queryParameters->requireQueryParameter('SigAlg', true);
        $samlResponse = $httpRequest->requireRawQueryParameter('SAMLResponse');
        $relayState = $httpRequest->optionalRawQueryParameter('RelayState');
        $sigAlg = $httpRequest->requireRawQueryParameter('SigAlg');
        if (null === $relayState) {
            return \sprintf('SAMLResponse=%s&SigAlg=%s', $samlResponse, $sigAlg);
        }

M src/QueryParameters.php => src/QueryParameters.php +5 -24
@@ 28,19 28,6 @@ namespace fkooman\SAML\SP;

use fkooman\SAML\SP\Exception\QueryParametersException;

/**
 * Ability to handle "raw" HTTP query string.
 *
 * We need to use the *exact* values as used in the HTTP query string for
 * HTTP-Redirect signature verification before they are URL-decoded by PHP.
 * Some implementations, e.g. Microsoft use upper-case URL encoding instead of
 * the more typical lower case encoding which exposed a bug in the code that
 * assumed everyone would be using lower-case encoding. The specification is
 * quite clear about how to implement it. Doesn't make it a good specification
 * though!
 *
 * @see saml-bindings-2.0-os (3.4.4.1 DEFLATE Encoding)
 */
class QueryParameters
{
    /** @var array<string,string> */


@@ 82,32 69,29 @@ class QueryParameters
                    continue;
                }
                [$k, $v] = $keyValue;
                $queryData[$k] = $v;
                $queryData[$k] = urldecode($v);
            }
        }

        return new self($queryData);
    }

    /**
     * @throws \fkooman\SAML\SP\Exception\QueryParametersException
     */
    public function requireQueryParameter(string $parameterName, bool $rawValue = false): string
    public function requireQueryParameter(string $parameterName): string
    {
        if (null === $parameterValue = $this->optionalQueryParameter($parameterName, $rawValue)) {
        if (null === $parameterValue = $this->optionalQueryParameter($parameterName)) {
            throw new QueryParametersException(\sprintf('query parameter "%s" missing', $parameterName));
        }

        return $parameterValue;
    }

    public function optionalQueryParameter(string $parameterName, bool $rawValue = false): ?string
    public function optionalQueryParameter(string $parameterName): ?string
    {
        if (!\array_key_exists($parameterName, $this->queryData)) {
            return null;
        }

        return $rawValue ? $this->queryData[$parameterName] : \urldecode($this->queryData[$parameterName]);
        return $this->queryData[$parameterName];
    }

    public function add(string $parameterName, string $parameterValue): void


@@ 115,9 99,6 @@ class QueryParameters
        $this->queryData[$parameterName] = $parameterValue;
    }

    /**
     * @throws \fkooman\SAML\SP\Exception\QueryParametersException
     */
    public function appendToUrl(string $inUrl): string
    {
        if (0 === count($this->queryData)) {

M src/SP.php => src/SP.php +4 -7
@@ 30,6 30,7 @@ use DateTimeImmutable;
use DateTimeZone;
use fkooman\SAML\SP\Exception\SpException;
use fkooman\SAML\SP\Log\LoggerInterface;
use fkooman\SAML\SP\Web\Request;

/**
 * The main controlling class for the SP.


@@ 219,17 220,13 @@ class SP
    }

    /**
     * @param string $queryString
     *
     * @throws \fkooman\SAML\SP\Exception\SpException
     *
     * @return string
     */
    public function handleLogoutResponse($queryString)
    public function handleLogoutResponse(Request $httpRequest)
    {
        $queryParameters = QueryParameters::fromQueryString($queryString);
        $relayState = $queryParameters->requireQueryParameter('RelayState');

        $relayState = $httpRequest->requireQueryParameter('RelayState');
        if (null === $sessionValue = $this->session->take(self::SESSION_KEY_PREFIX . $relayState)) {
            throw new SpException('"RelayState" not found in session data');
        }


@@ 246,7 243,7 @@ class SP

        $logoutResponse = new LogoutResponse();
        $logoutResponse->verify(
            $queryParameters,
            $httpRequest,
            $logoutRequestState->getRequestId(),
            $this->spInfo->getSloUrl(),
            $idpInfo

M src/Web/Request.php => src/Web/Request.php +60 -29
@@ 33,16 33,16 @@ class Request
    /** @var array */
    private $serverData;

    /** @var array */
    private $getData;
    /** @var array<string,string> */
    private array $queryData;

    /** @var array */
    private $postData;

    public function __construct(array $serverData, array $getData, array $postData)
    public function __construct(array $serverData, array $postData)
    {
        $this->serverData = $serverData;
        $this->getData = $getData;
        $this->queryData = self::parseQuery($serverData);
        $this->postData = $postData;
    }



@@ 117,49 117,47 @@ class Request
    }

    /**
     * Return the "raw" query string.
     * For SAML HTTP-Redirect binding signature verification we require the
     * "raw" query string.
     *
     * @return string
     * @see saml-bindings-2.0-os (3.4.4.1 DEFLATE Encoding)
     */
    public function getQueryString()
    public function requireRawQueryParameter(string $parameterName): string
    {
        if (null === $queryString = $this->optionalHeader('QUERY_STRING')) {
            return '';
        if (!array_key_exists($parameterName, $this->queryData)) {
            throw new HttpException(400, sprintf('missing query parameter "%s"', $parameterName));
        }

        return $queryString;
        return $this->queryData[$parameterName];
    }

    /**
     * @param string $queryKey
     * For SAML HTTP-Redirect binding signature verification we require the
     * "raw" query string.
     *
     * @return string
     * @see saml-bindings-2.0-os (3.4.4.1 DEFLATE Encoding)
     */
    public function requireQueryParameter($queryKey)
    public function optionalRawQueryParameter(string $parameterName): ?string
    {
        if (!\array_key_exists($queryKey, $this->getData)) {
            throw new HttpException(400, \sprintf('missing query parameter "%s"', $queryKey));
        }
        $queryValue = $this->getData[$queryKey];
        if (!\is_string($queryValue)) {
            throw new HttpException(400, \sprintf('value of query parameter "%s" MUST be string', $queryKey));
        if (!array_key_exists($parameterName, $this->queryData)) {
            return null;
        }

        return $queryValue;
        return $this->queryData[$parameterName];
    }

    /**
     * @param string $queryKey
     *
     * @return string|null
     */
    public function optionalQueryParameter($queryKey)
    public function requireQueryParameter(string $parameterName): string
    {
        if (!\array_key_exists($queryKey, $this->getData)) {
            return null;
        return urldecode($this->requireRawQueryParameter($parameterName));
    }

    public function optionalQueryParameter(string $parameterName): ?string
    {
        if (null !== $parameterValue = $this->optionalRawQueryParameter($parameterName)) {
            return urldecode($parameterValue);
        }

        return $this->requireQueryParameter($queryKey);
        return null;
    }

    /**


@@ 240,6 238,39 @@ class Request
    }

    /**
     * @return array<string,string>
     */
    private static function parseQuery(array $serverData): array
    {
        if (!array_key_exists('QUERY_STRING', $serverData)) {
            return [];
        }

        if (!is_string($serverData['QUERY_STRING'])) {
            return [];
        }

        $queryData = [];
        foreach (explode('&', $serverData['QUERY_STRING']) as $queryElement) {
            if ('' !== $queryElement) {
                if (false === strpos($queryElement, '=')) {
                    $queryData[$queryElement] = '';

                    continue;
                }
                $keyValue = explode('=', $queryElement, 2);
                if (2 !== count($keyValue)) {
                    continue;
                }
                [$k, $v] = $keyValue;
                $queryData[$k] = $v;
            }
        }

        return $queryData;
    }

    /**
     * @return string
     */
    private function getScheme()

M src/Web/Service.php => src/Web/Service.php +1 -1
@@ 239,7 239,7 @@ class Service
            case '/slo':
                // we need the "raw" query string to be able to verify the
                // signatures...
                $returnTo = $this->sp->handleLogoutResponse($request->getQueryString());
                $returnTo = $this->sp->handleLogoutResponse($request);

                return new RedirectResponse($returnTo);
            default:

M tests/QueryParametersTest.php => tests/QueryParametersTest.php +0 -7
@@ 42,13 42,6 @@ class QueryParametersTest extends TestCase
        $this->assertNull($qp->optionalQueryParameter('baz'));
    }

    public function testRaw(): void
    {
        $qp = QueryParameters::fromQueryString('foo=bar%2e&bar=foo');
        $this->assertSame('bar.', $qp->requireQueryParameter('foo'));
        $this->assertSame('bar%2e', $qp->requireQueryParameter('foo', true));
    }

    public function testDuplicate(): void
    {
        $qp = QueryParameters::fromQueryString('foo=bar&foo=baz');

M tests/SPTest.php => tests/SPTest.php +10 -1
@@ 39,6 39,7 @@ use fkooman\SAML\SP\MetadataSource;
use fkooman\SAML\SP\NameId;
use fkooman\SAML\SP\PrivateKey;
use fkooman\SAML\SP\SpInfo;
use fkooman\SAML\SP\Web\Request;
use PHPUnit\Framework\TestCase;

/**


@@ 378,7 379,15 @@ class SPTest extends TestCase
                'Signature' => 'MNvAlJmSRH0PpRNpVLt4/diLRhi7LjV986I0ZL+VXMSxsEnd197D7H/xG4EpdpiA4n+BWPyPIhlvq3xCjcr1SI3UJ53NKYyOfTUq62c+Cz6ZmSmq3ttpw5VvSf8slUd/jfNs7NfdT80V7mo992DZjzE8DBoTzUS+ByrJCfkhiPrpjc2TwiL4X6XZ80DD51k4tdQyDVbgBCnw/AlIljzhodIzg+PQp89LkAXx+4wbsxy1pqltxOhZ2toE91SPzuVYPKJQXlFVLqaGPegU+9yJCtf676KNoJ19lxtKXigBZzEouJ72p/Tsxsa+gQI47YadTOP9a3KQSE1IHYeFYzg/EsbU5RqhLPYyFksSEY+a4VNenoIWbC7X3UMj8BTMrmtE+dqQgaJ/RhhCr+lwbtNCOqJ2l9h8bc5Nrq8ycgM6l0iR+h4AFpIYl3XdPcebNu90Sn8LNFQu+30vf9QT21VFQQmo2yKnYBUwyPbW8WgTVX+/JihlpnGjuRadY7efcHEK',
            ]
        );
        $returnTo = $this->sp->handleLogoutResponse($queryString);

        $httpRequest = new Request(
            [
                'QUERY_STRING' => $queryString,
            ],
            []
        );

        $returnTo = $this->sp->handleLogoutResponse($httpRequest);
        $this->assertSame('http://localhost:8081/', $returnTo);
        $this->assertNull($session->get(TestSP::SESSION_KEY_PREFIX . '+/M7/sd8CgDR7BXVpw2lqgsalw54taH0E2eYa2RrZcI='));
    }

M web/index.php => web/index.php +1 -2
@@ 45,7 45,7 @@ $logger = new SyslogLogger('php-saml-sp');

try {
    $config = Config::fromFile($baseDir . '/config/config.php');
    $request = new Request($_SERVER, $_GET, $_POST);
    $request = new Request($_SERVER, $_POST);

    $seCookie = new SeCookie();
    $seSession = new SeSession($config);


@@ 96,7 96,6 @@ try {
    );
    $sp = new SP($spInfo, $idpSource, $seSession, $logger);
    $service = new Service($config, $tpl, $sp, $seCookie);
    $request = new Request($_SERVER, $_GET, $_POST);
    $response = $service->run($request);
    $seSession->stop();
    $response->send();