rebase and revert entiyID checks

cloudfoundry · Jul 16, 2024 · 8998521 · 8998521
1 parent 114af0c
commit 8998521
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 50 deletions.
diff --git a/.../src/main/java/org/cloudfoundry/identity/uaa/provider/SamlIdentityProviderDefinition.java b/.../src/main/java/org/cloudfoundry/identity/uaa/provider/SamlIdentityProviderDefinition.java
@@ -122,7 +122,6 @@ public void setIdpEntityId(final String idpEntityId) {
         this.idpEntityId = idpEntityId;
     }
 
-    private boolean validateXml(String xml) {
     private static boolean validateXml(String xml) {
         if (xml == null || xml.toUpperCase().contains("<!DOCTYPE")) {
             return false;

diff --git a/...java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderDefinitionTests.java b/...java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderDefinitionTests.java
@@ -181,13 +181,14 @@ public void test_Get_Data_Type_When_Valid() {
     public void testSetIdpEntityAlias() {
         SamlIdentityProviderDefinition def = new SamlIdentityProviderDefinition();
         def.setIdpEntityAlias("testalias");
+        assertThat(def.getIdpEntityAlias()).isEqualTo("testalias");
     }
 
     @Test
     public void testSetIdpEntityId() {
         SamlIdentityProviderDefinition def = new SamlIdentityProviderDefinition();
         def.setIdpEntityId("testalias");
-        assertEquals("testalias", def.getIdpEntityId());
+        assertThat(def.getIdpEntityId()).isEqualTo("testalias");
     }
 
     @Test

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java
@@ -138,7 +138,7 @@ public ResponseEntity<IdentityProvider> createIdentityProvider(@RequestBody Iden
             SamlIdentityProviderDefinition definition = ObjectUtils.castInstance(body.getConfig(), SamlIdentityProviderDefinition.class);
             definition.setZoneId(zoneId);
             definition.setIdpEntityAlias(body.getOriginKey());
-            definition.setIdpEntityId(samlConfigurator.validateSamlIdentityProviderDefinition(definition, true));
+            samlConfigurator.validateSamlIdentityProviderDefinition(definition);
             body.setConfig(definition);
         }
 
@@ -223,7 +223,7 @@ public ResponseEntity<IdentityProvider> updateIdentityProvider(@PathVariable Str
             SamlIdentityProviderDefinition definition = ObjectUtils.castInstance(body.getConfig(), SamlIdentityProviderDefinition.class);
             definition.setZoneId(zoneId);
             definition.setIdpEntityAlias(body.getOriginKey());
-            samlConfigurator.validateSamlIdentityProviderDefinition(definition, false);
+            samlConfigurator.validateSamlIdentityProviderDefinition(definition);
             body.setConfig(definition);
         }
 

diff --git a/...n/java/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderData.java b/...n/java/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderData.java
@@ -50,13 +50,6 @@ public class BootstrapSamlIdentityProviderData implements InitializingBean {
     private boolean legacyShowSamlLink = true;
     private List<IdentityProviderWrapper<SamlIdentityProviderDefinition>> samlProviders = new LinkedList<>();
     private Map<String, Map<String, Object>> providers = null;
-    private final SamlIdentityProviderConfigurator samlConfigurator;
-
-    public BootstrapSamlIdentityProviderData(
-        final @Qualifier("metaDataProviders") SamlIdentityProviderConfigurator samlConfigurator
-    ) {
-      this.samlConfigurator = samlConfigurator;
-    }
 
     public static IdentityProvider<SamlIdentityProviderDefinition> parseSamlProvider(SamlIdentityProviderDefinition def) {
         IdentityProvider<SamlIdentityProviderDefinition> provider = new IdentityProvider();
@@ -181,14 +174,6 @@ public void setIdentityProviders(Map<String, Map<String, Object>> providers) {
             def.setAuthnContext(authnContext);
 
             IdentityProvider provider = parseSamlProvider(def);
-            try {
-                if (def.getType() == SamlIdentityProviderDefinition.MetadataLocation.DATA) {
-                    ExtendedMetadataDelegate metadataDelegate = samlConfigurator.getExtendedMetadataDelegate(def);
-                    def.setIdpEntityId(((ConfigMetadataProvider) metadataDelegate.getDelegate()).getEntityID());
-                }
-            } catch (MetadataProviderException e) {
-                throw new IllegalArgumentException(e.getMessage(), e);
-            }
             IdentityProviderWrapper wrapper = new IdentityProviderWrapper(provider);
             wrapper.setOverride(override == null || override);
             samlProviders.add(wrapper);

diff --git a/...in/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfigurator.java b/...in/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfigurator.java
@@ -71,12 +71,10 @@ public List<SamlIdentityProviderDefinition> getIdentityProviderDefinitions(List<
      * adds or replaces a SAML identity proviider
      *
      * @param providerDefinition - the provider to be added
-     * @param creation - check new created config
      * @throws MetadataProviderException if the system fails to fetch meta data for this provider
      */
+    public synchronized void validateSamlIdentityProviderDefinition(SamlIdentityProviderDefinition providerDefinition) /* throws MetadataProviderException */ {
 //        ExtendedMetadataDelegate added, deleted = null;
-    public synchronized String validateSamlIdentityProviderDefinition(SamlIdentityProviderDefinition providerDefinition, boolean creation) {
-        ExtendedMetadataDelegate added, deleted = null;
         if (providerDefinition == null) {
             throw new NullPointerException();
         }
@@ -93,22 +91,20 @@ public synchronized String validateSamlIdentityProviderDefinition(SamlIdentityPr
 //            throw new MetadataProviderException("Emtpy entityID for SAML provider with zoneId:" + providerDefinition.getZoneId() + " and origin:" + providerDefinition.getIdpEntityAlias());
 //        }
 
-        boolean entityIDexists = creation && entityIdExists(entityIDToBeAdded, providerDefinition.getZoneId());
+        boolean entityIDexists = false;
 
-        if (!entityIDexists) {
-            for (SamlIdentityProviderDefinition existing : getIdentityProviderDefinitions()) {
-                ConfigMetadataProvider existingProvider = (ConfigMetadataProvider) getExtendedMetadataDelegate(existing).getDelegate();
-                if (entityIDToBeAdded.equals(existingProvider.getEntityID()) && !(existing.getUniqueAlias().equals(clone.getUniqueAlias()))) {
-                    entityIDexists = true;
-                    break;
-                }
-            }
-        }
+//        for (SamlIdentityProviderDefinition existing : getIdentityProviderDefinitions()) {
+////            ConfigMetadataProvider existingProvider = (ConfigMetadataProvider) getExtendedMetadataDelegate(existing).getDelegate();
+////            if (entityIDToBeAdded.equals(existingProvider.getEntityID()) &&
+////                    !(existing.getUniqueAlias().equals(clone.getUniqueAlias()))) {
+////                entityIDexists = true;
+////                break;
+////            }
+//        }
 
-        if (entityIDexists) {
-            throw new MetadataProviderException("Duplicate entity ID:" + entityIDToBeAdded);
-        }
-        return entityIDToBeAdded;
+//        if (entityIDexists) {
+//            throw new MetadataProviderException("Duplicate entity ID:" + entityIDToBeAdded);
+//        }
     }
 
 //    public ExtendedMetadataDelegate getExtendedMetadataDelegateFromCache(SamlIdentityProviderDefinition def) throws MetadataProviderException {

diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityProviderBootstrapTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/config/IdentityProviderBootstrapTest.java
@@ -427,7 +427,7 @@ void samlBootstrap() throws Exception {
         bootstrap.afterPropertiesSet();
 
         IdentityProvider samlProvider = provisioning.retrieveByExternId(samlIdentityProviderDefinition.getIdpEntityAlias(), SAML, IdentityZone.getUaaZoneId());
-        assertNotNull(samlProvider);
+        assertThat(samlProvider).isNotNull();
         samlIdentityProviderDefinition.setZoneId(IdentityZone.getUaaZoneId());
         assertThat(samlProvider.getConfig()).isEqualTo(samlIdentityProviderDefinition);
         assertThat(samlProvider.getCreated()).isNotNull();

diff --git a/...a/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderDataTests.java b/...a/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderDataTests.java
@@ -13,13 +13,9 @@
 package org.cloudfoundry.identity.uaa.provider.saml;
 
 import org.cloudfoundry.identity.uaa.provider.AbstractIdentityProviderDefinition;
-import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning;
 import org.cloudfoundry.identity.uaa.provider.SamlIdentityProviderDefinition;
 import org.junit.Before;
-import org.junit.BeforeClass;
 import org.junit.Test;
-import org.opensaml.DefaultBootstrap;
-import org.opensaml.xml.parse.BasicParserPool;
 import org.springframework.beans.factory.config.YamlMapFactoryBean;
 import org.springframework.beans.factory.config.YamlProcessor;
 import org.springframework.core.io.ByteArrayResource;
@@ -149,16 +145,9 @@ public class BootstrapSamlIdentityProviderDataTests {
         "        " + testXmlFileData.replace("\n","\n        ") + "\n"
         ;
 
-    @BeforeClass
-    public static void initializeOpenSAML() throws Exception {
-        if (!org.apache.xml.security.Init.isInitialized()) {
-            DefaultBootstrap.bootstrap();
-        }
-    }
-
     @Before
     public void setUp() {
-        bootstrap = new BootstrapSamlIdentityProviderData(new SamlIdentityProviderConfigurator(new BasicParserPool(), mock(JdbcIdentityProviderProvisioning.class), mock(FixedHttpMetaDataProvider.class)));
+        bootstrap = new BootstrapSamlIdentityProviderData();
         singleAdd = new SamlIdentityProviderDefinition()
             .setMetaDataLocation(String.format(BootstrapSamlIdentityProviderDataTests.xmlWithoutID, new RandomValueStringGenerator().generate()))
             .setIdpEntityAlias(singleAddAlias)

diff --git a/...va/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfiguratorTests.java b/...va/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfiguratorTests.java
@@ -115,7 +115,7 @@ private String getSimpleSamlPhpMetadata(String domain) {
 
     @BeforeEach
     public void setUp() {
-        bootstrap = new BootstrapSamlIdentityProviderData(new SamlIdentityProviderConfigurator(new BasicParserPool(), mock(JdbcIdentityProviderProvisioning.class), mock(FixedHttpMetaDataProvider.class)));
+        bootstrap = new BootstrapSamlIdentityProviderData();
         singleAdd = new SamlIdentityProviderDefinition()
                 .setMetaDataLocation(String.format(BootstrapSamlIdentityProviderDataTests.xmlWithoutID, new RandomValueStringGenerator().generate()))
                 .setIdpEntityAlias(singleAddAlias)