Fixes cardinality auth filter unit tests (#860) (#864)

lib/SimpleSAML/Utils/HttpAdapter.php

+<?php
+
+namespace SimpleSAML\Utils;
+
+/**
+ * Provides a non-static wrapper for the HTTP utility class.
+ *
+ * @package SimpleSAML\Utils
+ */
+class HttpAdapter
+{
+    /**
+     * @see HTTP::getServerHTTPS()
+     */
+    public function getServerHTTPS()
+    {
+        return HTTP::getServerHTTPS();
+    }
+
+    /**
+     * @see HTTP::getServerPort()
+     */
+    public function getServerPort()
+    {
+        return HTTP::getServerPort();
+    }
+
+    /**
+     * @see HTTP::addURLParameters()
+     */
+    public function addURLParameters($url, $parameters)
+    {
+        return HTTP::addURLParameters($url, $parameters);
+    }
+
+    /**
+     * @see HTTP::checkSessionCookie()
+     */
+    public function checkSessionCookie($retryURL = null)
+    {
+        HTTP::checkSessionCookie($retryURL);
+    }
+
+    /**
+     * @see HTTP::checkURLAllowed()
+     */
+    public function checkURLAllowed($url, array $trustedSites = null)
+    {
+        return HTTP::checkURLAllowed($url, $trustedSites);
+    }
+
+    /**
+     * @see HTTP::fetch()
+     */
+    public function fetch($url, $context = array(), $getHeaders = false)
+    {
+        return HTTP::fetch($url, $context, $getHeaders);
+    }
+
+    /**
+     * @see HTTP::getAcceptLanguage()
+     */
+    public function getAcceptLanguage()
+    {
+        return HTTP::getAcceptLanguage();
+    }
+
+    /**
+     * @see HTTP::guessBasePath()
+     */
+    public function guessBasePath()
+    {
+        return HTTP::guessBasePath();
+    }
+
+    /**
+     * @see HTTP::getBaseURL()
+     */
+    public function getBaseURL()
+    {
+        return HTTP::getBaseURL();
+    }
+
+    /**
+     * @see HTTP::getFirstPathElement()
+     */
+    public function getFirstPathElement($trailingslash = true)
+    {
+        return HTTP::getFirstPathElement($trailingslash);
+    }
+
+    /**
+     * @see HTTP::getPOSTRedirectURL()
+     */
+    public function getPOSTRedirectURL($destination, $data)
+    {
+        return HTTP::getPOSTRedirectURL($destination, $data);
+    }
+
+    /**
+     * @see HTTP::getSelfHost()
+     */
+    public function getSelfHost()
+    {
+        return HTTP::getSelfHost();
+    }
+
+    /**
+     * @see HTTP::getSelfHostWithNonStandardPort()
+     */
+    public function getSelfHostWithNonStandardPort()
+    {
+        return HTTP::getSelfHostWithNonStandardPort();
+    }
+
+    /**
+     * @see HTTP::getSelfHostWithPath()
+     */
+    public function getSelfHostWithPath()
+    {
+        return HTTP::getSelfHostWithPath();
+    }
+
+    /**
+     * @see HTTP::getSelfURL()
+     */
+    public function getSelfURL()
+    {
+        return HTTP::getSelfURL();
+    }
+
+    /**
+     * @see HTTP::getSelfURLHost()
+     */
+    public function getSelfURLHost()
+    {
+        return HTTP::getSelfURLHost();
+    }
+
+    /**
+     * @see HTTP::getSelfURLNoQuery()
+     */
+    public function getSelfURLNoQuery()
+    {
+        return HTTP::getSelfURLNoQuery();
+    }
+
+    /**
+     * @see HTTP::isHTTPS()
+     */
+    public function isHTTPS()
+    {
+        return HTTP::isHTTPS();
+    }
+
+    /**
+     * @see HTTP::normalizeURL()
+     */
+    public function normalizeURL($url)
+    {
+        return HTTP::normalizeURL($url);
+    }
+
+    /**
+     * @see HTTP::parseQueryString()
+     */
+    public function parseQueryString($query_string)
+    {
+        return HTTP::parseQueryString($query_string);
+    }
+
+    /**
+     * @see HTTP::redirectTrustedURL()
+     */
+    public function redirectTrustedURL($url, $parameters = array())
+    {
+        HTTP::redirectTrustedURL($url, $parameters);
+    }
+
+    /**
+     * @see HTTP::redirectUntrustedURL()
+     */
+    public function redirectUntrustedURL($url, $parameters = array())
+    {
+        HTTP::redirectUntrustedURL($url, $parameters);
+    }
+
+    /**
+     * @see HTTP::resolveURL()
+     */
+    public function resolveURL($url, $base = null)
+    {
+        return HTTP::resolveURL($url, $base);
+    }
+
+    /**
+     * @see HTTP::setCookie()
+     */
+    public function setCookie($name, $value, $params = null, $throw = true)
+    {
+        HTTP::setCookie($name, $value, $params, $throw);
+    }
+
+    /**
+     * @see HTTP::submitPOSTData()
+     */
+    public function submitPOSTData($destination, $data)
+    {
+        HTTP::submitPOSTData($destination, $data);
+    }
+}

modules/core/lib/Auth/Process/Cardinality.php

 <?php
 
+use SimpleSAML\Utils\HTTPAdapter;
+
 /**
  * Filter to ensure correct cardinality of attributes
  *
@@ -14,18 +16,24 @@ class sspmod_core_Auth_Process_Cardinality extends SimpleSAML_Auth_ProcessingFil
     /** @var array Entities that should be ignored */
     private $ignoreEntities = array();
 
+    /** @var HTTP */
+    private $http;
+
     /**
      * Initialize this filter, parse configuration.
      *
      * @param array $config  Configuration information about this filter.
      * @param mixed $reserved  For future use.
+     * @param HTTPAdapter $http  HTTP utility service (handles redirects).
      * @throws SimpleSAML_Error_Exception
      */
-    public function __construct($config, $reserved)
+    public function __construct($config, $reserved, HTTPAdapter $http = null)
     {
         parent::__construct($config, $reserved);
         assert(is_array($config));
 
+        $this->http = $http ?: new HTTPAdapter();
+
         foreach ($config as $attribute => $rules) {
             if ($attribute === '%ignoreEntities') {
                 $this->ignoreEntities = $config['%ignoreEntities'];
@@ -157,7 +165,7 @@ class sspmod_core_Auth_Process_Cardinality extends SimpleSAML_Auth_ProcessingFil
         if (array_key_exists('core:cardinality:errorAttributes', $request)) {
             $id = SimpleSAML_Auth_State::saveState($request, 'core:cardinality');
             $url = SimpleSAML\Module::getModuleURL('core/cardinality_error.php');
-            \SimpleSAML\Utils\HTTP::redirectTrustedURL($url, array('StateId' => $id));
+            $this->http->redirectTrustedURL($url, array('StateId' => $id));
             return;
         }
     }

modules/core/lib/Auth/Process/CardinalitySingle.php

 <?php
 
+use SimpleSAML\Utils\HttpAdapter;
+
 /**
  * Filter to ensure correct cardinality of single-valued attributes
  *
@@ -26,17 +28,23 @@ class sspmod_core_Auth_Process_CardinalitySingle extends SimpleSAML_Auth_Process
     /** @var array Entities that should be ignored */
     private $ignoreEntities = array();
 
+    /** @var HTTP */
+    private $http;
+
     /**
      * Initialize this filter, parse configuration.
      *
      * @param array $config  Configuration information about this filter.
      * @param mixed $reserved  For future use.
+     * @param HTTPAdapter $http  HTTP utility service (handles redirects).
      */
-    public function __construct($config, $reserved)
+    public function __construct($config, $reserved, HTTPAdapter $http = null)
     {
         parent::__construct($config, $reserved);
         assert(is_array($config));
 
+        $this->http = $http ?: new HTTPAdapter();
+
         if (array_key_exists('singleValued', $config)) {
             $this->singleValued = $config['singleValued'];
         }
@@ -102,7 +110,7 @@ class sspmod_core_Auth_Process_CardinalitySingle extends SimpleSAML_Auth_Process
         if (array_key_exists('core:cardinality:errorAttributes', $request)) {
             $id = SimpleSAML_Auth_State::saveState($request, 'core:cardinality');
             $url = SimpleSAML\Module::getModuleURL('core/cardinality_error.php');
-            \SimpleSAML\Utils\HTTP::redirectTrustedURL($url, array('StateId' => $id));
+            $this->http->redirectTrustedURL($url, array('StateId' => $id));
             return;
         }
     }

tests/modules/core/lib/Auth/Process/CardinalitySingleTest.php

@@ -9,6 +9,8 @@ if (class_exists('\PHPUnit\Framework\TestCase', true) and !class_exists('\PHPUni
  */
 class Test_Core_Auth_Process_CardinalitySingleTest extends \PHPUnit_Framework_TestCase
 {
+    private $http;
+
     /**
      * Helper function to run the filter with a given configuration.
      *
@@ -16,11 +18,11 @@ class Test_Core_Auth_Process_CardinalitySingleTest extends \PHPUnit_Framework_Te
      * @param  array $request The request state.
      * @return array  The state array after processing.
      */
-    private static function processFilter(array $config, array $request)
+    private function processFilter(array $config, array $request)
     {
         $_SERVER['SERVER_PROTOCOL'] = 'HTTP/1.1';
         $_SERVER['REQUEST_METHOD'] = 'GET';
-        $filter = new sspmod_core_Auth_Process_CardinalitySingle($config, null);
+        $filter = new sspmod_core_Auth_Process_CardinalitySingle($config, null, $this->http);
         $filter->process($request);
         return $request;
     }
@@ -28,6 +30,9 @@ class Test_Core_Auth_Process_CardinalitySingleTest extends \PHPUnit_Framework_Te
     protected function setUp()
     {
         \SimpleSAML_Configuration::loadFromArray(array(), '[ARRAY]', 'simplesaml');
+        $this->http = $this->getMockBuilder('SimpleSAML\Utils\HTTPAdapter')
+                           ->setMethods(array('redirectTrustedURL'))
+                           ->getMock();
     }
 
     /**
@@ -43,7 +48,7 @@ class Test_Core_Auth_Process_CardinalitySingleTest extends \PHPUnit_Framework_Te
                 'eduPersonPrincipalName' => array('joe@example.com'),
             ),
         );
-        $result = self::processFilter($config, $request);
+        $result = $this->processFilter($config, $request);
         $attributes = $result['Attributes'];
         $expectedData = array('eduPersonPrincipalName' => array('joe@example.com'));
         $this->assertEquals($expectedData, $attributes, "Assertion values should not have changed");
@@ -62,7 +67,7 @@ class Test_Core_Auth_Process_CardinalitySingleTest extends \PHPUnit_Framework_Te
                 'eduPersonPrincipalName' => array('joe@example.com', 'bob@example.net'),
             ),
         );
-        $result = self::processFilter($config, $request);
+        $result = $this->processFilter($config, $request);
         $attributes = $result['Attributes'];
         $expectedData = array('eduPersonPrincipalName' => array('joe@example.com'));
         $this->assertEquals($expectedData, $attributes, "Only first value should be returned");
@@ -78,7 +83,7 @@ class Test_Core_Auth_Process_CardinalitySingleTest extends \PHPUnit_Framework_Te
                 'eduPersonPrincipalName' => array('joe@example.com'),
             ),
         );
-        $result = self::processFilter($config, $request);
+        $result = $this->processFilter($config, $request);
         $attributes = $result['Attributes'];
         $expectedData = array('eduPersonPrincipalName' => array('joe@example.com'));
         $this->assertEquals($expectedData, $attributes, "Assertion values should not have changed");
@@ -98,7 +103,7 @@ class Test_Core_Auth_Process_CardinalitySingleTest extends \PHPUnit_Framework_Te
                 'eduPersonPrincipalName' => array('joe@example.com', 'bob@example.net'),
             ),
         );
-        $result = self::processFilter($config, $request);
+        $result = $this->processFilter($config, $request);
         $attributes = $result['Attributes'];
         $expectedData = array('eduPersonPrincipalName' => array('joe@example.com|bob@example.net'));
         $this->assertEquals($expectedData, $attributes, "Flattened string should be returned");
@@ -115,7 +120,7 @@ class Test_Core_Auth_Process_CardinalitySingleTest extends \PHPUnit_Framework_Te
                 'eduPersonPrincipalName' => array('joe@example.com'),
             ),
         );
-        $result = self::processFilter($config, $request);
+        $result = $this->processFilter($config, $request);
         $attributes = $result['Attributes'];
         $expectedData = array('eduPersonPrincipalName' => array('joe@example.com'));
         $this->assertEquals($expectedData, $attributes, "Assertion values should not have changed");
@@ -123,13 +128,9 @@ class Test_Core_Auth_Process_CardinalitySingleTest extends \PHPUnit_Framework_Te
 
     /**
      * Test abort
-     * @expectedException PHPUnit_Framework_Error
-     * @expectedExceptionMessageRegExp /REQUEST_URI/
      */
     public function testAbort()
     {
-        $this->markTestSkipped('Breaks testsuite see #860');
-
         $config = array(
             'singleValued' => array('eduPersonPrincipalName'),
         );
@@ -138,6 +139,10 @@ class Test_Core_Auth_Process_CardinalitySingleTest extends \PHPUnit_Framework_Te
                 'eduPersonPrincipalName' => array('joe@example.com', 'bob@example.net'),
             ),
         );
-        self::processFilter($config, $request);
+
+        $this->http->expects($this->once())
+                   ->method('redirectTrustedURL');
+
+        $this->processFilter($config, $request);
     }
 }

tests/modules/core/lib/Auth/Process/CardinalityTest.php

@@ -9,6 +9,8 @@ if (class_exists('\PHPUnit\Framework\TestCase', true) and !class_exists('\PHPUni
  */
 class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
 {
+    private $http;
+
     /**
      * Helper function to run the filter with a given configuration.
      *
@@ -16,11 +18,11 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
      * @param  array $request The request state.
      * @return array  The state array after processing.
      */
-    private static function processFilter(array $config, array $request)
+    private function processFilter(array $config, array $request)
     {
         $_SERVER['SERVER_PROTOCOL'] = 'HTTP/1.1';
         $_SERVER['REQUEST_METHOD'] = 'GET';
-        $filter = new sspmod_core_Auth_Process_Cardinality($config, null);
+        $filter = new sspmod_core_Auth_Process_Cardinality($config, null, $this->http);
         $filter->process($request);
         return $request;
     }
@@ -28,6 +30,9 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
     protected function setUp()
     {
         \SimpleSAML_Configuration::loadFromArray(array(), '[ARRAY]', 'simplesaml');
+        $this->http = $this->getMockBuilder('SimpleSAML\Utils\HTTPAdapter')
+                           ->setMethods(array('redirectTrustedURL'))
+                           ->getMock();
     }
 
     /*
@@ -43,7 +48,7 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
                 'mail' => array('joe@example.com', 'bob@example.com'),
             ),
         );
-        $result = self::processFilter($config, $request);
+        $result = $this->processFilter($config, $request);
         $attributes = $result['Attributes'];
         $expectedData = array('mail' => array('joe@example.com', 'bob@example.com'));
         $this->assertEquals($expectedData, $attributes, "Assertion values should not have changed");
@@ -62,7 +67,7 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
                 'mail' => array('joe@example.com', 'bob@example.com'),
             ),
         );
-        $result = self::processFilter($config, $request);
+        $result = $this->processFilter($config, $request);
         $attributes = $result['Attributes'];
         $expectedData = array('mail' => array('joe@example.com', 'bob@example.com'));
         $this->assertEquals($expectedData, $attributes, "Assertion values should not have changed");
@@ -81,7 +86,7 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
                 'mail' => array('joe@example.com', 'bob@example.com'),
             ),
         );
-        $result = self::processFilter($config, $request);
+        $result = $this->processFilter($config, $request);
         $attributes = $result['Attributes'];
         $expectedData = array('mail' => array('joe@example.com', 'bob@example.com'));
         $this->assertEquals($expectedData, $attributes, "Assertion values should not have changed");
@@ -89,12 +94,9 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
 
     /**
      * Test maximum is out of bounds results in redirect
-     * @expectedException PHPUnit_Framework_Error
-     * @expectedExceptionMessageRegExp /REQUEST_URI/
      */
     public function testMaxOutOfBounds()
     {
-        $this->markTestSkipped('Breaks testsuite see #860');
         $config = array(
             'mail' => array('max' => 2),
         );
@@ -103,17 +105,18 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
                 'mail' => array('joe@example.com', 'bob@example.com', 'fred@example.com'),
             ),
         );
-        self::processFilter($config, $request);
+
+        $this->http->expects($this->once())
+                   ->method('redirectTrustedURL');
+
+        $this->processFilter($config, $request);
     }
 
     /**
      * Test minimum is out of bounds results in redirect
-     * @expectedException PHPUnit_Framework_Error
-     * @expectedExceptionMessageRegExp /REQUEST_URI/
      */
     public function testMinOutOfBounds()
     {
-        $this->markTestSkipped('Breaks testsuite see #860');
         $config = array(
             'mail' => array('min' => 3),
         );
@@ -122,24 +125,29 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
                 'mail' => array('joe@example.com', 'bob@example.com'),
             ),
         );
-        self::processFilter($config, $request);
+
+        $this->http->expects($this->once())
+                   ->method('redirectTrustedURL');
+
+        $this->processFilter($config, $request);
     }
 
     /**
      * Test missing attribute results in redirect
-     * @expectedException PHPUnit_Framework_Error
-     * @expectedExceptionMessageRegExp /REQUEST_URI/
      */
     public function testMissingAttribute()
     {
-        $this->markTestSkipped('Breaks testsuite see #860');
         $config = array(
             'mail' => array('min' => 1),
         );
         $request = array(
             'Attributes' => array( ),
         );
-        self::processFilter($config, $request);
+
+        $this->http->expects($this->once())
+                   ->method('redirectTrustedURL');
+
+        $this->processFilter($config, $request);
     }
 
     /*
@@ -153,7 +161,6 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
      */
     public function testMinInvalid()
     {
-        $this->markTestSkipped('Breaks testsuite see #860');
         $config = array(
             'mail' => array('min' => false),
         );
@@ -162,7 +169,7 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
                 'mail' => array('joe@example.com', 'bob@example.com'),
             ),
         );
-        self::processFilter($config, $request);
+        $this->processFilter($config, $request);
     }
 
     /**
@@ -172,7 +179,6 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
      */
     public function testMinNegative()
     {
-        $this->markTestSkipped('Breaks testsuite see #860');
         $config = array(
             'mail' => array('min' => -1),
         );
@@ -181,7 +187,7 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
                 'mail' => array('joe@example.com', 'bob@example.com'),
             ),
         );
-        self::processFilter($config, $request);
+        $this->processFilter($config, $request);
     }
 
     /**
@@ -191,7 +197,6 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
      */
     public function testMaxInvalid()
     {
-        $this->markTestSkipped('Breaks testsuite see #860');
         $config = array(
             'mail' => array('max' => false),
         );
@@ -200,7 +205,7 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
                 'mail' => array('joe@example.com', 'bob@example.com'),
             ),
         );
-        self::processFilter($config, $request);
+        $this->processFilter($config, $request);
     }
 
     /**
@@ -210,7 +215,6 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
      */
     public function testMinGreaterThanMax()
     {
-        $this->markTestSkipped('Breaks testsuite see #860');
         $config = array(
             'mail' => array('min' => 2, 'max' => 1),
         );
@@ -219,7 +223,7 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
                 'mail' => array('joe@example.com', 'bob@example.com'),
             ),
         );
-        self::processFilter($config, $request);
+        $this->processFilter($config, $request);
     }
 
     /**
@@ -229,7 +233,6 @@ class Test_Core_Auth_Process_CardinalityTest extends \PHPUnit_Framework_TestCase
      */
     public function testInvalidAttributeName()
     {
-        $this->markTestSkipped('Breaks testsuite see #860');
         $config = array(
             array('min' => 2, 'max' => 1),
         );

tools/phpunit/phpunit.xml

@@ -23,6 +23,7 @@
             <exclude>
                 <directory>./../../vendor/</directory>
                 <directory>./../../tests/</directory>
+                <file>./../../lib/SimpleSAML/Utils/HttpAdapter.php</file>
             </exclude>
         </whitelist>
     </filter>