Fix SAML RelayState allowlist validation - #4014
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens SAML IdP-initiated login redirects by validating the RelayState URL against the identity zone’s configured links.logout.whitelist and falling back to a safe default when the target is not allowlisted, preventing open redirect abuse in UAA’s SAML login flow.
Changes:
- Updated
UaaSavedRequestAwareAuthenticationSuccessHandlerto resolveRelayStateredirects viaUaaUrlUtils.findMatchingRedirectUri(...)using the zone’s logout whitelist and a safe fallback. - Added/adjusted unit tests to cover whitelisted vs non-whitelisted
RelayStatebehavior. - Updated SAML integration tests to assert the post-login redirect no longer goes to an external non-whitelisted URL.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/SamlLoginIT.java |
Updates integration assertions to reflect the new safe fallback redirect behavior for non-allowlisted RelayState. |
server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java |
Adds unit coverage for RelayState redirect allowlist behavior (whitelisted vs not whitelisted). |
server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java |
Implements allowlist validation for SAML RelayState redirects using the zone’s configured whitelist and a fallback URL. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java:55
- In the RelayState redirect branch, the handler bypasses the normal success-handler flow and never clears authentication-related session attributes (e.g., SPRING_SECURITY_LAST_EXCEPTION). This can leave stale error state in the session even after a successful SAML login, which is inconsistent with the other branches that call clearAuthenticationAttributes().
log.debug("Redirecting after SAML login. requestedRelayState='{}' redirectUri='{}'", relayState, redirectUri);
this.getRedirectStrategy().sendRedirect(request, response, redirectUri);
fhanik
left a comment
There was a problem hiding this comment.
Maybe add imports instead of fully qualified names in the code
|
Thanks @fhanik comments addressed |
fhanik
left a comment
There was a problem hiding this comment.
I believe we will need release notes for these changes. we can do that as a separate PR before next release
Fix: Prevented open redirect vulnerabilities by securely validating the RelayState parameter during SAML authentication.
Details: Updated UaaSavedRequestAwareAuthenticationSuccessHandler to validate the RelayState URL against the links.logout.whitelist configured in the identity zone (using UaaUrlUtils.findMatchingRedirectUri). If the target is not in the allowlist, the application safely falls back to redirecting to the context path root (/).