Add OpenSaml work around for FIPS initialization#3809
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a centralized OpenSAML initialization path intended to work around FIPS-related initialization failures (per the referenced Spring Security issue) and updates the OpenSAML dependency accordingly.
Changes:
- Add
IdentityZoneConfig.setupOpenSaml()as a shared OpenSAML initialization hook and call it from SAML components and various tests. - Ensure SAML relying party registration initialization depends on both BouncyCastle setup and OpenSAML setup.
- Bump OpenSAML from
4.0.1to4.3.2and adjust SAML test object construction (e.g., settingIssueInstant).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| uaa/src/test/java/org/cloudfoundry/identity/uaa/YamlConfigurationValidationTests.java | Ensures OpenSAML is initialized before YAML-driven context refresh in tests. |
| uaa/src/test/java/org/cloudfoundry/identity/uaa/DefaultTestContext.java | Initializes OpenSAML during test application context initialization. |
| server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/TestOpenSamlObjects.java | Switches to shared OpenSAML init and sets IssueInstant on generated objects. |
| server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/TestCustomOpenSamlObjects.java | Switches to shared OpenSAML init for custom OpenSAML object test utilities. |
| server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/OpenSaml4AuthenticationProviderUnitTests.java | Adds per-test OpenSAML setup call. |
| server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/OpenSaml4AuthenticationProviderUaaTests.java | Replaces OpenSAML core initialization with the shared setup method. |
| server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/BootstrapSamlIdentityProviderDataTests.java | Ensures OpenSAML setup occurs before SAML IDP bootstrap tests. |
| server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlRelyingPartyRegistrationRepositoryConfig.java | Adds dependency ordering so OpenSAML setup runs before relying party repository creation. |
| server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/Saml2BearerGrantAuthenticationConverter.java | Replaces Spring’s OpenSAML init with the shared setup method. |
| server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/OpenSaml4AuthenticationProvider.java | Replaces Spring’s OpenSAML init with the shared setup method. |
| server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/IdentityZoneConfig.java | Introduces the custom OpenSAML initialization workaround used across the codebase. |
| dependencies.gradle | Bumps OpenSAML version to 4.3.2. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| OpenSamlInitializationService.initialize(); | ||
| } catch (NoClassDefFoundError | NoSuchMethodError e) { | ||
| // ignore |
There was a problem hiding this comment.
The catch (NoClassDefFoundError | NoSuchMethodError) block silently ignores initialization failures without any logging or context, which can mask real classpath/version problems and make FIPS/OpenSAML startup issues hard to diagnose. If the intent is to ignore a known Spring Security/OpenSAML initialization exception, consider catching the specific exception type(s) expected and logging at least a debug/warn message.
| // ignore | |
| log.warn("OpenSAML initialization via Spring Security was skipped due to a classpath or version compatibility issue. Continuing with existing initialization path.", e); |
Implement what suggested and ignore then the spring security exception. See
spring-projects/spring-security#14656
other ref:
https://shibboleth.atlassian.net/wiki/spaces/DEV/pages/1159627167/FIPS