diff --git a/server/build.gradle b/server/build.gradle index f9a2c28993..6efd1517b1 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -112,6 +112,7 @@ dependencies { testImplementation(libraries.jsonPathAssert) testImplementation(libraries.guavaTestLib) + testImplementation(libraries.xmlUnit) implementation(libraries.commonsIo) } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityZoneConfigurationBootstrap.java b/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityZoneConfigurationBootstrap.java index baaeaf5ebf..d8d5ce2236 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityZoneConfigurationBootstrap.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/IdentityZoneConfigurationBootstrap.java @@ -59,6 +59,8 @@ public class IdentityZoneConfigurationBootstrap implements InitializingBean { private String samlSpPrivateKeyPassphrase; private String samlSpCertificate; private boolean disableSamlInResponseToCheck = false; + private boolean samlWantAssertionSigned = true; + private boolean samlRequestSigned = true; private Map> samlKeys; private String activeKeyId; @@ -89,6 +91,8 @@ public void afterPropertiesSet() throws InvalidIdentityZoneDetailsException { definition.getSamlConfig().setPrivateKey(samlSpPrivateKey); definition.getSamlConfig().setPrivateKeyPassword(samlSpPrivateKeyPassphrase); definition.getSamlConfig().setDisableInResponseToCheck(disableSamlInResponseToCheck); + definition.getSamlConfig().setWantAssertionSigned(samlWantAssertionSigned); + definition.getSamlConfig().setRequestSigned(samlRequestSigned); definition.setIdpDiscoveryEnabled(idpDiscoveryEnabled); definition.setAccountChooserEnabled(accountChooserEnabled); definition.setDefaultIdentityProvider(defaultIdentityProvider); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepository.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepository.java index 74b6f764f7..077ad8a340 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepository.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepository.java @@ -18,17 +18,14 @@ public class ConfiguratorRelyingPartyRegistrationRepository private final SamlIdentityProviderConfigurator configurator; private final KeyWithCert keyWithCert; - private final Boolean samlSignRequest; private final String samlEntityID; - public ConfiguratorRelyingPartyRegistrationRepository(Boolean samlSignRequest, - @Qualifier("samlEntityID") String samlEntityID, + public ConfiguratorRelyingPartyRegistrationRepository(@Qualifier("samlEntityID") String samlEntityID, KeyWithCert keyWithCert, SamlIdentityProviderConfigurator configurator) { Assert.notNull(configurator, "configurator cannot be null"); this.configurator = configurator; this.keyWithCert = keyWithCert; - this.samlSignRequest = samlSignRequest; this.samlEntityID = samlEntityID; } @@ -45,7 +42,7 @@ public RelyingPartyRegistration findByRegistrationId(String registrationId) { for (SamlIdentityProviderDefinition identityProviderDefinition : identityProviderDefinitions) { if (identityProviderDefinition.getIdpEntityAlias().equals(registrationId)) { return RelyingPartyRegistrationBuilder.buildRelyingPartyRegistration( - samlEntityID, identityProviderDefinition.getNameID(), samlSignRequest, + samlEntityID, identityProviderDefinition.getNameID(), keyWithCert, identityProviderDefinition.getMetaDataLocation(), registrationId); } } @@ -69,7 +66,7 @@ else if (zone.getConfig() != null && zone.getConfig().getSamlConfig() != null) { } return RelyingPartyRegistrationBuilder.buildRelyingPartyRegistration( - samlEntityID, null, samlSignRequest, + samlEntityID, null, keyWithCert, "dummy-saml-idp-metadata.xml", null, samlServiceUri); } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/RelyingPartyRegistrationBuilder.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/RelyingPartyRegistrationBuilder.java index 24bbf673e1..df405b0849 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/RelyingPartyRegistrationBuilder.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/RelyingPartyRegistrationBuilder.java @@ -25,16 +25,16 @@ private RelyingPartyRegistrationBuilder() { } public static RelyingPartyRegistration buildRelyingPartyRegistration( - String samlEntityID, String samlSpNameId, boolean samlSignRequest, + String samlEntityID, String samlSpNameId, KeyWithCert keyWithCert, String metadataLocation, String rpRegstrationId) { return buildRelyingPartyRegistration(samlEntityID, samlSpNameId, - samlSignRequest, keyWithCert, metadataLocation, rpRegstrationId, + keyWithCert, metadataLocation, rpRegstrationId, samlEntityID); } public static RelyingPartyRegistration buildRelyingPartyRegistration( - String samlEntityID, String samlSpNameId, boolean samlSignRequest, + String samlEntityID, String samlSpNameId, KeyWithCert keyWithCert, String metadataLocation, String rpRegstrationId, String samlServiceUri) { SamlIdentityProviderDefinition.MetadataLocation type = SamlIdentityProviderDefinition.getType(metadataLocation); @@ -64,9 +64,6 @@ public static RelyingPartyRegistration buildRelyingPartyRegistration( c.add(Saml2MessageBinding.REDIRECT); c.add(Saml2MessageBinding.POST); }) - .assertingPartyDetails(details -> details - .wantAuthnRequestsSigned(samlSignRequest) - ) .signingX509Credentials(cred -> cred .add(Saml2X509Credential.signing(keyWithCert.getPrivateKey(), keyWithCert.getCertificate())) ) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlMetadataEndpoint.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlMetadataEndpoint.java index 37374c341b..c4abceb7da 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlMetadataEndpoint.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlMetadataEndpoint.java @@ -1,7 +1,9 @@ package org.cloudfoundry.identity.uaa.provider.saml; import org.cloudfoundry.identity.uaa.zone.IdentityZone; +import org.cloudfoundry.identity.uaa.zone.SamlConfig; import org.cloudfoundry.identity.uaa.zone.ZoneAware; +import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.opensaml.saml.common.xml.SAMLConstants; import org.opensaml.saml.saml2.metadata.EntityDescriptor; import org.opensaml.saml.saml2.metadata.SPSSODescriptor; @@ -11,14 +13,11 @@ import org.springframework.security.saml2.provider.service.metadata.Saml2MetadataResolver; import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration; import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrationRepository; -import org.springframework.security.saml2.provider.service.web.DefaultRelyingPartyRegistrationResolver; -import org.springframework.security.saml2.provider.service.web.RelyingPartyRegistrationResolver; import org.springframework.util.Assert; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RestController; -import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; @@ -27,80 +26,73 @@ @RestController public class SamlMetadataEndpoint implements ZoneAware { public static final String DEFAULT_REGISTRATION_ID = "example"; - private static final String DEFAULT_FILE_NAME = "saml-sp.xml"; private static final String APPLICATION_XML_CHARSET_UTF_8 = "application/xml; charset=UTF-8"; - private static final String CONTENT_DISPOSITION_FORMAT = "attachment; filename=\"%s\"; filename*=UTF-8''%s"; - // @todo - this should be a Zone aware resolver - private final RelyingPartyRegistrationResolver relyingPartyRegistrationResolver; private final Saml2MetadataResolver saml2MetadataResolver; + private final IdentityZoneManager identityZoneManager; - private String fileName; - private String encodedFileName; - - private final Boolean wantAssertionSigned; private final RelyingPartyRegistrationRepository relyingPartyRegistrationRepository; public SamlMetadataEndpoint(RelyingPartyRegistrationRepository relyingPartyRegistrationRepository, - SamlConfigProps samlConfigProps) { + IdentityZoneManager identityZoneManager) { Assert.notNull(relyingPartyRegistrationRepository, "relyingPartyRegistrationRepository cannot be null"); this.relyingPartyRegistrationRepository = relyingPartyRegistrationRepository; - this.relyingPartyRegistrationResolver = new DefaultRelyingPartyRegistrationResolver(relyingPartyRegistrationRepository); + this.identityZoneManager = identityZoneManager; OpenSamlMetadataResolver resolver = new OpenSamlMetadataResolver(); this.saml2MetadataResolver = resolver; resolver.setEntityDescriptorCustomizer(new EntityDescriptorCustomizer()); - this.wantAssertionSigned = samlConfigProps.getWantAssertionSigned(); - setFileName(DEFAULT_FILE_NAME); } private class EntityDescriptorCustomizer implements Consumer { @Override public void accept(OpenSamlMetadataResolver.EntityDescriptorParameters entityDescriptorParameters) { + SamlConfig samlConfig = identityZoneManager.getCurrentIdentityZone().getConfig().getSamlConfig(); + EntityDescriptor descriptor = entityDescriptorParameters.getEntityDescriptor(); SPSSODescriptor spssodescriptor = descriptor.getSPSSODescriptor(SAMLConstants.SAML20P_NS); - spssodescriptor.setWantAssertionsSigned(wantAssertionSigned); - spssodescriptor.setAuthnRequestsSigned(entityDescriptorParameters.getRelyingPartyRegistration().getAssertingPartyDetails().getWantAuthnRequestsSigned()); + spssodescriptor.setWantAssertionsSigned(samlConfig.isWantAssertionSigned()); + spssodescriptor.setAuthnRequestsSigned(samlConfig.isRequestSigned()); } } @GetMapping(value = "/saml/metadata", produces = APPLICATION_XML_CHARSET_UTF_8) - public ResponseEntity legacyMetadataEndpoint(HttpServletRequest request) { - return metadataEndpoint(DEFAULT_REGISTRATION_ID, request); + public ResponseEntity legacyMetadataEndpoint() { + return metadataEndpoint(DEFAULT_REGISTRATION_ID); } @GetMapping(value = "/saml/metadata/{registrationId}", produces = APPLICATION_XML_CHARSET_UTF_8) - public ResponseEntity metadataEndpoint(@PathVariable String registrationId, HttpServletRequest request) { + public ResponseEntity metadataEndpoint(@PathVariable String registrationId) { RelyingPartyRegistration relyingPartyRegistration = relyingPartyRegistrationRepository.findByRegistrationId(registrationId); if (relyingPartyRegistration == null) { return ResponseEntity.status(HttpServletResponse.SC_UNAUTHORIZED).build(); } String metadata = saml2MetadataResolver.resolve(relyingPartyRegistration); - // @todo - fileName may need to be dynamic based on registrationID - String[] fileNames = retrieveZoneAwareFileNames(); + String contentDisposition = ContentDispositionFilename.getContentDisposition(retrieveZone()); return ResponseEntity.ok() - .header(HttpHeaders.CONTENT_DISPOSITION, String.format( - CONTENT_DISPOSITION_FORMAT, fileNames[0], fileNames[1])) + .header(HttpHeaders.CONTENT_DISPOSITION, contentDisposition) .body(metadata); } +} - public void setFileName(String fileName) { - encodedFileName = URLEncoder.encode(fileName, StandardCharsets.UTF_8); - this.fileName = fileName; - } +record ContentDispositionFilename(String fileName) { + private static final String CONTENT_DISPOSITION_FORMAT = "attachment; filename=\"%s\"; filename*=UTF-8''%s"; + private static final String DEFAULT_FILE_NAME = "saml-sp.xml"; - private String[] retrieveZoneAwareFileNames() { - IdentityZone zone = retrieveZone(); - String[] fileNames = new String[2]; + static ContentDispositionFilename retrieveZoneAwareContentDispositionFilename(IdentityZone zone) { if (zone.isUaa()) { - fileNames[0] = fileName; - fileNames[1] = encodedFileName; - } - else { - fileNames[0] = "saml-" + zone.getSubdomain() + "-sp.xml"; - fileNames[1] = URLEncoder.encode(fileNames[0], - StandardCharsets.UTF_8); + return new ContentDispositionFilename(DEFAULT_FILE_NAME); } - return fileNames; + String filename = "saml-%s-sp.xml".formatted(zone.getSubdomain()); + return new ContentDispositionFilename(filename); + } + + static String getContentDisposition(IdentityZone zone) { + return retrieveZoneAwareContentDispositionFilename(zone).getContentDisposition(); + } + + String getContentDisposition() { + String encodedFileName = URLEncoder.encode(fileName, StandardCharsets.UTF_8); + return CONTENT_DISPOSITION_FORMAT.formatted(fileName, encodedFileName); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfig.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfig.java index 92c4df5b7b..7baad24488 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfig.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfig.java @@ -31,21 +31,17 @@ public class SamlRelyingPartyRegistrationRepositoryConfig { private final SamlConfigProps samlConfigProps; private final BootstrapSamlIdentityProviderData bootstrapSamlIdentityProviderData; private final String samlSpNameID; - private final Boolean samlSignRequest; public SamlRelyingPartyRegistrationRepositoryConfig(@Qualifier("samlEntityID") String samlEntityID, SamlConfigProps samlConfigProps, BootstrapSamlIdentityProviderData bootstrapSamlIdentityProviderData, @Value("${login.saml.nameID:urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified}") - String samlSpNameID, - @Value("${login.saml.signRequest:true}") - Boolean samlSignRequest + String samlSpNameID ) { this.samlEntityID = samlEntityID; this.samlConfigProps = samlConfigProps; this.bootstrapSamlIdentityProviderData = bootstrapSamlIdentityProviderData; this.samlSpNameID = samlSpNameID; - this.samlSignRequest = samlSignRequest; } @Autowired @@ -69,20 +65,20 @@ RelyingPartyRegistrationRepository relyingPartyRegistrationRepository(SamlIdenti // even when there are no SAML IDPs configured. // See relevant issue: https://github.com/spring-projects/spring-security/issues/11369 RelyingPartyRegistration defaultRelyingPartyRegistration = RelyingPartyRegistrationBuilder.buildRelyingPartyRegistration( - samlEntityID, samlSpNameID, samlSignRequest, keyWithCert, CLASSPATH_DUMMY_SAML_IDP_METADATA_XML, DEFAULT_REGISTRATION_ID); + samlEntityID, samlSpNameID, keyWithCert, CLASSPATH_DUMMY_SAML_IDP_METADATA_XML, DEFAULT_REGISTRATION_ID); relyingPartyRegistrations.add(defaultRelyingPartyRegistration); for (SamlIdentityProviderDefinition samlIdentityProviderDefinition : bootstrapSamlIdentityProviderData.getIdentityProviderDefinitions()) { relyingPartyRegistrations.add( RelyingPartyRegistrationBuilder.buildRelyingPartyRegistration( - samlEntityID, samlSpNameID, samlSignRequest, keyWithCert, + samlEntityID, samlSpNameID, keyWithCert, samlIdentityProviderDefinition.getMetaDataLocation(), samlIdentityProviderDefinition.getIdpEntityAlias()) ); } InMemoryRelyingPartyRegistrationRepository bootstrapRepo = new InMemoryRelyingPartyRegistrationRepository(relyingPartyRegistrations); - ConfiguratorRelyingPartyRegistrationRepository configuratorRepo = new ConfiguratorRelyingPartyRegistrationRepository(samlSignRequest, samlEntityID, keyWithCert, samlIdentityProviderConfigurator); + ConfiguratorRelyingPartyRegistrationRepository configuratorRepo = new ConfiguratorRelyingPartyRegistrationRepository(samlEntityID, keyWithCert, samlIdentityProviderConfigurator); return new DelegatingRelyingPartyRegistrationRepository(bootstrapRepo, configuratorRepo); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java index c3a2e5699b..8a51ef7646 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityZoneConfigurationBootstrapTests.java @@ -132,15 +132,20 @@ void keyIdNullException() { } @Test - void defaultSamlKeys() throws Exception { + void samlKeysAndSigningConfigs() throws Exception { bootstrap.setSamlSpPrivateKey(SamlTestUtils.PROVIDER_PRIVATE_KEY); bootstrap.setSamlSpCertificate(SamlTestUtils.PROVIDER_CERTIFICATE); bootstrap.setSamlSpPrivateKeyPassphrase(SamlTestUtils.PROVIDER_PRIVATE_KEY_PASSWORD); + bootstrap.setSamlWantAssertionSigned(false); + bootstrap.setSamlRequestSigned(false); bootstrap.afterPropertiesSet(); + IdentityZone uaa = provisioning.retrieve(IdentityZone.getUaaZoneId()); assertThat(uaa.getConfig().getSamlConfig().getPrivateKey()).isEqualTo(SamlTestUtils.PROVIDER_PRIVATE_KEY); assertThat(uaa.getConfig().getSamlConfig().getPrivateKeyPassword()).isEqualTo(SamlTestUtils.PROVIDER_PRIVATE_KEY_PASSWORD); assertThat(uaa.getConfig().getSamlConfig().getCertificate()).isEqualTo(SamlTestUtils.PROVIDER_CERTIFICATE); + assertThat(uaa.getConfig().getSamlConfig().isWantAssertionSigned()).isEqualTo(false); + assertThat(uaa.getConfig().getSamlConfig().isRequestSigned()).isEqualTo(false); } @Test @@ -253,7 +258,6 @@ void logoutRedirect() throws Exception { assertThat(config.getLinks().getLogout().isDisableRedirectParameter()).isFalse(); } - @Test void testPrompts() throws Exception { List prompts = Arrays.asList( diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepositoryTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepositoryTest.java index e1eef21bbb..a14f3a6ce3 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepositoryTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepositoryTest.java @@ -46,14 +46,14 @@ class ConfiguratorRelyingPartyRegistrationRepositoryTest { @BeforeEach void setUp() { - repository = new ConfiguratorRelyingPartyRegistrationRepository(true, ENTITY_ID, mockKeyWithCert, + repository = new ConfiguratorRelyingPartyRegistrationRepository(ENTITY_ID, mockKeyWithCert, mockConfigurator); } @Test void constructorWithNullConfiguratorThrows() { assertThatThrownBy(() -> new ConfiguratorRelyingPartyRegistrationRepository( - true, ENTITY_ID, mockKeyWithCert, null) + ENTITY_ID, mockKeyWithCert, null) ).isInstanceOf(IllegalArgumentException.class); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/RelyingPartyRegistrationBuilderTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/RelyingPartyRegistrationBuilderTest.java index db65a2524c..54b8bc1461 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/RelyingPartyRegistrationBuilderTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/RelyingPartyRegistrationBuilderTest.java @@ -31,7 +31,6 @@ class RelyingPartyRegistrationBuilderTest { private static final String ENTITY_ID = "entityId"; private static final String NAME_ID = "nameIdFormat"; private static final String REGISTRATION_ID = "registrationId"; - private static final boolean SIGN_REQUEST = true; @Mock private KeyWithCert mockKeyWithCert; @@ -42,7 +41,7 @@ void buildsRelyingPartyRegistrationFromLocation() { when(mockKeyWithCert.getPrivateKey()).thenReturn(mock(PrivateKey.class)); RelyingPartyRegistration registration = RelyingPartyRegistrationBuilder - .buildRelyingPartyRegistration(ENTITY_ID, NAME_ID, SIGN_REQUEST, mockKeyWithCert, "saml-sample-metadata.xml", REGISTRATION_ID); + .buildRelyingPartyRegistration(ENTITY_ID, NAME_ID, mockKeyWithCert, "saml-sample-metadata.xml", REGISTRATION_ID); assertThat(registration) .returns(REGISTRATION_ID, RelyingPartyRegistration::getRegistrationId) .returns(ENTITY_ID, RelyingPartyRegistration::getEntityId) @@ -62,7 +61,7 @@ void buildsRelyingPartyRegistrationFromXML() { String metadataXml = loadResouceAsString("saml-sample-metadata.xml"); RelyingPartyRegistration registration = RelyingPartyRegistrationBuilder - .buildRelyingPartyRegistration(ENTITY_ID, NAME_ID, SIGN_REQUEST, mockKeyWithCert, metadataXml, REGISTRATION_ID); + .buildRelyingPartyRegistration(ENTITY_ID, NAME_ID, mockKeyWithCert, metadataXml, REGISTRATION_ID); assertThat(registration) .returns(REGISTRATION_ID, RelyingPartyRegistration::getRegistrationId) @@ -81,7 +80,7 @@ void failsWithInvalidXML() { String metadataXml = "\ninvalid xml"; assertThatThrownBy(() -> RelyingPartyRegistrationBuilder.buildRelyingPartyRegistration(ENTITY_ID, NAME_ID, - SIGN_REQUEST, mockKeyWithCert, metadataXml, REGISTRATION_ID)) + mockKeyWithCert, metadataXml, REGISTRATION_ID)) .isInstanceOf(Saml2Exception.class) .hasMessageContaining("Unsupported element"); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlMetadataEndpointTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlMetadataEndpointTest.java new file mode 100644 index 0000000000..64b63def06 --- /dev/null +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlMetadataEndpointTest.java @@ -0,0 +1,105 @@ +package org.cloudfoundry.identity.uaa.provider.saml; + +import org.cloudfoundry.identity.uaa.zone.IdentityZone; +import org.cloudfoundry.identity.uaa.zone.IdentityZoneConfiguration; +import org.cloudfoundry.identity.uaa.zone.SamlConfig; +import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpHeaders; +import org.springframework.http.ResponseEntity; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration; +import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrationRepository; +import org.springframework.security.saml2.provider.service.registration.Saml2MessageBinding; +import org.xmlunit.assertj.XmlAssert; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.cloudfoundry.identity.uaa.provider.saml.Saml2TestUtils.xmlNamespaces; +import static org.cloudfoundry.identity.uaa.provider.saml.TestSaml2X509Credentials.relyingPartySigningCredential; +import static org.cloudfoundry.identity.uaa.provider.saml.TestSaml2X509Credentials.relyingPartyVerifyingCredential; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class SamlMetadataEndpointTest { + private static final String ASSERTION_CONSUMER_SERVICE = "https://acsl"; + private static final String REGISTRATION_ID = "regId"; + private static final String ENTITY_ID = "entityId"; + + SamlMetadataEndpoint endpoint; + + @Mock + RelyingPartyRegistrationRepository repository; + @Mock + IdentityZoneManager identityZoneManager; + @Mock + RelyingPartyRegistration registration; + @Mock + IdentityZone identityZone; + @Mock + IdentityZoneConfiguration identityZoneConfiguration; + @Mock + SamlConfig samlConfig; + + @BeforeEach + void setUp() { + endpoint = spy(new SamlMetadataEndpoint(repository, identityZoneManager)); + when(repository.findByRegistrationId(REGISTRATION_ID)).thenReturn(registration); + when(registration.getEntityId()).thenReturn(ENTITY_ID); + when(registration.getSigningX509Credentials()).thenReturn(List.of(relyingPartySigningCredential())); + when(registration.getDecryptionX509Credentials()).thenReturn(List.of(relyingPartyVerifyingCredential())); + when(registration.getAssertionConsumerServiceBinding()).thenReturn(Saml2MessageBinding.REDIRECT); + when(registration.getAssertionConsumerServiceLocation()).thenReturn(ASSERTION_CONSUMER_SERVICE); + when(identityZoneManager.getCurrentIdentityZone()).thenReturn(identityZone); + when(identityZone.getConfig()).thenReturn(identityZoneConfiguration); + when(identityZoneConfiguration.getSamlConfig()).thenReturn(samlConfig); + } + + @Test + void testDefaultFileName() { + ResponseEntity response = endpoint.metadataEndpoint(REGISTRATION_ID); + assertThat(response.getHeaders().getFirst(HttpHeaders.CONTENT_DISPOSITION)) + .isEqualTo("attachment; filename=\"saml-sp.xml\"; filename*=UTF-8''saml-sp.xml"); + } + + @Test + void testZonedFileName() { + when(identityZone.isUaa()).thenReturn(false); + when(identityZone.getSubdomain()).thenReturn("testzone1"); + when(endpoint.retrieveZone()).thenReturn(identityZone); + + ResponseEntity response = endpoint.metadataEndpoint(REGISTRATION_ID); + assertThat(response.getHeaders().getFirst(HttpHeaders.CONTENT_DISPOSITION)) + .isEqualTo("attachment; filename=\"saml-testzone1-sp.xml\"; filename*=UTF-8''saml-testzone1-sp.xml"); + } + + @Test + void testDefaultMetadataXml() { + when(samlConfig.isWantAssertionSigned()).thenReturn(true); + when(samlConfig.isRequestSigned()).thenReturn(true); + + ResponseEntity response = endpoint.metadataEndpoint(REGISTRATION_ID); + XmlAssert xmlAssert =XmlAssert.assertThat(response.getBody()).withNamespaceContext(xmlNamespaces()); + xmlAssert.valueByXPath("//md:EntityDescriptor/@entityID").isEqualTo(ENTITY_ID); + xmlAssert.valueByXPath("//md:SPSSODescriptor/@AuthnRequestsSigned").isEqualTo(true); + xmlAssert.valueByXPath("//md:SPSSODescriptor/@WantAssertionsSigned").isEqualTo(true); + xmlAssert.valueByXPath("//md:AssertionConsumerService/@Location").isEqualTo(ASSERTION_CONSUMER_SERVICE); + } + + @Test + void testDefaultMetadataXml_alternateValues() { + when(samlConfig.isWantAssertionSigned()).thenReturn(false); + when(samlConfig.isRequestSigned()).thenReturn(false); + + ResponseEntity response = endpoint.metadataEndpoint(REGISTRATION_ID); + XmlAssert xmlAssert =XmlAssert.assertThat(response.getBody()).withNamespaceContext(xmlNamespaces()); + xmlAssert.valueByXPath("//md:SPSSODescriptor/@AuthnRequestsSigned").isEqualTo(false); + xmlAssert.valueByXPath("//md:SPSSODescriptor/@WantAssertionsSigned").isEqualTo(false); + } +} diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfigTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfigTest.java index 576868ea12..02a191ca7e 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfigTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfigTest.java @@ -26,7 +26,6 @@ class SamlRelyingPartyRegistrationRepositoryConfigTest { private static final String CERT = KeyWithCertTest.goodCert; private static final String ENTITY_ID = "entityId"; private static final String NAME_ID = "nameIdFormat"; - private static final boolean SIGN_REQUEST = true; @Mock SamlConfigProps samlConfigProps; @@ -55,14 +54,14 @@ public void setup() { @Test void relyingPartyRegistrationRepository() throws CertificateException { - SamlRelyingPartyRegistrationRepositoryConfig config = new SamlRelyingPartyRegistrationRepositoryConfig(ENTITY_ID, samlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID, SIGN_REQUEST); + SamlRelyingPartyRegistrationRepositoryConfig config = new SamlRelyingPartyRegistrationRepositoryConfig(ENTITY_ID, samlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID); RelyingPartyRegistrationRepository repository = config.relyingPartyRegistrationRepository(samlIdentityProviderConfigurator); assertThat(repository).isNotNull(); } @Test void relyingPartyRegistrationResolver() throws CertificateException { - SamlRelyingPartyRegistrationRepositoryConfig config = new SamlRelyingPartyRegistrationRepositoryConfig(ENTITY_ID, samlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID, SIGN_REQUEST); + SamlRelyingPartyRegistrationRepositoryConfig config = new SamlRelyingPartyRegistrationRepositoryConfig(ENTITY_ID, samlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID); RelyingPartyRegistrationRepository repository = config.relyingPartyRegistrationRepository(samlIdentityProviderConfigurator); RelyingPartyRegistrationResolver resolver = config.relyingPartyRegistrationResolver(repository); @@ -71,7 +70,7 @@ void relyingPartyRegistrationResolver() throws CertificateException { @Test void buildsRegistrationForExample() throws CertificateException { - SamlRelyingPartyRegistrationRepositoryConfig config = new SamlRelyingPartyRegistrationRepositoryConfig(ENTITY_ID, samlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID, SIGN_REQUEST); + SamlRelyingPartyRegistrationRepositoryConfig config = new SamlRelyingPartyRegistrationRepositoryConfig(ENTITY_ID, samlConfigProps, bootstrapSamlIdentityProviderData, NAME_ID); RelyingPartyRegistrationRepository repository = config.relyingPartyRegistrationRepository(samlIdentityProviderConfigurator); RelyingPartyRegistration registration = repository.findByRegistrationId("example"); assertThat(registration) diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/ZoneAwareMetadataGeneratorTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/ZoneAwareMetadataGeneratorTests.java index 0717837b59..6e6f6ed171 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/ZoneAwareMetadataGeneratorTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/ZoneAwareMetadataGeneratorTests.java @@ -89,25 +89,6 @@ void tearDown() { IdentityZoneHolder.clear(); } - @Test - @Disabled("SAML test doesn't compile") - void testRequestAndWantAssertionSignedInAnotherZone() { -// generator.setRequestSigned(true); -// generator.setWantAssertionSigned(true); -// assertTrue(generator.isRequestSigned()); -// assertTrue(generator.isWantAssertionSigned()); -// -// generator.setRequestSigned(false); -// generator.setWantAssertionSigned(false); -// assertFalse(generator.isRequestSigned()); -// assertFalse(generator.isWantAssertionSigned()); -// -// IdentityZoneHolder.set(otherZone); -// -// assertTrue(generator.isRequestSigned()); -// assertTrue(generator.isWantAssertionSigned()); - } - @Test @Disabled("SAML test doesn't compile") void testMetadataContainsSamlBearerGrantEndpoint() throws Exception { diff --git a/uaa/src/main/webapp/WEB-INF/spring-servlet.xml b/uaa/src/main/webapp/WEB-INF/spring-servlet.xml index 6dcfe30e2b..0384620bbf 100755 --- a/uaa/src/main/webapp/WEB-INF/spring-servlet.xml +++ b/uaa/src/main/webapp/WEB-INF/spring-servlet.xml @@ -493,6 +493,8 @@ @config['login']['saml']==null ? null : @config['login']['saml']['keys']}"/> + + diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/SamlLoginIT.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/SamlLoginIT.java index f74d03332e..3bba2fb122 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/SamlLoginIT.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/SamlLoginIT.java @@ -50,7 +50,6 @@ import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZoneConfiguration; import org.flywaydb.core.internal.util.StringUtils; -import org.hamcrest.Matchers; import org.junit.Rule; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; @@ -76,6 +75,7 @@ import org.springframework.util.FileCopyUtils; import org.springframework.web.client.RestOperations; import org.springframework.web.client.RestTemplate; +import org.xmlunit.assertj.XmlAssert; import java.io.IOException; import java.io.InputStreamReader; @@ -107,10 +107,9 @@ import static org.cloudfoundry.identity.uaa.provider.ExternalIdentityProviderDefinition.EMAIL_ATTRIBUTE_NAME; import static org.cloudfoundry.identity.uaa.provider.ExternalIdentityProviderDefinition.GROUP_ATTRIBUTE_NAME; import static org.cloudfoundry.identity.uaa.provider.ExternalIdentityProviderDefinition.USER_ATTRIBUTE_PREFIX; +import static org.cloudfoundry.identity.uaa.provider.saml.Saml2TestUtils.xmlNamespaces; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.springframework.http.HttpMethod.GET; import static org.springframework.http.HttpMethod.POST; import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; @@ -157,7 +156,9 @@ public class SamlLoginIT { @BeforeAll static void checkZoneDNSSupport() { - assertTrue(doesSupportZoneDNS(), "Expected testzone1.localhost, testzone2.localhost, testzone3.localhost, testzone4.localhost to resolve to 127.0.0.1"); + assertThat(doesSupportZoneDNS()) + .as("Expected testzone1.localhost, testzone2.localhost, testzone3.localhost, testzone4.localhost to resolve to 127.0.0.1") + .isTrue(); } public static String getValidRandomIDPMetaData() { @@ -216,23 +217,27 @@ void samlSPMetadata() { "%s/saml/metadata".formatted(baseUrl), String.class); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); String metadataXml = response.getBody(); + XmlAssert xmlAssert = XmlAssert.assertThat(metadataXml).withNamespaceContext(xmlNamespaces()); // The SAML SP metadata should match the following UAA configs: // login.entityID - assertThat(metadataXml).contains("entityID=\"cloudfoundry-saml-login\"") - // TODO: Are DigestMethod and SignatureMethod needed? - // login.saml.signatureAlgorithm - //.contains("") - //.contains("") - // login.saml.signRequest - .contains("AuthnRequestsSigned=\"true\"") - // login.saml.wantAssertionSigned - .contains("WantAssertionsSigned=\"true\"") - // login.saml.nameID - .contains("urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified"); - - assertEquals("saml-sp.xml", - response.getHeaders().getContentDisposition().getFilename()); + xmlAssert.valueByXPath("//md:EntityDescriptor/@entityID").isEqualTo("cloudfoundry-saml-login"); + // login.saml.signRequest + xmlAssert.valueByXPath("//md:EntityDescriptor/md:SPSSODescriptor/@AuthnRequestsSigned").isEqualTo(true); + // login.saml.wantAssertionSigned + xmlAssert.valueByXPath("//md:EntityDescriptor/md:SPSSODescriptor/@WantAssertionsSigned").isEqualTo(true); + // TODO the AssertionConsumerService location needs to be a valid URL (currently it's: {baseUrl}/saml/....) + xmlAssert.valueByXPath("//md:EntityDescriptor/md:SPSSODescriptor/md:AssertionConsumerService/@Location").contains("/saml/SSO/alias/cloudfoundry-saml-login"); + +// assertThat(metadataXml).contains("entityID=\"cloudfoundry-saml-login\"") +// // TODO: Are DigestMethod and SignatureMethod needed? +// // login.saml.signatureAlgorithm +// //.contains("") +// //.contains("") +// // login.saml.nameID +// .contains("urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified"); + + assertThat(response.getHeaders().getContentDisposition().getFilename()).isEqualTo("saml-sp.xml"); } @Test @@ -252,6 +257,8 @@ void samlSPMetadataForZone() { IdentityZoneConfiguration config = new IdentityZoneConfiguration(); config.getCorsPolicy().getDefaultConfiguration().setAllowedMethods(List.of(GET.toString(), POST.toString())); config.getSamlConfig().setEntityID(zoneId + "-saml-login"); + config.getSamlConfig().setWantAssertionSigned(false); + config.getSamlConfig().setRequestSigned(false); IntegrationTestUtils.createZoneOrUpdateSubdomain(identityClient, baseUrl, zoneId, zoneId, config); RestTemplate request = new RestTemplate(); @@ -259,17 +266,20 @@ void samlSPMetadataForZone() { zoneUrl + "/saml/metadata", String.class); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); String metadataXml = response.getBody(); + XmlAssert xmlAssert = XmlAssert.assertThat(metadataXml).withNamespaceContext(xmlNamespaces()); // The SAML SP metadata should match the following UAA configs: // login.entityID - assertThat(metadataXml).contains("entityID=\"" + zoneId + "-saml-login\"") - .contains("AuthnRequestsSigned=\"true\"") - .contains("WantAssertionsSigned=\"true\"") - // TODO: Improve this check - .contains("/saml/SSO/alias/" + zoneId + ".cloudfoundry-saml-login"); - - assertEquals("saml-" + zoneId + "-sp.xml", - response.getHeaders().getContentDisposition().getFilename()); + xmlAssert.valueByXPath("//md:EntityDescriptor/@entityID").isEqualTo("testzone1-saml-login"); + // in default zone, determined by UAA.yml field: login.saml.signRequest; in other zone, determined by zone config field: config.samlConfig.requestSigned + xmlAssert.valueByXPath("//md:EntityDescriptor/md:SPSSODescriptor/@AuthnRequestsSigned").isEqualTo(false); + + // in default zone, determined by UAA.yml field: login.saml.wantAssertionSigned; in other zone, determined by zone config field: config.samlConfig.wantAssertionSigned + xmlAssert.valueByXPath("//md:EntityDescriptor/md:SPSSODescriptor/@WantAssertionsSigned").isEqualTo(false); + // TODO the AssertionConsumerService location needs to be a valid URL (currently it's: {baseUrl}/saml/....) + xmlAssert.valueByXPath("//md:EntityDescriptor/md:SPSSODescriptor/md:AssertionConsumerService/@Location").contains("/saml/SSO/alias/testzone1.cloudfoundry-saml-login"); + + assertThat(response.getHeaders().getContentDisposition().getFilename()).isEqualTo("saml-testzone1-sp.xml"); } @Test @@ -491,7 +501,7 @@ void singleLogoutWithNoLogoutUrlOnIDPWithLogoutRedirect() { IntegrationTestUtils.createOrUpdateProvider(zoneAdminToken, baseUrl, provider); LoginPage loginPage = LoginPage.go(webDriver, zoneUrl); - loginPage.validateTitle(Matchers.containsString("testzone2")); + loginPage.validateTitle(containsString("testzone2")); loginPage.clickSamlLink_goesToSamlLoginPage("simplesamlphp") .login_goesToHomePage(testAccounts.getUserName(), testAccounts.getPassword()); diff --git a/uaa/src/test/resources/integration_test_properties.yml b/uaa/src/test/resources/integration_test_properties.yml index 489d421308..829c7b056d 100644 --- a/uaa/src/test/resources/integration_test_properties.yml +++ b/uaa/src/test/resources/integration_test_properties.yml @@ -107,7 +107,7 @@ login: #Local/SP metadata - requests signed signRequest: true #Local/SP metadata - want incoming assertions signed - #wantAssertionSigned: true + wantAssertionSigned: true #Algorithm for SAML signatures. Defaults to SHA1. Accepts SHA1, SHA256, SHA512 #signatureAlgorithm: SHA256 providers: