Fix: unreachable bootstrap SAML IDP metadata crashes UAA startup#3974
Merged
Conversation
#3971 made SamlRelyingPartyRegistrationRepositoryConfig resolve each bootstrap-YAML SAML IDP's metadata via SamlIdentityProviderConfigurator.resolveMetadataXml(), which performs a live HTTP(S) fetch. That call happened while building the RelyingPartyRegistrationBuilder.Params object, outside the try block that only wrapped buildRelyingPartyRegistration(params), and it can throw RestClientException (DNS/connect failures) rather than the Saml2Exception the catch clause was written for. An unreachable bootstrap IDP metadata URL -- previously logged as an error and skipped, matching this loop's own documented intent ("some SAML IDPs might require you to supply the SAML SP metadata first before you can get the SAML IDP metadata") -- now takes down the entire Spring context and prevents UAA from starting. Move the resolution inside the try block and broaden the catch to Exception, so a single bad/unreachable bootstrap IDP is logged and skipped instead of crashing startup, restoring the pre-#3971 resilience guarantee. Verified against scripts/saml/saml-legacy-uaa.yml's 'simple.saml.invalid' provider, which is deliberately configured with an unreachable metadata URL to exercise exactly this path. Add a regression test asserting relyingPartyRegistrationRepository() still returns successfully when a bootstrap IDP's metadata fetch throws. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Catching Exception broadly would also swallow genuine bugs (e.g. an NPE from a real defect) as a quiet log line instead of surfacing them. Catch only the exception types this call site can actually throw: Saml2Exception (buildRelyingPartyRegistration's own parsing failures), RestClientException (the live metadata fetch: DNS/connect/HTTP failures), and IllegalStateException (a malformed metadata URI in resolveMetadataXml). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restores UAA’s ability to start successfully when a bootstrap-configured SAML IDP has an unreachable or otherwise failing metadata URL, by ensuring failures during metadata resolution/build are handled as “log and skip” rather than crashing Spring context initialization.
Changes:
- Moves bootstrap IDP metadata resolution into the existing per-IDP
tryblock so fetch failures are caught and tolerated. - Broadens the bootstrap loop error handling to avoid taking down startup when metadata resolution fails.
- Adds a regression test to ensure the relying-party repository bean still builds when a bootstrap IDP’s metadata resolution throws.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfig.java |
Wraps bootstrap IDP metadata resolution + registration build in a single try/catch to prevent startup crashes when one bootstrap IDP is unreachable. |
server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfigTest.java |
Adds a test ensuring repository creation tolerates an exception thrown during bootstrap IDP metadata resolution. |
Address Copilot review comment on #3974: the regression test only asserted the bean builds and the default registration survives, not that the bad provider itself was dropped. Assert that looking it up by its registrationId falls through to DefaultRelyingPartyRegistrationRepository's generic stub (entityId == the SP's own ENTITY_ID) rather than some partial/broken registration. That fallback path calls retrieveKeyManager(), which reads IdentityZoneHolder's static SAML key manager state -- add the same IdentityZoneHolder.Initializer setup ConfiguratorRelyingPartyRegistrationRepositoryTest already uses, since this is the first test in this class to exercise it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
…rap IdP simplesamlphp.cfapps.io was a real (now decommissioned) Pivotal Web Services demo host that happened to stop resolving, which is what surfaced the startup-crash bug this PR fixes. Point simple.saml.invalid at the RFC 2606 reserved .invalid TLD instead, so this fixture's "must never resolve" intent is guaranteed and documented rather than incidental. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
strehle
approved these changes
Jul 8, 2026
github-project-automation
Bot
moved this from Inbox
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Foundational Infrastructure Working Group
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #3971. That PR made
SamlRelyingPartyRegistrationRepositoryConfig's bootstrap-YAML SAML IDP loop resolve each provider's metadata viaSamlIdentityProviderConfigurator.resolveMetadataXml(), which performs a live HTTP(S) fetch. That call happens while building theRelyingPartyRegistrationBuilder.Paramsobject -- outside thetryblock that only wrapsbuildRelyingPartyRegistration(params)-- and it can throwRestClientException(DNS/connect failures) rather than theSaml2Exceptionthe existingcatchclause was written for.Net effect: a bootstrap SAML IDP with a momentarily (or permanently) unreachable metadata URL -- previously logged as an error and skipped, per this loop's own documented intent ("some SAML IDPs might require you to supply the SAML SP metadata first before you can get the SAML IDP metadata") -- now crashes the entire Spring context and prevents UAA from starting.
Reproduced locally via
scripts/saml/saml-legacy-uaa.yml'ssimple.saml.invalidprovider, which is deliberately configured with an unreachable metadata URL (http://simplesamlphp.cfapps.io/..., a long-decommissioned Pivotal Web Services demo host) specifically to exercise this "tolerate an unreachable bootstrap IDP" path:Change
tryblock inSamlRelyingPartyRegistrationRepositoryConfig.relyingPartyRegistrationRepository().catchfromSaml2ExceptiontoException, since the failure surface now includesRestClientExceptionfrom the live fetch, in addition toSaml2ExceptionfrombuildRelyingPartyRegistration()itself.Test plan
relyingPartyRegistrationRepositoryToleratesUnreachableBootstrapIdpMetadatatoSamlRelyingPartyRegistrationRepositoryConfigTest, asserting the bean still builds successfully when a bootstrap IDP's metadata resolution throwsResourceAccessException.develop's current HEAD (reproducing the crash) and passes with this fix.org.cloudfoundry.identity.uaa.provider.saml.*package: 269/269 passing.