From c1038214b9742d0267671d583acfd398138bd72a Mon Sep 17 00:00:00 2001 From: Joe Mahady Date: Wed, 29 Jul 2026 11:09:26 +0100 Subject: [PATCH 1/3] Implement SAML RelayState allowlist validation ai-assisted=yes Co-authored-by: Cursor --- ...uestAwareAuthenticationSuccessHandler.java | 7 +++++- ...wareAuthenticationSuccessHandlerTests.java | 22 ++++++++++++++++++- .../uaa/integration/feature/SamlLoginIT.java | 4 ++-- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java b/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java index f277bbfebe3..c705f9bf245 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java @@ -47,7 +47,12 @@ public void onAuthenticationSuccess(HttpServletRequest request, HttpServletRespo String relayState = UaaStringUtils.getCleanedUserControlString(request.getParameter(Saml2ParameterNames.RELAY_STATE), UaaStringUtils.EMPTY_STRING); if (UaaStringUtils.hasText(relayState) && UaaUrlUtils.isUrl(relayState)) { log.debug("Redirecting to relayState URI: {}", relayState); - this.getRedirectStrategy().sendRedirect(request, response, relayState); + java.util.List samlRelayStateWhitelist = java.util.Optional.ofNullable( + org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.get().getConfig().getLinks().getLogout().getWhitelist() + ).orElse(java.util.Collections.emptyList()); + String fallbackUrl = this.getDefaultTargetUrl(); + String matchingRedirectUri = UaaUrlUtils.findMatchingRedirectUri(samlRelayStateWhitelist, relayState, fallbackUrl); + this.getRedirectStrategy().sendRedirect(request, response, matchingRedirectUri); } else { super.onAuthenticationSuccess(request, response, authentication); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java index 2d605c0d240..eeffd9e3a3c 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java @@ -76,7 +76,7 @@ void invalidFormRedirectIsNotReturned() { } @Test - void onAuthenticationSuccess_noSavedRequest_hasRelayStateUrl() throws Exception { + void onAuthenticationSuccess_noSavedRequest_hasRelayStateUrl_notWhitelisted() throws Exception { String redirectUri = "https://test.com/test2"; request.setParameter(Saml2ParameterNames.RELAY_STATE, redirectUri); @@ -84,7 +84,27 @@ void onAuthenticationSuccess_noSavedRequest_hasRelayStateUrl() throws Exception var authentication = mock(Authentication.class); handler.onAuthenticationSuccess(request, response, authentication); + assertThat(response.getRedirectedUrl()).isEqualTo("/"); + } + + @Test + void onAuthenticationSuccess_noSavedRequest_hasRelayStateUrl_whitelisted() throws Exception { + String redirectUri = "https://test.com/test2"; + request.setParameter(Saml2ParameterNames.RELAY_STATE, redirectUri); + + org.cloudfoundry.identity.uaa.zone.IdentityZone zone = org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.get(); + zone.getConfig().getLinks().getLogout().setWhitelist(java.util.List.of("https://test.com/test2")); + org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.set(zone); + + var response = new MockHttpServletResponse(); + var authentication = mock(Authentication.class); + handler.onAuthenticationSuccess(request, response, authentication); + assertThat(response.getRedirectedUrl()).isEqualTo(redirectUri); + + // clean up + zone.getConfig().getLinks().getLogout().setWhitelist(null); + org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.clear(); } @Test diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/SamlLoginIT.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/SamlLoginIT.java index c526d4657e6..861d91e2ce7 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/SamlLoginIT.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/SamlLoginIT.java @@ -779,7 +779,7 @@ void relayStateRedirectFromIdpInitiatedLogin() { webDriver.findElement(By.xpath(samlServerConfig.getLoginPromptXpathExpr())); sendCredentials(testAccounts.getUserName(), testAccounts.getPassword()); Page.assertThatUrlEventuallySatisfies(webDriver, - assertUrl -> assertUrl.startsWith("https://www.google.com")); + assertUrl -> assertUrl.startsWith(baseUrl)); webDriver.get("%s/logout.do".formatted(baseUrl)); } @@ -1228,7 +1228,7 @@ void backportFrom77RelayTest() { sendCredentials(testAccounts.getUserName(), "koala"); Page.assertThatUrlEventuallySatisfies(webDriver, - assertUrl -> assertUrl.startsWith("https://www.google.com")); + assertUrl -> assertUrl.startsWith(zoneUrl)); webDriver.get(baseUrl + "/logout.do"); webDriver.get(zoneUrl + "/logout.do"); } From e1362560db0d1ab3167695e4cffa77f357927233 Mon Sep 17 00:00:00 2001 From: Joe Mahady Date: Fri, 7 Aug 2026 13:29:11 +0100 Subject: [PATCH 2/3] Fix SAML RelayState allowlist validation --- ...uestAwareAuthenticationSuccessHandler.java | 6 +-- ...wareAuthenticationSuccessHandlerTests.java | 20 +++++---- ...enticationSuccessHandlerZonePathTests.java | 41 +++++++++++++++++++ 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java b/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java index c705f9bf245..7fd998e7124 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java @@ -46,13 +46,13 @@ public void onAuthenticationSuccess(HttpServletRequest request, HttpServletRespo if (savedRequest == null) { String relayState = UaaStringUtils.getCleanedUserControlString(request.getParameter(Saml2ParameterNames.RELAY_STATE), UaaStringUtils.EMPTY_STRING); if (UaaStringUtils.hasText(relayState) && UaaUrlUtils.isUrl(relayState)) { - log.debug("Redirecting to relayState URI: {}", relayState); java.util.List samlRelayStateWhitelist = java.util.Optional.ofNullable( org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.get().getConfig().getLinks().getLogout().getWhitelist() ).orElse(java.util.Collections.emptyList()); String fallbackUrl = this.getDefaultTargetUrl(); - String matchingRedirectUri = UaaUrlUtils.findMatchingRedirectUri(samlRelayStateWhitelist, relayState, fallbackUrl); - this.getRedirectStrategy().sendRedirect(request, response, matchingRedirectUri); + String redirectUri = UaaUrlUtils.findMatchingRedirectUri(samlRelayStateWhitelist, relayState, fallbackUrl); + log.debug("Redirecting after SAML login. requestedRelayState='{}' redirectUri='{}'", relayState, redirectUri); + this.getRedirectStrategy().sendRedirect(request, response, redirectUri); } else { super.onAuthenticationSuccess(request, response, authentication); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java index eeffd9e3a3c..d4503c18a92 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java @@ -93,18 +93,20 @@ void onAuthenticationSuccess_noSavedRequest_hasRelayStateUrl_whitelisted() throw request.setParameter(Saml2ParameterNames.RELAY_STATE, redirectUri); org.cloudfoundry.identity.uaa.zone.IdentityZone zone = org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.get(); - zone.getConfig().getLinks().getLogout().setWhitelist(java.util.List.of("https://test.com/test2")); + java.util.List originalWhitelist = zone.getConfig().getLinks().getLogout().getWhitelist(); + zone.getConfig().getLinks().getLogout().setWhitelist(java.util.List.of(redirectUri)); org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.set(zone); - var response = new MockHttpServletResponse(); - var authentication = mock(Authentication.class); - handler.onAuthenticationSuccess(request, response, authentication); + try { + var response = new MockHttpServletResponse(); + var authentication = mock(Authentication.class); + handler.onAuthenticationSuccess(request, response, authentication); - assertThat(response.getRedirectedUrl()).isEqualTo(redirectUri); - - // clean up - zone.getConfig().getLinks().getLogout().setWhitelist(null); - org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.clear(); + assertThat(response.getRedirectedUrl()).isEqualTo(redirectUri); + } finally { + zone.getConfig().getLinks().getLogout().setWhitelist(originalWhitelist); + org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.set(zone); + } } @Test diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerZonePathTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerZonePathTests.java index 8e57a7ab9a9..602a2bb4793 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerZonePathTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerZonePathTests.java @@ -125,4 +125,45 @@ void onAuthenticationSuccess_withSavedRequest_redirects_to_saved_url(ZoneRequest assertThat(response.getRedirectedUrl()).isEqualTo(savedRedirectUrl); } + + @ParameterizedTest + @EnumSource(ZoneRequestPathMode.class) + void onAuthenticationSuccess_noSavedRequest_hasRelayStateUrl_notWhitelisted(ZoneRequestPathMode mode) throws Exception { + mode.setZone(); + mode.applyRequestPath(request, "/login.do"); + String redirectUri = "https://test.com/test2"; + request.setParameter(org.springframework.security.saml2.core.Saml2ParameterNames.RELAY_STATE, redirectUri); + + MockHttpServletResponse response = new MockHttpServletResponse(); + Authentication authentication = mock(Authentication.class); + + handler.onAuthenticationSuccess(request, response, authentication); + + String expected = mode.redirectPrefix().isEmpty() ? "/" : mode.redirectPrefix() + "/"; + assertThat(response.getRedirectedUrl()).isEqualTo(expected); + } + + @ParameterizedTest + @EnumSource(ZoneRequestPathMode.class) + void onAuthenticationSuccess_noSavedRequest_hasRelayStateUrl_whitelisted(ZoneRequestPathMode mode) throws Exception { + mode.setZone(); + mode.applyRequestPath(request, "/login.do"); + String redirectUri = "https://test.com/test2"; + request.setParameter(org.springframework.security.saml2.core.Saml2ParameterNames.RELAY_STATE, redirectUri); + + org.cloudfoundry.identity.uaa.zone.IdentityZone zone = org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.get(); + java.util.List originalWhitelist = zone.getConfig().getLinks().getLogout().getWhitelist(); + zone.getConfig().getLinks().getLogout().setWhitelist(java.util.List.of(redirectUri)); + + try { + MockHttpServletResponse response = new MockHttpServletResponse(); + Authentication authentication = mock(Authentication.class); + + handler.onAuthenticationSuccess(request, response, authentication); + + assertThat(response.getRedirectedUrl()).isEqualTo(redirectUri); + } finally { + zone.getConfig().getLinks().getLogout().setWhitelist(originalWhitelist); + } + } } From 0afc7830181967248a1de589dc52bcc2b2b16c9e Mon Sep 17 00:00:00 2001 From: Joe Mahady Date: Wed, 12 Aug 2026 10:58:11 +0100 Subject: [PATCH 3/3] Fix SAML RelayState allowlist validation - Review Comments --- ...questAwareAuthenticationSuccessHandler.java | 10 +++++++--- ...AwareAuthenticationSuccessHandlerTests.java | 14 +++++++++----- ...henticationSuccessHandlerZonePathTests.java | 18 +++++++++++------- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java b/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java index 7fd998e7124..4d7269ddee9 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandler.java @@ -26,8 +26,12 @@ import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler; import org.springframework.security.web.savedrequest.HttpSessionRequestCache; import org.springframework.security.web.savedrequest.SavedRequest; +import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Optional; @Slf4j public class UaaSavedRequestAwareAuthenticationSuccessHandler extends SavedRequestAwareAuthenticationSuccessHandler { @@ -46,9 +50,9 @@ public void onAuthenticationSuccess(HttpServletRequest request, HttpServletRespo if (savedRequest == null) { String relayState = UaaStringUtils.getCleanedUserControlString(request.getParameter(Saml2ParameterNames.RELAY_STATE), UaaStringUtils.EMPTY_STRING); if (UaaStringUtils.hasText(relayState) && UaaUrlUtils.isUrl(relayState)) { - java.util.List samlRelayStateWhitelist = java.util.Optional.ofNullable( - org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.get().getConfig().getLinks().getLogout().getWhitelist() - ).orElse(java.util.Collections.emptyList()); + List samlRelayStateWhitelist = Optional.ofNullable( + IdentityZoneHolder.get().getConfig().getLinks().getLogout().getWhitelist() + ).orElse(Collections.emptyList()); String fallbackUrl = this.getDefaultTargetUrl(); String redirectUri = UaaUrlUtils.findMatchingRedirectUri(samlRelayStateWhitelist, relayState, fallbackUrl); log.debug("Redirecting after SAML login. requestedRelayState='{}' redirectUri='{}'", relayState, redirectUri); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java index d4503c18a92..92152cbaca0 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerTests.java @@ -22,9 +22,13 @@ import org.springframework.security.core.Authentication; import org.springframework.security.saml2.core.Saml2ParameterNames; import org.springframework.security.web.savedrequest.SavedRequest; +import org.cloudfoundry.identity.uaa.zone.IdentityZone; +import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import jakarta.servlet.http.HttpSession; +import java.util.List; + import static org.assertj.core.api.Assertions.assertThat; import static org.cloudfoundry.identity.uaa.web.UaaSavedRequestAwareAuthenticationSuccessHandler.FORM_REDIRECT_PARAMETER; import static org.cloudfoundry.identity.uaa.web.UaaSavedRequestAwareAuthenticationSuccessHandler.URI_OVERRIDE_ATTRIBUTE; @@ -92,10 +96,10 @@ void onAuthenticationSuccess_noSavedRequest_hasRelayStateUrl_whitelisted() throw String redirectUri = "https://test.com/test2"; request.setParameter(Saml2ParameterNames.RELAY_STATE, redirectUri); - org.cloudfoundry.identity.uaa.zone.IdentityZone zone = org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.get(); - java.util.List originalWhitelist = zone.getConfig().getLinks().getLogout().getWhitelist(); - zone.getConfig().getLinks().getLogout().setWhitelist(java.util.List.of(redirectUri)); - org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.set(zone); + IdentityZone zone = IdentityZoneHolder.get(); + List originalWhitelist = zone.getConfig().getLinks().getLogout().getWhitelist(); + zone.getConfig().getLinks().getLogout().setWhitelist(List.of(redirectUri)); + IdentityZoneHolder.set(zone); try { var response = new MockHttpServletResponse(); @@ -105,7 +109,7 @@ void onAuthenticationSuccess_noSavedRequest_hasRelayStateUrl_whitelisted() throw assertThat(response.getRedirectedUrl()).isEqualTo(redirectUri); } finally { zone.getConfig().getLinks().getLogout().setWhitelist(originalWhitelist); - org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.set(zone); + IdentityZoneHolder.set(zone); } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerZonePathTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerZonePathTests.java index 602a2bb4793..34bb1db5e8f 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerZonePathTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/web/UaaSavedRequestAwareAuthenticationSuccessHandlerZonePathTests.java @@ -24,7 +24,12 @@ import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.core.Authentication; +import org.springframework.security.saml2.core.Saml2ParameterNames; +import org.springframework.security.web.savedrequest.SavedRequest; import org.cloudfoundry.identity.uaa.extensions.EnabledIfZonePathsEnabled; +import org.cloudfoundry.identity.uaa.zone.IdentityZone; + +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -113,8 +118,7 @@ void onAuthenticationSuccess_withSavedRequest_redirects_to_saved_url(ZoneRequest String savedRedirectUrl = mode.redirectPrefix().isEmpty() ? "http://localhost/oauth/authorize?client_id=admin" : "http://localhost" + mode.redirectPrefix() + "/oauth/authorize?client_id=admin"; - org.springframework.security.web.savedrequest.SavedRequest savedRequest = - mock(org.springframework.security.web.savedrequest.SavedRequest.class); + SavedRequest savedRequest = mock(SavedRequest.class); when(savedRequest.getRedirectUrl()).thenReturn(savedRedirectUrl); request.getSession(true).setAttribute(SPRING_SECURITY_SAVED_REQUEST, savedRequest); @@ -132,7 +136,7 @@ void onAuthenticationSuccess_noSavedRequest_hasRelayStateUrl_notWhitelisted(Zone mode.setZone(); mode.applyRequestPath(request, "/login.do"); String redirectUri = "https://test.com/test2"; - request.setParameter(org.springframework.security.saml2.core.Saml2ParameterNames.RELAY_STATE, redirectUri); + request.setParameter(Saml2ParameterNames.RELAY_STATE, redirectUri); MockHttpServletResponse response = new MockHttpServletResponse(); Authentication authentication = mock(Authentication.class); @@ -149,11 +153,11 @@ void onAuthenticationSuccess_noSavedRequest_hasRelayStateUrl_whitelisted(ZoneReq mode.setZone(); mode.applyRequestPath(request, "/login.do"); String redirectUri = "https://test.com/test2"; - request.setParameter(org.springframework.security.saml2.core.Saml2ParameterNames.RELAY_STATE, redirectUri); + request.setParameter(Saml2ParameterNames.RELAY_STATE, redirectUri); - org.cloudfoundry.identity.uaa.zone.IdentityZone zone = org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder.get(); - java.util.List originalWhitelist = zone.getConfig().getLinks().getLogout().getWhitelist(); - zone.getConfig().getLinks().getLogout().setWhitelist(java.util.List.of(redirectUri)); + IdentityZone zone = IdentityZoneHolder.get(); + List originalWhitelist = zone.getConfig().getLinks().getLogout().getWhitelist(); + zone.getConfig().getLinks().getLogout().setWhitelist(List.of(redirectUri)); try { MockHttpServletResponse response = new MockHttpServletResponse();