Separate assertion validation from decryption

In order to check if an assertion is signed without validating the
contents, we need to separate the logic. Code using the response
processor is not affected, the behaviour is identical. But if users of
the SAML2 library are calling the attribute processor directly, they
must not call decryptAssertions() before calling processAssertions().

There is no easy way to make this backwards compatible but it should
not cause problems because you would usually not use the assertion
processor stand-alone.

src/SAML2/Assertion/Processor.php

@@ -74,29 +74,49 @@ public function __construct(
     }
 
     /**
-     * @param \SAML2\Utilities\ArrayCollection $assertions
+     * Decrypt assertions, or do nothing if assertions are already decrypted.
+     *
+     * @param ArrayCollection $assertions
+     *
+     * @return \SAML2\Assertion[] Collection (\SAML2\Utilities\ArrayCollection) of decrypted assertions
+     */
+    public function decryptAssertions(ArrayCollection $assertions)
+    {
+        $decrypted = new ArrayCollection();
+        foreach ($assertions->getIterator() as $assertion) {
+            $decrypted->add($this->decryptAssertion($assertion));
+        }
+
+        return $decrypted;
+    }
+
+    /**
+     * @param \SAML2\Utilities\ArrayCollection|array $assertions Collection of decrypted assertions
      *
      * @return \SAML2\Assertion[] Collection (\SAML2\Utilities\ArrayCollection) of processed assertions
      */
     public function processAssertions($assertions)
     {
+        // BC compatibility, allow array argument.
+        if (is_array($assertions)) {
+            $assertions = new ArrayCollection($assertions);
+        }
+
         $processed = new ArrayCollection();
-        foreach ($assertions as $assertion) {
+        foreach ($assertions->getIterator() as $assertion) {
             $processed->add($this->process($assertion));
         }
 
         return $processed;
     }
 
     /**
-     * @param \SAML2\Assertion|\SAML2\EncryptedAssertion $assertion
+     * @param \SAML2\Assertion $assertion
      *
      * @return \SAML2\Assertion
      */
     public function process($assertion)
     {
-        $assertion = $this->decryptAssertion($assertion);
-
         if (!$assertion->getWasSignedAtConstruction()) {
             $this->logger->info(sprintf(
                 'Assertion with id "%s" was not signed at construction, not verifying the signature',

src/SAML2/Response/Processor.php

@@ -14,6 +14,7 @@
 use SAML2\Response\Exception\UnsignedResponseException;
 use SAML2\Response\Validation\PreconditionValidator;
 use SAML2\Signature\Validator;
+use SAML2\Utilities\ArrayCollection;
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects) - due to specific exceptions
@@ -144,10 +145,12 @@ private function processAssertions(Response $response)
             throw new NoAssertionsFoundException('No assertions found in response from IdP.');
         }
 
-        $assertions =  $this->assertionProcessor->processAssertions($assertions);
+        $decryptedAssertions = $this->assertionProcessor->decryptAssertions(
+            new ArrayCollection($assertions)
+        );
 
         if (!$this->responseIsSigned) {
-            foreach ($assertions->getIterator() as $assertion) {
+            foreach ($decryptedAssertions->getIterator() as $assertion) {
                 if (!$assertion->getWasSignedAtConstruction()) {
                     throw new UnsignedResponseException(
                         'Both the response and the assertion it contains are not signed.'
@@ -156,6 +159,6 @@ private function processAssertions(Response $response)
             }
         }
 
-        return $assertions;
+        return $this->assertionProcessor->processAssertions($decryptedAssertions);
     }
 }

tests/SAML2/Response/SignatureValidationTest.php

@@ -77,10 +77,12 @@ public function tearDown()
      */
     public function testThatAnUnsignedResponseWithASignedAssertionCanBeProcessed()
     {
-        $this->assertionProcessor->shouldReceive('processAssertions')
+        $this->assertionProcessor->shouldReceive('decryptAssertions')
             ->once()
             ->andReturn(new ArrayCollection());
 
+        $this->assertionProcessor->shouldReceive('processAssertions')->once();
+
         $processor = new Processor(new \Psr\Log\NullLogger());
 
         $processor->process(
@@ -96,6 +98,10 @@ public function testThatAnUnsignedResponseWithASignedAssertionCanBeProcessed()
      */
     public function testThatAnSignedResponseWithAnUnsignedAssertionCanBeProcessed()
     {
+        $this->assertionProcessor->shouldReceive('decryptAssertions')
+            ->once()
+            ->andReturn(new ArrayCollection());
+
         $this->assertionProcessor->shouldReceive('processAssertions')->once();
 
         $processor = new Processor(new \Psr\Log\NullLogger());
@@ -113,6 +119,10 @@ public function testThatAnSignedResponseWithAnUnsignedAssertionCanBeProcessed()
      */
     public function testThatASignedResponseWithASignedAssertionIsValid()
     {
+        $this->assertionProcessor->shouldReceive('decryptAssertions')
+            ->once()
+            ->andReturn(new ArrayCollection());
+
         $this->assertionProcessor->shouldReceive('processAssertions')->once();
 
         $processor = new Processor(new \Psr\Log\NullLogger());
@@ -136,14 +146,15 @@ public function testThatAnUnsignedResponseWithNoSignedAssertionsThrowsAnExceptio
             ->once()
             ->andReturn(false);
 
-        // The processAssertions is called to decrypt possible encrypted assertions,
-        // after which it should fail with an exception due to having no signature
-        $this->assertionProcessor->shouldReceive('processAssertions')
+        $this->assertionProcessor->shouldReceive('decryptAssertions')
             ->once()
             ->andReturn(new ArrayCollection([
                 $assertion
             ]));
 
+        // here the processAssertions may not be called as it should fail with an exception due to having no signature
+        $this->assertionProcessor->shouldReceive('processAssertions')->never();
+
         $processor = new Processor(new \Psr\Log\NullLogger());
 
         $processor->process(