Ignore dangling references to no longer existing mirrored IdPs during…

… deletion
cloudfoundry · Jan 8, 2024 · e215139 · e215139
1 parent 2bf9edc
commit e215139
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 20 deletions.
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
@@ -179,9 +179,17 @@ public ResponseEntity<IdentityProvider> deleteIdentityProvider(@PathVariable Str
 
         // delete mirrored IdP if alias fields are set
         if (hasText(existing.getAliasZid()) && hasText(existing.getAliasId())) {
-            IdentityProvider<?> mirroredIdp = identityProviderProvisioning.retrieve(existing.getAliasId(), existing.getAliasZid());
-            mirroredIdp.setSerializeConfigRaw(rawConfig);
-            publisher.publishEvent(new EntityDeletedEvent<>(mirroredIdp, authentication, identityZoneId));
+            final IdentityProvider<?> mirroredIdp = retrieveMirroredIdp(existing);
+            if (mirroredIdp != null) {
+                mirroredIdp.setSerializeConfigRaw(rawConfig);
+                publisher.publishEvent(new EntityDeletedEvent<>(mirroredIdp, authentication, identityZoneId));
+            } else {
+                logger.warn(
+                        "Mirrored IdP referenced in IdentityProvider[origin={}; zone={}}] not found, skipping deletion of mirrored IdP.",
+                        existing.getOriginKey(),
+                        existing.getIdentityZoneId()
+                );
+            }
         }
 
         return new ResponseEntity<>(existing, OK);

diff --git a/.../cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointsAliasMockMvcTests.java b/.../cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointsAliasMockMvcTests.java
@@ -285,9 +285,7 @@ private void shouldAccept_ReferencedIdpNotExisting_ShouldCreateNewMirroredIdp(fi
             final IdentityProvider<?> idp = createMirroredIdp(zone1, zone2);
 
             // delete the mirrored IdP directly in the DB -> after that, there is a dangling reference
-            final JdbcIdentityProviderProvisioning identityProviderProvisioning = webApplicationContext.getBean(JdbcIdentityProviderProvisioning.class);
-            final int rowsDeleted = identityProviderProvisioning.deleteByOrigin(idp.getOriginKey(), zone2.getId());
-            assertThat(rowsDeleted).isEqualTo(1);
+            deleteIdpViaDb(idp.getOriginKey(), zone2.getId());
 
             // update some other property on the original IdP
             idp.setName("some-new-name");
@@ -491,19 +489,26 @@ private void shouldRejectUpdate(final IdentityZone zone, final IdentityProvider<
         }
     }
 
+    private void deleteIdpViaDb(final String originKey, final String zoneId) {
+        final JdbcIdentityProviderProvisioning identityProviderProvisioning = webApplicationContext
+                .getBean(JdbcIdentityProviderProvisioning.class);
+        final int rowsDeleted = identityProviderProvisioning.deleteByOrigin(originKey, zoneId);
+        assertThat(rowsDeleted).isEqualTo(1);
+    }
+
     @Nested
     class Delete {
         @Test
-        void shouldDeleteMirroredIdp_UaaToCustomZone() throws Exception {
-            shouldDeleteMirroredIdp(IdentityZone.getUaa(), customZone);
+        void shouldAlsoDeleteMirroredIdp_UaaToCustomZone() throws Exception {
+            shouldAlsoDeleteMirroredIdp(IdentityZone.getUaa(), customZone);
         }
 
         @Test
-        void shouldDeleteMirroredIdp_CustomToUaaZone() throws Exception {
-            shouldDeleteMirroredIdp(customZone, IdentityZone.getUaa());
+        void shouldAlsoDeleteMirroredIdp_CustomToUaaZone() throws Exception {
+            shouldAlsoDeleteMirroredIdp(customZone, IdentityZone.getUaa());
         }
 
-        private void shouldDeleteMirroredIdp(final IdentityZone zone1, final IdentityZone zone2) throws Exception {
+        private void shouldAlsoDeleteMirroredIdp(final IdentityZone zone1, final IdentityZone zone2) throws Exception {
             final IdentityProvider<?> idpInZone1 = createMirroredIdp(zone1, zone2);
             final String id = idpInZone1.getId();
             assertThat(id).isNotBlank();
@@ -519,17 +524,48 @@ private void shouldDeleteMirroredIdp(final IdentityZone zone1, final IdentityZon
             assertThat(mirroredIdp.get().getAliasZid()).isNotBlank().isEqualTo(idpInZone1.getIdentityZoneId());
 
             // delete IdP in zone 1
-            final String accessTokenForZone1 = getAccessTokenForZone(zone1.getId());
+            final MvcResult deleteResult = deleteIdpAndReturnResult(zone1, id);
+            assertThat(deleteResult.getResponse().getStatus()).isEqualTo(HttpStatus.OK.value());
+
+            // check if IdP is no longer available in zone 2
+            assertIdpDoesNotExist(zone2, aliasId);
+        }
+
+        @Test
+        void shouldIgnoreDanglingReferenceToMirroredIdp_UaaToCustomZone() throws Exception {
+            shouldIgnoreDanglingReferenceToMirroredIdp(IdentityZone.getUaa(), customZone);
+        }
+
+        @Test
+        void shouldIgnoreDanglingReferenceToMirroredIdp_CustomToUaaZone() throws Exception {
+            shouldIgnoreDanglingReferenceToMirroredIdp(customZone, IdentityZone.getUaa());
+        }
+
+        private void shouldIgnoreDanglingReferenceToMirroredIdp(final IdentityZone zone1, final IdentityZone zone2) throws Exception {
+            final IdentityProvider<?> originalIdp = createMirroredIdp(zone1, zone2);
+
+            // create a dangling reference by deleting the mirrored IdP directly in the DB
+            deleteIdpViaDb(originalIdp.getOriginKey(), zone2.getId());
+
+            // delete the original IdP -> dangling reference should be ignored
+            final MvcResult deleteResult = deleteIdpAndReturnResult(zone1, originalIdp.getId());
+            assertThat(deleteResult.getResponse().getStatus()).isEqualTo(HttpStatus.OK.value());
+
+            // original IdP should no longer exist
+            assertIdpDoesNotExist(zone1, originalIdp.getId());
+        }
+
+        private MvcResult deleteIdpAndReturnResult(final IdentityZone zone, final String id) throws Exception {
+            final String accessTokenForZone1 = getAccessTokenForZone(zone.getId());
             final MockHttpServletRequestBuilder deleteRequestBuilder = delete("/identity-providers/" + id)
                     .header("Authorization", "Bearer " + accessTokenForZone1)
-                    .header(IdentityZoneSwitchingFilter.HEADER, zone1.getId());
-            final MvcResult response = mockMvc.perform(deleteRequestBuilder).andReturn();
-
-            assertThat(response.getResponse().getStatus()).isEqualTo(200);
+                    .header(IdentityZoneSwitchingFilter.HEADER, zone.getId());
+            return mockMvc.perform(deleteRequestBuilder).andReturn();
+        }
 
-            // check if IdP is no longer available in zone 2
-            final Optional<IdentityProvider<?>> mirroredIdpAfterDeletionOfOriginalIdp = readIdpFromZoneIfExists(zone2.getId(), aliasId);
-            assertThat(mirroredIdpAfterDeletionOfOriginalIdp).isNotPresent();
+        private void assertIdpDoesNotExist(final IdentityZone zone, final String id) throws Exception {
+            final Optional<IdentityProvider<?>> idp = readIdpFromZoneIfExists(zone.getId(), id);
+            assertThat(idp).isNotPresent();
         }
     }
 
@@ -620,7 +656,10 @@ private void assertOtherPropertiesAreEqual(final IdentityProvider<?> idp, final
 
     private IdentityProvider<?> createMirroredIdp(final IdentityZone zone1, final IdentityZone zone2) throws Exception {
         final IdentityProvider<?> provider = buildIdpWithAliasProperties(zone1.getId(), null, zone2.getId());
-        return createIdp(zone1, provider);
+        final IdentityProvider<?> createdOriginalIdp = createIdp(zone1, provider);
+        assertThat(createdOriginalIdp.getAliasId()).isNotBlank();
+        assertThat(createdOriginalIdp.getAliasZid()).isNotBlank();
+        return createdOriginalIdp;
     }
 
     private IdentityProvider<?> createIdp(final IdentityZone zone, final IdentityProvider<?> idp) throws Exception {