Do not store the username and password obtained via ECP in the state array.

modules/core/lib/Auth/UserPassBase.php

@@ -192,11 +192,20 @@ abstract class UserPassBase extends \SimpleSAML\Auth\Source
             $state['forcedUsername'] = $this->forcedUsername;
         }
 
-        // ECP requests supply authentication credentials with the AUthnRequest
-        // so we validate them now rather than redirecting
-        if (isset($state['core:auth:username']) && isset($state['core:auth:password'])) {
-            $username = $state['core:auth:username'];
-            $password = $state['core:auth:password'];
+        // ECP requests supply authentication credentials with the AuthnRequest
+        // so we validate them now rather than redirecting. The SAML spec
+        // doesn't define how the credentials are transferred, but Office 365
+        // uses the Authorization header, so we will just use that in lieu of
+        // other use cases.
+        if (isset($state['saml:Binding']) && $state['saml:Binding'] === \SAML2\Constants::BINDING_PAOS) {
+            if (!isset($_SERVER['PHP_AUTH_USER']) || !isset($_SERVER['PHP_AUTH_PW'])) {
+                \SimpleSAML\Logger::error("ECP AuthnRequest did not contain Basic Authentication header");
+                // TODO Return a SOAP fault instead of using the current binding?
+                throw new \SimpleSAML\Error\Error("WRONGUSERPASS");
+            }
+
+            $username = $_SERVER['PHP_AUTH_USER'];
+            $password = $_SERVER['PHP_AUTH_PW'];
 
             if (isset($state['forcedUsername'])) {
                 $username = $state['forcedUsername'];

modules/saml/lib/IdP/SAML2.php

@@ -455,26 +455,9 @@ class SAML2
             'saml:RequestedAuthnContext'  => $authnContext,
         ];
 
-        // ECP AuthnRequests need to supply credentials
-        if ($binding instanceof SOAP) {
-            self::processSOAPAuthnRequest($state);
-        }
-
         $idp->handleAuthenticationRequest($state);
     }
 
-    public static function processSOAPAuthnRequest(array &$state)
-    {
-        if (!isset($_SERVER['PHP_AUTH_USER']) || !isset($_SERVER['PHP_AUTH_PW'])) {
-            Logger::error("ECP AuthnRequest did not contain Basic Authentication header");
-            // TODO Throw some sort of ECP-specific exception / convert this to SOAP fault
-            throw new \SimpleSAML\Error\Error("WRONGUSERPASS");
-        }
-
-        $state['core:auth:username'] = $_SERVER['PHP_AUTH_USER'];
-        $state['core:auth:password'] = $_SERVER['PHP_AUTH_PW'];
-    }
-
     /**
      * Send a logout request to a given association.
      *

tests/modules/core/lib/Auth/UserPassBaseTest.php

@@ -6,11 +6,13 @@ class UserPassBaseTest extends \PHPUnit_Framework_TestCase
 {
     public function testAuthenticateECPCallsLoginAndSetsAttributes()
     {
-        $state = [];
+        $state = [
+            'saml:Binding' => \SAML2\Constants::BINDING_PAOS,
+        ];
         $attributes = ['attrib' => 'val'];
 
-        $username = $state['core:auth:username'] = 'username';
-        $password = $state['core:auth:password'] = 'password';
+        $username = $_SERVER['PHP_AUTH_USER'] = 'username';
+        $password = $_SERVER['PHP_AUTH_PW'] = 'password';
 
         $stub = $this->getMockBuilder('\SimpleSAML\Module\core\Auth\UserPassBase')
             ->disableOriginalConstructor()
@@ -27,15 +29,56 @@ class UserPassBaseTest extends \PHPUnit_Framework_TestCase
         $this->assertSame($attributes, $state['Attributes']);
     }
 
+
+    public function testAuthenticateECPMissingUsername()
+    {
+        $this->setExpectedException('\SimpleSAML\Error\Error', 'WRONGUSERPASS');
+
+        $state = [
+            'saml:Binding' => \SAML2\Constants::BINDING_PAOS,
+        ];
+
+        unset($_SERVER['PHP_AUTH_USER']);
+        $_SERVER['PHP_AUTH_PW'] = 'password';
+
+        $stub = $this->getMockBuilder('\SimpleSAML\Module\core\Auth\UserPassBase')
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+
+        $stub->authenticate($state);
+    }
+
+
+    public function testAuthenticateECPMissingPassword()
+    {
+        $this->setExpectedException('\SimpleSAML\Error\Error', 'WRONGUSERPASS');
+
+        $state = [
+            'saml:Binding' => \SAML2\Constants::BINDING_PAOS,
+        ];
+
+        $_SERVER['PHP_AUTH_USER'] = 'username';
+        unset($_SERVER['PHP_AUTH_PW']);
+
+        $stub = $this->getMockBuilder('\SimpleSAML\Module\core\Auth\UserPassBase')
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+
+        $stub->authenticate($state);
+    }
+
+
     public function testAuthenticateECPCallsLoginWithForcedUsername()
     {
-        $state = [];
+        $state = [
+            'saml:Binding' => \SAML2\Constants::BINDING_PAOS,
+        ];
         $attributes = [];
 
         $forcedUsername = 'forcedUsername';
 
-        $state['core:auth:username'] = 'username';
-        $password = $state['core:auth:password'] = 'password';
+        $_SERVER['PHP_AUTH_USER'] = 'username';
+        $password = $_SERVER['PHP_AUTH_PW'] = 'password';
 
         $stub = $this->getMockBuilder('\SimpleSAML\Module\core\Auth\UserPassBase')
             ->disableOriginalConstructor()

tests/modules/saml/lib/IdP/SAML2Test.php

@@ -9,43 +9,6 @@ use SimpleSAML\Test\Utils\ClearStateTestCase;
 
 class SAML2Test extends ClearStateTestCase
 {
-    public function testProcessSOAPAuthnRequest()
-    {
-        $username = $_SERVER['PHP_AUTH_USER'] = 'username';
-        $password = $_SERVER['PHP_AUTH_PW'] = 'password';
-        $state = [];
-
-        \SimpleSAML\Module\saml\IdP\SAML2::processSOAPAuthnRequest($state);
-
-        $this->assertEquals($username, $state['core:auth:username']);
-        $this->assertEquals($password, $state['core:auth:password']);
-    }
-
-    public function testProcessSOAPAuthnRequestMissingUsername()
-    {
-        $this->setExpectedException('\SimpleSAML\Error\Error', 'WRONGUSERPASS');
-
-        $_SERVER['PHP_AUTH_PW'] = 'password';
-        unset($_SERVER['PHP_AUTH_USER']);
-        $state = [];
-        Configuration::loadFromArray([
-            'baseurlpath' => 'https://idp.example.com/',
-        ], '', 'simplesaml');
-
-        \SimpleSAML\Module\saml\IdP\SAML2::processSOAPAuthnRequest($state);
-    }
-
-    public function testProcessSOAPAuthnRequestMissingPassword()
-    {
-        $this->setExpectedException('\SimpleSAML\Error\Error', 'WRONGUSERPASS');
-
-        $_SERVER['PHP_AUTH_USER'] = 'username';
-        unset($_SERVER['PHP_AUTH_PW']);
-        $state = [];
-
-        \SimpleSAML\Module\saml\IdP\SAML2::processSOAPAuthnRequest($state);
-    }
-
     /**
      * Default values for the state array expected to be generated at the start of logins
      * @var array