Fix: skipSslValidation ignored when fetching SAML IDP metadata during login

server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepository.java

@@ -89,7 +89,7 @@ private RelyingPartyRegistration createRelyingPartyRegistration(String registrat
                 .samlEntityID(zonedSamlEntityID)
                 .samlSpNameId(nameID)
                 .keys(keyWithCerts)
-                .metadataLocation(identityProviderDefinition.getMetaDataLocation())
+                .metadataLocation(configurator.resolveMetadataXml(identityProviderDefinition))
                 .rpRegistrationId(registrationId)
                 .samlSpAlias(zonedSamlEntityIDAlias)
                 .requestSigned(requestSigned)

server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfigurator.java

@@ -147,13 +147,25 @@ protected String adjustURIForPort(String uri) throws URISyntaxException {
     }
 
     protected RelyingPartyRegistration configureURLMetadata(SamlIdentityProviderDefinition def) {
+        SamlIdentityProviderDefinition resolved = def.clone();
+        resolved.setMetaDataLocation(resolveMetadataXml(def));
+        return configureXMLMetadata(resolved);
+    }
+
+    /**
+     * Resolves a SAML IDP definition's metadata to its literal XML content. If the
+     * definition's metadata is a URL, it is fetched via the {@code skipSslValidation}-aware
+     * {@link FixedHttpMetaDataProvider} (the same trust/cache behavior used to validate an
+     * IDP on creation); otherwise the already-inline XML is returned unchanged.
+     */
+    public String resolveMetadataXml(SamlIdentityProviderDefinition def) {
+        if (def.getType() != SamlIdentityProviderDefinition.MetadataLocation.URL) {
+            return def.getMetaDataLocation();
+        }
         try {
-            def = def.clone();
             String adjustedMetadataURIForPort = adjustURIForPort(def.getMetaDataLocation());
             byte[] metadata = fixedHttpMetaDataProvider.fetchMetadata(adjustedMetadataURIForPort, def.isSkipSslValidation());
-
-            def.setMetaDataLocation(new String(metadata, StandardCharsets.UTF_8));
-            return configureXMLMetadata(def);
+            return new String(metadata, StandardCharsets.UTF_8);
         } catch (URISyntaxException e) {
             throw new IllegalStateException("Invalid socket factory(invalid URI):" + def.getMetaDataLocation(), e);
         }

server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfig.java

@@ -82,7 +82,7 @@ RelyingPartyRegistrationRepository relyingPartyRegistrationRepository(SamlIdenti
                     .samlEntityID(samlEntityID)
                     .samlSpNameId(samlSpNameID)
                     .keys(defaultKeysWithCerts)
-                    .metadataLocation(samlIdentityProviderDefinition.getMetaDataLocation())
+                    .metadataLocation(samlIdentityProviderConfigurator.resolveMetadataXml(samlIdentityProviderDefinition))
                     .rpRegistrationId(samlIdentityProviderDefinition.getIdpEntityAlias())
                     .samlSpAlias(uaaWideSamlEntityIDAlias)
                     .requestSigned(samlConfigProps.getSignRequest())

server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepositoryTest.java

@@ -1,10 +1,16 @@
 package org.cloudfoundry.identity.uaa.provider.saml;
 
+import com.sun.net.httpserver.HttpsServer;
+import org.cloudfoundry.identity.uaa.impl.config.RestTemplateConfig;
+import org.cloudfoundry.identity.uaa.provider.IdentityProvider;
+import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning;
 import org.cloudfoundry.identity.uaa.provider.SamlIdentityProviderDefinition;
+import org.cloudfoundry.identity.uaa.test.network.NetworkTestUtils;
 import org.cloudfoundry.identity.uaa.zone.IdentityZone;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneConfiguration;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
 import org.cloudfoundry.identity.uaa.zone.SamlConfig;
+import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManagerImpl;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -14,17 +20,21 @@
 import org.springframework.core.io.DefaultResourceLoader;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceLoader;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.MediaType;
 import org.springframework.security.saml2.Saml2Exception;
 import org.springframework.security.saml2.core.Saml2X509Credential;
 import org.springframework.security.saml2.provider.service.registration.AssertingPartyMetadata;
 import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration;
 import org.springframework.util.FileCopyUtils;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.io.UncheckedIOException;
 import java.util.Arrays;
+import java.util.Date;
 import java.util.List;
 import java.util.Map;
 
@@ -37,6 +47,8 @@
 import static org.cloudfoundry.identity.uaa.provider.saml.TestCredentialObjects.samlKey2;
 import static org.cloudfoundry.identity.uaa.provider.saml.TestCredentialObjects.x509Certificate1;
 import static org.cloudfoundry.identity.uaa.provider.saml.TestCredentialObjects.x509Certificate2;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
@@ -82,6 +94,10 @@ static void beforeAll() {
     @BeforeEach
     void beforeEach() {
         repository = spy(new ConfiguratorRelyingPartyRegistrationRepository(ENTITY_ID, ENTITY_ID_ALIAS, configurator, List.of(), DEFAULT_NAME_ID));
+        // resolveMetadataXml() is a no-op passthrough for non-URL (already-inline) metadata,
+        // which is what every test using the mocked `configurator` exercises.
+        lenient().when(configurator.resolveMetadataXml(any(SamlIdentityProviderDefinition.class)))
+                .thenAnswer(invocation -> invocation.getArgument(0, SamlIdentityProviderDefinition.class).getMetaDataLocation());
     }
 
     @Test
@@ -337,6 +353,56 @@ void throwsWhenInvalidMetadataXmlIsStored() {
                 .hasMessageContaining(REGISTRATION_ID);
     }
 
+    @Test
+    void findByRegistrationIdHonorsSkipSslValidationForUrlMetadata() throws Exception {
+        SamlConfiguration.setupOpenSaml();
+
+        File keystore = NetworkTestUtils.getKeystore(new Date(), 10);
+        String metadataXml = loadResouceAsString("saml-sample-metadata.xml");
+        HttpHeaders headers = new HttpHeaders();
+        headers.setContentType(MediaType.APPLICATION_XML);
+        NetworkTestUtils.SimpleHttpResponseHandler handler = new NetworkTestUtils.SimpleHttpResponseHandler(headers, metadataXml);
+        HttpsServer httpsServer = NetworkTestUtils.startHttpsServer(keystore, NetworkTestUtils.keyPass, handler);
+        try {
+            String metadataUrl = "https://localhost:" + httpsServer.getAddress().getPort() + "/metadata";
+
+            // Real (non-mocked) configurator/metadata-provider so the SAML IDP's untrusted,
+            // self-signed certificate is genuinely exercised over the wire, the same way it
+            // would be for the admin validate path (SamlIdentityProviderConfigurator.configureURLMetadata).
+            SamlConfiguration samlConfiguration = new SamlConfiguration();
+            FixedHttpMetaDataProvider realFixedHttpMetaDataProvider = samlConfiguration.fixedHttpMetaDataProvider(
+                    RestTemplateConfig.createDefaults(), samlConfiguration.urlContentCache(samlConfiguration.timeService()));
+            IdentityProviderProvisioning provisioning = mock(IdentityProviderProvisioning.class);
+            SamlIdentityProviderConfigurator realConfigurator =
+                    new SamlIdentityProviderConfigurator(provisioning, new IdentityZoneManagerImpl(), realFixedHttpMetaDataProvider);
+
+            SamlIdentityProviderDefinition untrustedCertDefinition = new SamlIdentityProviderDefinition()
+                    .setIdpEntityAlias(REGISTRATION_ID)
+                    .setMetaDataLocation(metadataUrl);
+            untrustedCertDefinition.setSkipSslValidation(true);
+
+            IdentityProvider<SamlIdentityProviderDefinition> idp = mock(IdentityProvider.class);
+            when(idp.getConfig()).thenReturn(untrustedCertDefinition);
+            when(provisioning.retrieveByOrigin(REGISTRATION_ID, "uaa")).thenReturn(idp);
+
+            repository = spy(new ConfiguratorRelyingPartyRegistrationRepository(ENTITY_ID, ENTITY_ID_ALIAS, realConfigurator, List.of(), DEFAULT_NAME_ID));
+            when(repository.retrieveZone()).thenReturn(identityZone);
+            when(identityZone.getId()).thenReturn("uaa");
+            when(identityZone.isUaa()).thenReturn(true);
+            when(identityZone.getConfig()).thenReturn(identityZoneConfiguration);
+            when(identityZoneConfiguration.getSamlConfig()).thenReturn(samlConfig);
+
+            // The metadata endpoint presents a self-signed cert the JVM does not trust.
+            // skipSslValidation=true should make UAA trust it here too, just like it does
+            // on the admin validate path -- today it does not, because the live login-path
+            // fetch (RelyingPartyRegistrations.fromMetadataLocation) ignores skipSslValidation.
+            RelyingPartyRegistration registration = repository.findByRegistrationId(REGISTRATION_ID);
+            assertThat(registration.getRegistrationId()).isEqualTo(REGISTRATION_ID);
+        } finally {
+            httpsServer.stop(0);
+        }
+    }
+
     @Test
     void withSha512SignatureAlgorithm() {
         repository = spy(new ConfiguratorRelyingPartyRegistrationRepository(ENTITY_ID, ENTITY_ID_ALIAS, configurator, List.of(SignatureAlgorithm.SHA512), DEFAULT_NAME_ID));