diff --git a/modules/core/lib/Auth/UserPassBase.php b/modules/core/lib/Auth/UserPassBase.php index 5f0ee4ffa33040da24c254c4f1ffe34fa691d031..8473344666aa5e02a8df33ef1cde898c586fdfb8 100644 --- a/modules/core/lib/Auth/UserPassBase.php +++ b/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']; diff --git a/modules/saml/lib/IdP/SAML2.php b/modules/saml/lib/IdP/SAML2.php index f530451e0f861ec653a0a356e9f8de7827233e1b..1c787412d6a77bbc79ed880cbb756ae255684bdb 100644 --- a/modules/saml/lib/IdP/SAML2.php +++ b/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. * diff --git a/tests/modules/core/lib/Auth/UserPassBaseTest.php b/tests/modules/core/lib/Auth/UserPassBaseTest.php index cd5a328acad88345e76d28676b4ab543697743cb..f477ad2abbf4b8310e9998f8e57f1c1db264060a 100644 --- a/tests/modules/core/lib/Auth/UserPassBaseTest.php +++ b/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() diff --git a/tests/modules/saml/lib/IdP/SAML2Test.php b/tests/modules/saml/lib/IdP/SAML2Test.php index dbd026097c9268508c734ee8a5ca467fd7fe9738..607234a172c9b66fb2a585fba0d952efd200e253 100644 --- a/tests/modules/saml/lib/IdP/SAML2Test.php +++ b/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