Unable to update an existing active Saml identity provider and give it a new metadataurl when it's prior one was no longer valid

[dvuke]

What version of UAA are you running?

We're currently using 4.20 but found that the issue exists in the latest code base as well.

What did you do?

We're attempting to update an existing active identity provider whose metadataurl no longer points to a valid URL.

Ex:

1. create a SAML identity provider with a valid metadataurl
2. take the metadata provider offline (existing definition now has an invalid metadata url)
3. use the UAA end point to update the identity provider and provide a new valid metadata url.

This fails b/c of SamlIdentityProviderConfiguration -> validateSamlIdentityProviderDefinition

When it's validating the new properties of the definitions, it actually tries to go and get existing IdentityProviders for that identity zone and calls getExtendedMetadataDelegate on every existing provider (including the existing one that we already know doesn't have a valid metadataurl). downstream that throws an exception unable to fetch metadata and we're not able to update the provider.

for (SamlIdentityProviderDefinition existing : getIdentityProviderDefinitions()) {
ConfigMetadataProvider existingProvider = (ConfigMetadataProvider) getExtendedMetadataDelegate(existing).getDelegate();
if (entityIDToBeAdded.equals(existingProvider.getEntityID()) &&
!(existing.getUniqueAlias().equals(clone.getUniqueAlias()))) {
entityIDexists = true;
break;
}
}

What did you expect to see? What goal are you trying to achieve with the UAA?

I should be able to update an existing IdentityProvider and give it a new valid metadataurl after the current one is no longer valid.

What did you see instead?

It threw an exception unable to fetch metadata

Please include UAA logs if available.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/183213155

The labels on this github issue will be updated when the story is started.

[strehle]

@bruce-ricard @peterhaochen47 Can you please take care here, because SAML is something we dont use anymore

@dvuke there was an issue in past #1531 which was fixed with #1563 , so please can you add UAA logs from a recent UAA

[dvuke]

@strehle appreciate the response; unfortunately I'm stuck on v4.20 for the time being (sorry for posting issues on dated environments, I know that' frustrating); we'll try to upgrade soon and see if it resolves any of the issues we were seeing. if getExtendedMetadataDelegate() is failsafe in newer versions, it should be fine; but looking at the source code, calling that when a tenant has any invalid metadatasaml configs (even when the IDP is disabled) will throw the exception.

It has to do with how it was validating the entity id being unique for the tenant when updating an IDP saml config. (it goes and gets every one for the tenant to compare (enabled or not) and in that process it tries to reach out for every one of the IDPs metadata given its given configured URL, which when that url isn't valid anymore, throws an exception.

[bruce-ricard]

Apologies, I didn't see this until today. We'll take a look.

[bruce-ricard]

It looks like you've got the issue pretty much pinned pointed in the code @dvuke . Would you be able to submit a PR to fix the issue by any chance?