Skip to content

Commit

Permalink
Reject deletion of entities if alias exists and alias feature is disa…
Browse files Browse the repository at this point in the history 
…bled
  • Loading branch information
@adrianhoelzl-sap
adrianhoelzl-sap committed Mar 28, 2024
1 parent d6b85bc commit 07366c2
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 66 deletions.
3 changes: 3 additions & 0 deletions server/src/main/java/org/cloudfoundry/identity/uaa/alias/EntityAliasHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,9 @@ private T buildAliasEntity(final T originalEntity) {
protected abstract T cloneEntity(final T originalEntity);

public final Optional<T> retrieveAliasEntity(final T originalEntity) {
if (!hasText(originalEntity.getAliasId()) || !hasText(originalEntity.getAliasZid())) {
return Optional.empty();
}
return retrieveEntity(originalEntity.getAliasId(), originalEntity.getAliasZid());
}

Expand Down
20 changes: 9 additions & 11 deletions server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,19 @@ public ResponseEntity<IdentityProvider> deleteIdentityProvider(@PathVariable Str
return new ResponseEntity<>(UNPROCESSABLE_ENTITY);
}

// reject deletion if the IdP has an alias, but alias feature is disabled
final boolean idpHasAlias = hasText(existing.getAliasZid()) || hasText(existing.getAliasId());
if (idpHasAlias && !aliasEntitiesEnabled) {
return new ResponseEntity<>(BAD_REQUEST);
}

// delete the IdP
existing.setSerializeConfigRaw(rawConfig);
publisher.publishEvent(new EntityDeletedEvent<>(existing, authentication, identityZoneId));
redactSensitiveData(existing);

if (hasText(existing.getAliasZid()) && hasText(existing.getAliasId())) {
// delete the alias IdP if present
if (idpHasAlias) {
final Optional<IdentityProvider<?>> aliasIdpOpt = idpAliasHandler.retrieveAliasEntity(existing);
if (aliasIdpOpt.isEmpty()) {
// ignore dangling reference to alias
Expand All @@ -202,16 +210,6 @@ public ResponseEntity<IdentityProvider> deleteIdentityProvider(@PathVariable Str
}

final IdentityProvider<?> aliasIdp = aliasIdpOpt.get();
if (!aliasEntitiesEnabled) {
// if alias entities are not enabled, just break the reference
aliasIdp.setAliasId(null);
aliasIdp.setAliasZid(null);
identityProviderProvisioning.update(aliasIdp, aliasIdp.getIdentityZoneId());

return new ResponseEntity<>(existing, OK);
}

// also delete the alias IdP
aliasIdp.setSerializeConfigRaw(rawConfig);
publisher.publishEvent(new EntityDeletedEvent<>(aliasIdp, authentication, identityZoneId));
}
Expand Down
45 changes: 11 additions & 34 deletions ...r/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -670,45 +670,22 @@ void testDeleteIdpWithAlias_DanglingReference() {
void testDeleteIdpWithAlias_AliasFeatureDisabled() {
arrangeAliasEntitiesEnabled(false);

// ensure event publisher is present
final ApplicationEventPublisher mockEventPublisher = mock(ApplicationEventPublisher.class);
identityProviderEndpoints.setApplicationEventPublisher(mockEventPublisher);

// arrange IdP with alias exists
final String customZoneId = UUID.randomUUID().toString();
final Pair<IdentityProvider<?>, IdentityProvider<?>> idpAndAlias = arrangeIdpWithAliasExists(UAA, customZoneId);
final IdentityProvider<?> idp = idpAndAlias.getLeft();
final IdentityProvider<?> aliasIdp = idpAndAlias.getRight();

final ApplicationEventPublisher mockEventPublisher = mock(ApplicationEventPublisher.class);
identityProviderEndpoints.setApplicationEventPublisher(mockEventPublisher);
doNothing().when(mockEventPublisher).publishEvent(any());

identityProviderEndpoints.deleteIdentityProvider(idp.getId(), true);

// the original IdP should be deleted
final ArgumentCaptor<EntityDeletedEvent<?>> entityDeletedEventCaptor = ArgumentCaptor.forClass(EntityDeletedEvent.class);
verify(mockEventPublisher, times(1)).publishEvent(entityDeletedEventCaptor.capture());
final EntityDeletedEvent<?> event = entityDeletedEventCaptor.getValue();
Assertions.assertThat(event).isNotNull();
Assertions.assertThat(event.getIdentityZoneId()).isEqualTo(UAA);
Assertions.assertThat(((IdentityProvider<?>) event.getSource()).getId()).isEqualTo(idp.getId());

// instead of being deleted, the alias IdP should just have its reference to the original IdP removed
final ArgumentCaptor<IdentityProvider> updateIdpParamCaptor = ArgumentCaptor.forClass(IdentityProvider.class);
verify(mockIdentityProviderProvisioning).update(updateIdpParamCaptor.capture(), eq(customZoneId));
final IdentityProvider updateIdpParam = updateIdpParamCaptor.getValue();
Assertions.assertThat(updateIdpParam).isNotNull();
Assertions.assertThat(updateIdpParam.getAliasId()).isBlank();
Assertions.assertThat(updateIdpParam.getAliasZid()).isBlank();
assertIdpsAreEqualApartFromAliasProperties(updateIdpParam, aliasIdp);
}
final ResponseEntity<IdentityProvider> response = identityProviderEndpoints.deleteIdentityProvider(
idp.getId(),
true
);

private static void assertIdpsAreEqualApartFromAliasProperties(
final IdentityProvider<?> idp1,
final IdentityProvider<?> idp2
) {
idp2.setAliasId(null);
idp1.setAliasId(null);
idp2.setAliasZid(null);
idp1.setAliasZid(null);
Assertions.assertThat(idp1).isEqualTo(idp2);
// deletion should be rejected
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
}

private Pair<IdentityProvider<?>, IdentityProvider<?>> arrangeIdpWithAliasExists(final String zone1Id, final String zone2Id) {
Expand All @@ -734,7 +711,7 @@ private Pair<IdentityProvider<?>, IdentityProvider<?>> arrangeIdpWithAliasExists
aliasIdp.setIdentityZoneId(zone2Id);
aliasIdp.setAliasId(idpId);
aliasIdp.setAliasZid(zone1Id);
when(mockIdpAliasHandler.retrieveAliasEntity(idp)).thenReturn(Optional.of(aliasIdp));
lenient().when(mockIdpAliasHandler.retrieveAliasEntity(idp)).thenReturn(Optional.of(aliasIdp));

return Pair.of(idp, aliasIdp);
}
Expand Down
67 changes: 46 additions & 21 deletions .../cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointsAliasMockMvcTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,13 @@ public DeleteBase(final boolean aliasFeatureEnabled) {
void setUp() {
arrangeAliasFeatureEnabled(aliasFeatureEnabled);
}
}

@Nested
class AliasFeatureEnabled extends DeleteBase {
public AliasFeatureEnabled() {
super(true);
}

@Test
void shouldIgnoreDanglingReferenceToAliasIdp_UaaToCustomZone() throws Throwable {
Expand All @@ -1102,7 +1109,10 @@ void shouldIgnoreDanglingReferenceToAliasIdp_CustomToUaaZone() throws Throwable
shouldIgnoreDanglingReferenceToAliasIdp(customZone, IdentityZone.getUaa());
}

private void shouldIgnoreDanglingReferenceToAliasIdp(final IdentityZone zone1, final IdentityZone zone2) throws Throwable {
private void shouldIgnoreDanglingReferenceToAliasIdp(
final IdentityZone zone1,
final IdentityZone zone2
) throws Throwable {
final IdentityProvider<?> originalIdp = executeWithTemporarilyEnabledAliasFeature(
aliasFeatureEnabled,
() -> createIdpWithAlias(zone1, zone2)
Expand Down Expand Up @@ -1155,21 +1165,6 @@ private void deletionWithExistingAliasIdp(
final MvcResult deleteResult = deleteIdpAndReturnResult(zone1, id);
assertThat(deleteResult.getResponse().getStatus()).isEqualTo(HttpStatus.OK.value());

// alias IdP should still exist, but without reference to original IdP
assertAliasIdpAfterDeletion(aliasId, aliasZid);
}

protected abstract void assertAliasIdpAfterDeletion(final String aliasId, final String aliasZid) throws Exception;
}

@Nested
class AliasFeatureEnabled extends DeleteBase {
public AliasFeatureEnabled() {
super(true);
}

@Override
protected void assertAliasIdpAfterDeletion(final String aliasId, final String aliasZid) throws Exception {
// if the alias feature is enabled, the alias should also be removed
assertIdpDoesNotExist(aliasId, aliasZid);
}
Expand All @@ -1181,13 +1176,43 @@ public AliasFeatureDisabled() {
super(false);
}

@Override
protected void assertAliasIdpAfterDeletion(final String aliasId, final String aliasZid) throws Exception {
// if the alias feature is disabled, only the reference should be removed from the alias IdP
assertReferenceWasRemovedFromAlias(aliasId, aliasZid);
@Test
void shouldRejectDeletion_WhenAliasIdpExists_UaaToCustomZone() throws Throwable {
shouldRejectDeletion_WhenAliasIdpExists(IdentityZone.getUaa(), customZone);
}

@Test
void shouldRejectDeletion_WhenAliasIdpExists_CustomToUaaZone() throws Throwable {
shouldRejectDeletion_WhenAliasIdpExists(customZone, IdentityZone.getUaa());
}
}

private void shouldRejectDeletion_WhenAliasIdpExists(
final IdentityZone zone1,
final IdentityZone zone2
) throws Throwable {
// create IdP in zone 1 with alias in zone 2
final IdentityProvider<?> idpInZone1 = executeWithTemporarilyEnabledAliasFeature(
aliasFeatureEnabled,
() -> createIdpWithAlias(zone1, zone2)
);
final String id = idpInZone1.getId();
assertThat(id).isNotBlank();
final String aliasId = idpInZone1.getAliasId();
assertThat(aliasId).isNotBlank();
final String aliasZid = idpInZone1.getAliasZid();
assertThat(aliasZid).isNotBlank().isEqualTo(zone2.getId());

// check if alias IdP is available in zone 2
final Optional<IdentityProvider<?>> aliasIdp = readIdpFromZoneIfExists(zone2.getId(), aliasId);
assertThat(aliasIdp).isPresent();
assertThat(aliasIdp.get().getAliasId()).isNotBlank().isEqualTo(id);
assertThat(aliasIdp.get().getAliasZid()).isNotBlank().isEqualTo(idpInZone1.getIdentityZoneId());

// delete IdP in zone 1 -> should be rejected since alias feature is disabled
final MvcResult deleteResult = deleteIdpAndReturnResult(zone1, id);
assertThat(deleteResult.getResponse().getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value());
}
}

private MvcResult deleteIdpAndReturnResult(final IdentityZone zone, final String id) throws Exception {
final String accessTokenForZone1 = getAccessTokenForZone(zone.getId());
Expand Down

0 comments on commit 07366c2

Please sign in to comment.