diff --git a/modules/saml/docs/sp.md b/modules/saml/docs/sp.md index 9fd231bbe9eabdc6eb8388375d687455f84767e8..efa40a0f6ee4817afbb95dd7a650ad7b6a0a09d9 100644 --- a/modules/saml/docs/sp.md +++ b/modules/saml/docs/sp.md @@ -113,7 +113,9 @@ Options `AssertionConsumerService` : List of Assertion Consumer Services in the generated metadata. Specified in the array of arrays format as seen in the [Metadata endpoints](./simplesamlphp-metadata-endpoints) - documentation. + documentation. Note that this list is taken at face value, so it's not useful to list + anything here that the SP auth source does not actually support (unless the URLs point + externally). `AssertionConsumerServiceIndex` : The Assertion Consumer Service Index to be used in the AuthnRequest in place of the Assertion diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php index bddeac4d3633c41835e23b93424a1cdebe3f0ff0..1a6f8db57369e11c51f3f82b5b31fb8136471f99 100644 --- a/modules/saml/lib/Auth/Source/SP.php +++ b/modules/saml/lib/Auth/Source/SP.php @@ -62,7 +62,7 @@ class SP extends \SimpleSAML\Auth\Source * * @var string[] */ - private array $protocols = []; + private array $protocols = [Constants::NS_SAMLP]; /** @@ -351,18 +351,12 @@ class SP extends \SimpleSAML\Auth\Source 'Binding' => Constants::BINDING_HTTP_POST, 'Location' => Module::getModuleURL('saml/sp/saml2-acs.php/' . $this->getAuthId()), ]; - if (!in_array(Constants::NS_SAMLP, $this->protocols, true)) { - $this->protocols[] = Constants::NS_SAMLP; - } break; case Constants::BINDING_HTTP_ARTIFACT: $acs = [ 'Binding' => Constants::BINDING_HTTP_ARTIFACT, 'Location' => Module::getModuleURL('saml/sp/saml2-acs.php/' . $this->getAuthId()), ]; - if (!in_array(Constants::NS_SAMLP, $this->protocols, true)) { - $this->protocols[] = Constants::NS_SAMLP; - } break; case Constants::BINDING_HOK_SSO: $acs = [ @@ -370,9 +364,6 @@ class SP extends \SimpleSAML\Auth\Source 'Location' => Module::getModuleURL('saml/sp/saml2-acs.php/' . $this->getAuthId()), 'hoksso:ProtocolBinding' => Constants::BINDING_HTTP_REDIRECT, ]; - if (!in_array(Constants::NS_SAMLP, $this->protocols, true)) { - $this->protocols[] = Constants::NS_SAMLP; - } break; default: Logger::warning('Unknown acs.Binding value specified, ignoring: ' . $service); diff --git a/tests/modules/saml/lib/Auth/Source/SPTest.php b/tests/modules/saml/lib/Auth/Source/SPTest.php index 883fa0096c5de6405038f9ec5c5c7a7205e9f2cc..6b380455143ff250ef84e730489c9083a10527fd 100644 --- a/tests/modules/saml/lib/Auth/Source/SPTest.php +++ b/tests/modules/saml/lib/Auth/Source/SPTest.php @@ -1198,4 +1198,54 @@ class SPTest extends ClearStateTestCase $this->assertEquals('new_', $md['keys'][0]['prefix']); $this->assertEquals('', $md['keys'][1]['prefix']); } + + /** + * We only support SAML 2.0 as a protocol with this auth source + */ + public function testSupportedProtocolsReturnsSAML20Only(): void + { + $spId = 'myhosted-sp'; + $info = ['AuthId' => $spId]; + $config = []; + $as = new SpTester($info, $config); + + $md = $as->getHostedMetadata(); + $protocols = $as->getSupportedProtocols(); + $this->assertIsArray($protocols); + $this->assertCount(1, $protocols); + $this->assertEquals('urn:oasis:names:tc:SAML:2.0:protocol', $protocols[0]); + } + + /** + * We only support SAML 2.0 as a protocol with this auth source + */ + public function testSAML11BindingsDoesNotInfluenceProtocolsSupported(): void + { + $spId = 'myhosted-sp'; + $info = ['AuthId' => $spId]; + $config = [ + 'AssertionConsumerService' => [ + [ + 'index' => 1, + 'isDefault' => TRUE, + 'Location' => 'https://sp.example.org/ACS', + 'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST', + ], + [ + 'index' => 17, + 'Location' => 'https://sp.example.org/ACS', + 'Binding' => 'urn:oasis:names:tc:SAML:1.0:profiles:browser-post', + ], + ], + ]; + $as = new SpTester($info, $config); + + $md = $as->getHostedMetadata(); + + $protocols = $as->getSupportedProtocols(); + $this->assertIsArray($protocols); + $this->assertCount(1, $protocols); + $this->assertEquals('urn:oasis:names:tc:SAML:2.0:protocol', $protocols[0]); + } + }