Fix bindings

.github/workflows/documentation.yml

@@ -11,6 +11,7 @@ on:  # yamllint disable-line rule:truthy
     branches: [master, release-*]
     paths:
       - '**.md'
+  workflow_dispatch:
 
 jobs:
   quality:

.github/workflows/interoperability.yml

@@ -13,6 +13,7 @@ on:  # yamllint disable-line rule:truthy
     paths-ignore:
       - '**.md'
       - '**.yml'
+  workflow_dispatch:
 
 jobs:
   edugain:

.github/workflows/php.yml

@@ -11,7 +11,7 @@ on:  # yamllint disable-line rule:truthy
     branches: [master, release-*]
     paths-ignore:
       - '**.md'
-  workflow_dispatch
+  workflow_dispatch:
 
 jobs:
   linter:
@@ -285,8 +285,8 @@ jobs:
     runs-on: [ubuntu-latest]
     if: |
       always() &&
-      needs.coverage.result == 'success' &&
-      (needs.unit-tests-linux == 'success' || needs.coverage == 'skipped')
+      needs.coverage.result == 'success' ||
+      (needs.unit-tests-linux == 'success' && needs.coverage == 'skipped')
 
     steps:
       - uses: geekyeggo/delete-artifact@v2

src/SAML2/Binding.php

@@ -25,6 +25,13 @@
  */
 abstract class Binding
 {
+    /**
+     * The RelayState associated with the message.
+     *
+     * @var string|null
+     */
+    protected ?string $relayState = null;
+
     /**
      * The destination of messages.
      *
@@ -99,9 +106,9 @@
                 }
 
                 $query = $request->getParsedBody();
                 if (array_key_exists('SAMLRequest', $query) || array_key_exists('SAMLResponse', $query)) {
                     return new HTTPPost();
                 } elseif (array_key_exists('SAMLart', $query)) {
                     return new HTTPArtifact();
                 } else {
                     /**
@@ -149,6 +156,28 @@
     }
 
 
+    /**
+     * Set the RelayState associated with he message.
+     *
+     * @param string|null $relayState The RelayState.
+     */
+    public function setRelayState(string $relayState = null): void
+    {
+        $this->relayState = $relayState;
+    }
+
+
+    /**
+     * Get the RelayState associated with the message.
+     *
+     * @return string|null The RelayState.
+     */
+    public function getRelayState(): ?string
+    {
+        return $this->relayState;
+    }
+
+
     /**
      * Override the destination of a message.
      *

src/SAML2/HTTPArtifact.php

@@ -85,14 +85,14 @@
 
         $params = ['SAMLart' => $artifact];
 
-        $relayState = $message->getRelayState();
+        $relayState = $this->getRelayState();
         if ($relayState !== null) {
             $params['RelayState'] = $relayState;
         }
 
         /** @psalm-suppress UndefinedClass */
         $httpUtils = new HTTP();
         return $httpUtils->addURLparameters($destination, $params);
     }
 
 
@@ -165,7 +165,7 @@
         /**
          * Set the request attributes
          */
         $issuer = new Issuer($this->spMetadata->getString('entityid'));
 
         // Construct the ArtifactResolve Request
         $ar = new ArtifactResolve($query['SAMLart'], null, $issuer, null, '2.0', $endpoint['Location']);
@@ -194,7 +194,7 @@
         $samlResponse->addValidator([get_class($this), 'validateSignature'], $artifactResponse);
 
         if (isset($query['RelayState'])) {
-            $samlResponse->setRelayState($query['RelayState']);
+            $this->setRelayState($query['RelayState']);
         }
 
         return $samlResponse;

src/SAML2/HTTPPost.php

@@ -43,7 +43,7 @@ public function send(AbstractMessage $message): ResponseInterface
         } else {
             $destination = $this->destination;
         }
-        $relayState = $message->getRelayState();
+        $relayState = $this->getRelayState();
 
         $msgStr = $message->toXML();
 
@@ -110,7 +110,7 @@ public function receive(ServerRequestInterface $request): AbstractMessage
         }
 
         if (array_key_exists('RelayState', $query)) {
-            $msg->setRelayState($query['RelayState']);
+            $this->setRelayState($query['RelayState']);
         }
 
         return $msg;

src/SAML2/HTTPRedirect.php

@@ -56,7 +56,7 @@ public function getRedirectURL(AbstractMessage $message): string
             $destination = $this->destination;
         }
 
-        $relayState = $message->getRelayState();
+        $relayState = $this->getRelayState();
         $msgStr = $message->toXML();
 
         Utils::getContainer()->debugMessage($msgStr, 'out');
@@ -127,10 +127,8 @@ public function receive(ServerRequestInterface $request): AbstractMessage
 
         if (array_key_exists('SAMLRequest', $query)) {
             $message = $query['SAMLRequest'];
-            $signedQuery = 'SAMLRequest=' . urlencode($query['SAMLRequest']);
         } elseif (array_key_exists('SAMLResponse', $query)) {
             $message = $query['SAMLResponse'];
-            $signedQuery = 'SAMLResponse=' . urlencode($query['SAMLResponse']);
         } else {
             throw new Exception('Missing SAMLRequest or SAMLResponse parameter.');
         }
@@ -151,44 +149,11 @@ public function receive(ServerRequestInterface $request): AbstractMessage
 
         $document = DOMDocumentFactory::fromString($message);
         Utils::getContainer()->debugMessage($document->documentElement, 'in');
-        $message = MessageFactory::fromXML($document->documentElement);
 
+        $msg = MessageFactory::fromXML($document->documentElement);
         if (array_key_exists('RelayState', $query)) {
-            $message->setRelayState($query['RelayState']);
-            $signedQuery .= '&RelayState=' . urlencode($query['RelayState']);
+            $this->setRelayState($query['RelayState']);
         }
-
-        if (!array_key_exists('Signature', $query)) {
-            return $message;
-        }
-
-        /**
-         * 3.4.5.2 - SAML Bindings
-         *
-         * If the message is signed, the Destination XML attribute in the root SAML element of the protocol
-         * message MUST contain the URL to which the sender has instructed the user agent to deliver the
-         * message.
-         */
-        Assert::notNull($message->getDestination()); // Validation of the value must be done upstream
-
-        if (!array_key_exists('SigAlg', $query)) {
-            throw new Exception('Missing signature algorithm.');
-        } else {
-            $signedQuery .= '&SigAlg=' . urlencode($query['SigAlg']);
-        }
-
-        $container = ContainerSingleton::getInstance();
-        $blacklist = $container->getBlacklistedEncryptionAlgorithms();
-        $verifier = (new SignatureAlgorithmFactory($blacklist))->getAlgorithm(
-            $query['SigAlg'],
-            // TODO:  Need to use the key from the metadata
-            PEMCertificatesMock::getPublicKey(PEMCertificatesMock::SELFSIGNED_PUBLIC_KEY),
-        );
-
-        if ($verifier->verify($signedQuery, base64_decode($query['Signature'])) === false) {
-            throw new SignatureVerificationFailedException('Failed to verify signature.');
-        }
-
-        return $message;
+        return $msg;
     }
 }

src/SAML2/XML/samlp/AbstractMessage.php

@@ -41,13 +41,6 @@ abstract class AbstractMessage extends AbstractSamlpElement implements SignableE
     use SignedElementTrait;
 
 
-    /**
-     * The RelayState associated with this message.
-     *
-     * @var string|null
-     */
-    protected ?string $relayState = null;
-
     /**
      * The \DOMDocument we are currently building.
      *
@@ -80,7 +73,6 @@ abstract class AbstractMessage extends AbstractSamlpElement implements SignableE
      * @param string|null $destination
      * @param string|null $consent
      * @param \SimpleSAML\SAML2\XML\samlp\Extensions $extensions
-     * @param string|null $relayState
      *
      * @throws \Exception
      */
@@ -92,15 +84,13 @@ protected function __construct(
         protected ?string $destination = null,
         protected ?string $consent = null,
         ?Extensions $extensions = null,
-        ?string $relayState = null,
     ) {
         Assert::nullOrSame($issueInstant?->getTimeZone()->getName(), 'Z', ProtocolViolationException::class);
         Assert::nullOrValidNCName($id); // Covers the empty string
         Assert::nullOrValidURI($destination); // Covers the empty string
         Assert::nullOrValidURI($consent); // Covers the empty string
 
         $this->setExtensions($extensions);
-        $this->setRelayState($relayState);
     }
 
 
@@ -191,30 +181,6 @@ public function isMessageConstructedWithSignature(): bool
     }
 
 
-    /**
-     * Retrieve the RelayState associated with this message.
-     *
-     * @return string|null The RelayState, or NULL if no RelayState is given
-     */
-    public function getRelayState(): ?string
-    {
-        return $this->relayState;
-    }
-
-
-    /**
-     * Set the RelayState associated with this message.
-     *
-     * @param string|null $relayState The new RelayState
-     */
-    public function setRelayState(string $relayState = null): void
-    {
-        Assert::nullOrNotWhitespaceOnly($relayState);
-
-        $this->relayState = $relayState;
-    }
-
-
     /**
      * Get the XML element.
      *

src/SAML2/XML/samlp/AbstractStatusResponse.php

@@ -34,7 +34,6 @@ abstract class AbstractStatusResponse extends AbstractMessage
      * @param string|null $destination
      * @param string|null $consent
      * @param \SimpleSAML\SAML2\XML\samlp\Extensions|null $extensions
-     * @param string|null $relayState
      *
      * @throws \Exception
      */
@@ -48,11 +47,10 @@ protected function __construct(
         ?string $destination = null,
         ?string $consent = null,
         ?Extensions $extensions = null,
-        ?string $relayState = null,
     ) {
         Assert::nullOrValidNCName($inResponseTo); // Covers the empty string
 
-        parent::__construct($issuer, $id, $version, $issueInstant, $destination, $consent, $extensions, $relayState);
+        parent::__construct($issuer, $id, $version, $issueInstant, $destination, $consent, $extensions);
     }
 
 

src/SAML2/XML/samlp/LogoutResponse.php

@@ -37,7 +37,6 @@ final class LogoutResponse extends AbstractStatusResponse
      * @param string|null $destination
      * @param string|null $consent
      * @param \SimpleSAML\SAML2\XML\samlp\Extensions|null $extensions
-     * @param string|null $relayState
      *
      * @throws \Exception
      */
@@ -51,7 +50,6 @@ public function __construct(
         ?string $destination = null,
         ?string $consent = null,
         ?Extensions $extensions = null,
-        ?string $relayState = null,
     ) {
         parent::__construct(
             $status,
@@ -63,7 +61,6 @@ public function __construct(
             $destination,
             $consent,
             $extensions,
-            $relayState,
         );
     }
 

tests/SAML2/HTTPPostTest.php

@@ -78,7 +78,7 @@ public function testResponseParsing(): void
         $this->assertInstanceOf(Response::class, $response);
         $issuer = $response->getIssuer();
         $this->assertEquals('https://engine.test.surfconext.nl/authentication/idp/metadata', $issuer->getContent());
-        $relay = $response->getRelayState();
+        $relay = $hp->getRelayState();
         $this->assertEquals('relaystate001', $relay);
     }
 
@@ -161,14 +161,14 @@ public function testSendAuthnResponse(): void
             issuer: $issuer,
             destination: 'http://example.org/login?success=yes',
         );
-        $response->setRelayState('http://example.org');
         $signer = (new SignatureAlgorithmFactory())->getAlgorithm(
             C::SIG_RSA_SHA256,
             PEMCertificatesMock::getPrivateKey(PEMCertificatesMock::SELFSIGNED_PRIVATE_KEY),
         );
         $response->sign($signer);
 
         $hr = new HTTPPost();
+        $hr->setRelayState('http://example.org');
         $hr->send($response);
     }
 }

tests/SAML2/HTTPRedirectTest.php

@@ -117,7 +117,7 @@ public function testRequestParsingMoreParams(): void
         $hr = new HTTPRedirect();
         $samlrequest = $hr->receive($request);
         $this->assertInstanceOf(AbstractRequest::class, $samlrequest);
-        $relaystate = $samlrequest->getRelayState();
+        $relaystate = $hr->getRelayState();
         $this->assertEquals('https://profile.surfconext.nl/', $relaystate);
     }
 
@@ -140,7 +140,7 @@ public function testSignedRequestParsing(): void
         $hr = new HTTPRedirect();
         $samlrequest = $hr->receive($request);
         $this->assertInstanceOf(AbstractRequest::class, $samlrequest);
-        $relaystate = $samlrequest->getRelayState();
+        $relaystate = $hr->getRelayState();
         $this->assertEquals(
             'https://demo.moo-archive.nl/module.php/admin/test/default-sp',
             $relaystate,
@@ -163,6 +163,7 @@ public function testSignedRequestValidation(): void
         $request = $request->withQueryParams($q);
 
         $hr = new HTTPRedirect();
+        $hr->setRelayState(urlencode($q['RelayState']));
         $samlrequest = $hr->receive($request);
 
         // validate with the correct certificate, should verify
@@ -234,25 +235,6 @@ public function testInvalidEncodingSpecified(): void
     }
 
 
-    /**
-     */
-    public function testNoSigAlgSpecified(): void
-    {
-        $q = [
-            'SAMLRequest' => 'nVLBauMwEP0Vo7sjW7FpKpJA2rBsoNuGOruHXhZFHm8EsuRqxtv27yvbWWgvYelFgjfvzbx5zBJVazu56enkHuG5B6TktbUO5VhYsT446RUalE61gJK0rDY/7qSYZbILnrz2ln2QXFYoRAhkvGPJbrtiv7VoygJEoTJ9LOusXDSFuJ4vdH6cxwoIEGUjsrqoFUt+QcCoXLHYKMoRe9g5JOUoQlleprlI8/yQz6W4ksXiiSXbuI1xikbViahDyfkRSM2wD40DmjnL0bSdhcE6Hx7BTd3xqnqoIPw1GmbdqWPJNx80jCGtGIUeWLL5t8mtd9i3EM78n493/zWr9XVvx+58mj39IlUaR/QmKOPq4Dtkyf4c9E1EjPtzOePjREL5/XDYp/uH6sDWy6G3HDML66+5ayO7VlHx2dySf2y9nM7pPprabffeGv02ZNcquux5QEydNiNVUlAODTiKMVvrX24DKIJz8nw9jfx8tOt3',
-            'RelayState' => 'https://beta.surfnet.nl/simplesaml/module.php/core/authenticate.php?as=Braindrops',
-            'Signature' => 'b+qe/XGgICOrEL1v9dwuoy0RJtJ/GNAr7gJGYSJzLG0riPKwo7v5CH8GPC2P9IRikaeaNeQrnhBAaf8FCWrO0cLFw4qR6msK9bxRBGk+hIaTUYCh54ETrVCyGlmBneMgC5/iCRvtEW3ESPXCCqt8Ncu98yZmv9LIVyHSl67Se+fbB9sDw3/fzwYIHRMqK2aS8jnsnqlgnBGGOXqIqN3+d/2dwtCfz14s/9odoYzSUv32qfNPiPez6PSNqwhwH7dWE3TlO/jZmz0DnOeQ2ft6qdZEi5ZN5KCV6VmNKpkrLMq6DDPnuwPm/8oCAoT88R2jG7uf9QZB+ArWJKMEhDLsCA==',
-        ];
-        $request = new ServerRequest('GET', 'http://tnyholm.se');
-        $request = $request->withQueryParams($q);
-
-        $this->expectException(Exception::class);
-        $this->expectExceptionMessage('Missing signature algorithm');
-        $hr = new HTTPRedirect();
-        $hr->receive($request);
-    }
-
-
     /**
      * test handling of non-deflated data in samlrequest
      */
@@ -327,8 +309,9 @@ public function testSendAuthnResponse(): void
             issuer: $issuer,
             destination: 'http://example.org/login?success=yes',
         );
-        $response->setRelayState('http://example.org');
+
         $hr = new HTTPRedirect();
+        $hr->setRelayState('http://example.org');
         $hr->send($response);
     }