From 248739b1873ec8698a47a5c168e5a0b1fb7c47bf Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 17 Sep 2024 19:42:11 +0200 Subject: [PATCH 1/4] Draft to logout --- .../uaa/authentication/UaaAuthentication.java | 9 +++++++++ .../ZoneAwareWhitelistLogoutHandler.java | 2 +- .../manager/ExternalLoginAuthenticationManager.java | 7 +++++++ .../oauth/ExternalOAuthAuthenticationManager.java | 11 +++++++++-- .../provider/oauth/ExternalOAuthLogoutHandler.java | 12 ++++++++++-- 5 files changed, 36 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/UaaAuthentication.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/UaaAuthentication.java index bcc7837e1ad..16b32823e69 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/UaaAuthentication.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/UaaAuthentication.java @@ -48,6 +48,7 @@ public class UaaAuthentication implements Authentication, Serializable { private Set authenticationMethods; private Set authContextClassRef; private Long lastLoginSuccessTime; + private String idpIdToken; private Map userAttributes; @@ -184,6 +185,14 @@ public boolean equals(Object o) { return true; } + public String getIdpIdToken() { + return this.idpIdToken; + } + + public void setIdpIdToken(final String idpIdToken) { + this.idpIdToken = idpIdToken; + } + @Override public int hashCode() { int result = authorities.hashCode(); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandler.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandler.java index 66a2ef4e2a7..1903963bbf9 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandler.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandler.java @@ -65,7 +65,7 @@ protected String determineTargetUrl(HttpServletRequest request, HttpServletRespo if (logoutUrl == null) { return getZoneHandler().determineTargetUrl(request, response); } else { - return externalOAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, logoutUrl, oauthConfig); + return externalOAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, logoutUrl, "xxx", oauthConfig); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java index 3bd353fd43f..2688407cf32 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java @@ -9,6 +9,7 @@ import org.cloudfoundry.identity.uaa.provider.ExternalIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; +import org.cloudfoundry.identity.uaa.provider.oauth.ExternalOAuthAuthenticationManager; import org.cloudfoundry.identity.uaa.scim.ScimGroupExternalMember; import org.cloudfoundry.identity.uaa.scim.ScimGroupExternalMembershipManager; import org.cloudfoundry.identity.uaa.user.DialableByPhone; @@ -39,6 +40,7 @@ import org.springframework.security.core.userdetails.User; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UsernameNotFoundException; +import org.springframework.security.saml.context.SAMLMessageContext; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; @@ -155,6 +157,11 @@ public Authentication authenticate(Authentication request) throws Authentication uaaAuthenticationDetails = UaaAuthenticationDetails.UNKNOWN; } UaaAuthentication success = new UaaAuthentication(new UaaPrincipal(user), user.getAuthorities(), uaaAuthenticationDetails); + success.setSamlMessageContext(new SAMLMessageContext()); + if (authenticationData instanceof ExternalOAuthAuthenticationManager.AuthenticationData authenticationInternal) { + success.setIdpIdToken(authenticationInternal.getIdToken()); + } + //ExternalOAuthAuthenticationManager.AuthenticationData xxx = populateAuthenticationAttributes(success, request, authenticationData); publish(new IdentityProviderAuthenticationSuccessEvent(user, success, user.getOrigin(), IdentityZoneHolder.getCurrentZoneId())); return success; diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java index 60c1f504a1e..56b259363bf 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java @@ -227,12 +227,14 @@ public AuthenticationData getExternalAuthenticationDetails(Authentication authen AuthenticationData authenticationData = new AuthenticationData(); AbstractExternalOAuthIdentityProviderDefinition config = (AbstractExternalOAuthIdentityProviderDefinition) provider.getConfig(); - Map claims = getClaimsFromToken(codeToken, config); + String idToken = getTokenFromCode(codeToken, config); + Map claims = getClaimsFromToken(idToken, config); if (claims == null) { return null; } authenticationData.setClaims(claims); + authenticationData.setIdToken(idToken); Map attributeMappings = config.getAttributeMappings(); @@ -791,10 +793,11 @@ public KeyInfoService getKeyInfoService() { return keyInfoService; } - protected static class AuthenticationData { + public static class AuthenticationData { private Map claims; private String username; + private String idToken; private List authorities; private Map attributeMappings; @@ -830,5 +833,9 @@ public List getAuthorities() { public void setAuthorities(List authorities) { this.authorities = authorities; } + + public String getIdToken() { return this.idToken; } + + public void setIdToken(final String idToken) { this.idToken = idToken; } } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandler.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandler.java index 417e2f40aa4..52f5e37bb79 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandler.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandler.java @@ -42,8 +42,12 @@ public ExternalOAuthLogoutHandler(final IdentityProviderProvisioning providerPro protected String determineTargetUrl(final HttpServletRequest request, final HttpServletResponse response, final Authentication authentication) { final AbstractExternalOAuthIdentityProviderDefinition oauthConfig = this.getOAuthProviderForAuthentication(authentication); + String idTokenHint = null; final String logoutUrl = this.getLogoutUrl(oauthConfig); + if (authentication instanceof UaaAuthentication uaaAuthentication) { + idTokenHint = uaaAuthentication.getIdpIdToken(); + } if (logoutUrl == null) { final String defaultUrl = getZoneDefaultUrl(); if (LOGGER.isWarnEnabled()) { @@ -52,10 +56,10 @@ protected String determineTargetUrl(final HttpServletRequest request, final Http return defaultUrl; } - return this.constructOAuthProviderLogoutUrl(request, logoutUrl, oauthConfig); + return this.constructOAuthProviderLogoutUrl(request, logoutUrl, idTokenHint, oauthConfig); } - public String constructOAuthProviderLogoutUrl(final HttpServletRequest request, final String logoutUrl, + public String constructOAuthProviderLogoutUrl(final HttpServletRequest request, final String logoutUrl, String idTokenHint, final AbstractExternalOAuthIdentityProviderDefinition oauthConfig) { final StringBuilder oauthLogoutUriBuilder = new StringBuilder(request.getRequestURL()); if (StringUtils.hasText(request.getQueryString())) { @@ -66,6 +70,10 @@ public String constructOAuthProviderLogoutUrl(final HttpServletRequest request, final StringBuilder sb = new StringBuilder(logoutUrl); sb.append("?post_logout_redirect_uri="); sb.append(oauthLogoutUri); + if (idTokenHint != null) { + sb.append("&id_token_hint="); + sb.append(idTokenHint); + } sb.append("&client_id="); sb.append(oauthConfig.getRelyingPartyId()); return sb.toString(); From 90524732864647900812c981676db2543a81a458 Mon Sep 17 00:00:00 2001 From: d036670 Date: Thu, 26 Sep 2024 12:09:47 +0200 Subject: [PATCH 2/4] fix tests --- .../uaa/authentication/ZoneAwareWhitelistLogoutHandler.java | 6 +++++- .../manager/ExternalLoginAuthenticationManager.java | 1 - .../provider/oauth/ExternalOAuthAuthenticationManager.java | 2 +- .../ZoneAwareWhitelistLogoutHandlerTests.java | 2 +- .../oauth/ExternalOAuthAuthenticationManagerIT.java | 2 +- .../uaa/provider/oauth/ExternalOAuthLogoutHandlerTest.java | 2 +- 6 files changed, 9 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandler.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandler.java index 1903963bbf9..a37e22f0641 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandler.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandler.java @@ -65,7 +65,11 @@ protected String determineTargetUrl(HttpServletRequest request, HttpServletRespo if (logoutUrl == null) { return getZoneHandler().determineTargetUrl(request, response); } else { - return externalOAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, logoutUrl, "xxx", oauthConfig); + String idTokenHint = null; + if (authentication instanceof UaaAuthentication uaaAuthentication) { + idTokenHint = uaaAuthentication.getIdpIdToken(); + } + return externalOAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, logoutUrl, idTokenHint, oauthConfig); } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java index 2688407cf32..ab4c9bdc1b2 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java @@ -161,7 +161,6 @@ public Authentication authenticate(Authentication request) throws Authentication if (authenticationData instanceof ExternalOAuthAuthenticationManager.AuthenticationData authenticationInternal) { success.setIdpIdToken(authenticationInternal.getIdToken()); } - //ExternalOAuthAuthenticationManager.AuthenticationData xxx = populateAuthenticationAttributes(success, request, authenticationData); publish(new IdentityProviderAuthenticationSuccessEvent(user, success, user.getOrigin(), IdentityZoneHolder.getCurrentZoneId())); return success; diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java index 56b259363bf..622f1103b0f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java @@ -836,6 +836,6 @@ public void setAuthorities(List authorities) { public String getIdToken() { return this.idToken; } - public void setIdToken(final String idToken) { this.idToken = idToken; } + public void setIdToken(String idToken) { this.idToken = idToken; } } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandlerTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandlerTests.java index 6b3e16d87d5..4b7754e07fc 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandlerTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandlerTests.java @@ -156,7 +156,7 @@ public void test_external_client_redirect() { configuration.getLinks().getLogout().setWhitelist(Collections.singletonList("http://somethingelse.com")); configuration.getLinks().getLogout().setDisableRedirectParameter(false); when(oAuthLogoutHandler.getLogoutUrl(null)).thenReturn(""); - when(oAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, "", null)).thenReturn("/login"); + when(oAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, "", null, null)).thenReturn("/login"); request.setParameter("redirect", "http://testing.com"); request.setParameter(CLIENT_ID, CLIENT_ID); assertEquals("/login", handler.determineTargetUrl(request, response)); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java index 0a47ca5b5d6..61f6a40be30 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java @@ -688,7 +688,7 @@ void idToken_In_Redirect_Should_Use_it() { xCodeToken.setIdToken(idToken); externalOAuthAuthenticationManager.authenticate(xCodeToken); - verify(externalOAuthAuthenticationManager, times(1)).getClaimsFromToken(same(xCodeToken), any()); + verify(externalOAuthAuthenticationManager, times(0)).getClaimsFromToken(same(xCodeToken), any()); verify(externalOAuthAuthenticationManager, times(1)).getClaimsFromToken(eq(idToken), any()); verify(externalOAuthAuthenticationManager, never()).getRestTemplate(any()); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandlerTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandlerTest.java index 5e756edd8da..a129a10ccac 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandlerTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandlerTest.java @@ -104,7 +104,7 @@ void determineDefaultTargetUrl() { @Test void constructOAuthProviderLogoutUrl() { - oAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, "", oAuthIdentityProviderDefinition); + oAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, "", null, oAuthIdentityProviderDefinition); } @Test From 2f5ec5a5b7eba4a63af5179548fe88aa0db28961 Mon Sep 17 00:00:00 2001 From: d036670 Date: Thu, 26 Sep 2024 15:40:46 +0200 Subject: [PATCH 3/4] Tests added --- .../ZoneAwareWhitelistLogoutHandlerTests.java | 16 +++++++++++++++- .../oauth/ExternalOAuthLogoutHandlerTest.java | 2 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandlerTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandlerTests.java index 4b7754e07fc..17bd3b7eb9c 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandlerTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/ZoneAwareWhitelistLogoutHandlerTests.java @@ -27,12 +27,14 @@ import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.cloudfoundry.identity.uaa.provider.NoSuchClientException; +import org.springframework.security.core.context.SecurityContextHolder; import javax.servlet.ServletException; import java.io.IOException; import java.util.Collections; import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; @@ -69,6 +71,7 @@ public void setUp() { public void tearDown() { IdentityZoneHolder.clear(); IdentityZone.getUaa().setConfig(original); + SecurityContextHolder.clearContext(); } @Test @@ -123,6 +126,17 @@ public void test_whitelist_redirect_with_wildcard() { assertEquals("http://www.somethingelse.com", handler.determineTargetUrl(request, response)); } + @Test + public void test_id_token_hint() { + UaaAuthentication uaaAuthentication = mock(UaaAuthentication.class); + SecurityContextHolder.getContext().setAuthentication(uaaAuthentication); + doReturn("eyToken").when(uaaAuthentication).getIdpIdToken(); + configuration.getLinks().getLogout().setDisableRedirectParameter(false); + when(oAuthLogoutHandler.getLogoutUrl(null)).thenReturn(""); + when(oAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, "", "eyToken", null)).thenReturn("http://testing.com"); + assertEquals("http://testing.com", handler.determineTargetUrl(request, response)); + } + @Test public void test_client_redirect() { configuration.getLinks().getLogout().setWhitelist(Collections.singletonList("http://somethingelse.com")); @@ -156,7 +170,7 @@ public void test_external_client_redirect() { configuration.getLinks().getLogout().setWhitelist(Collections.singletonList("http://somethingelse.com")); configuration.getLinks().getLogout().setDisableRedirectParameter(false); when(oAuthLogoutHandler.getLogoutUrl(null)).thenReturn(""); - when(oAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, "", null, null)).thenReturn("/login"); + when(oAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, "", null, null)).thenReturn("/login"); request.setParameter("redirect", "http://testing.com"); request.setParameter(CLIENT_ID, CLIENT_ID); assertEquals("/login", handler.determineTargetUrl(request, response)); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandlerTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandlerTest.java index a129a10ccac..e0c96f3b58a 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandlerTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthLogoutHandlerTest.java @@ -104,7 +104,7 @@ void determineDefaultTargetUrl() { @Test void constructOAuthProviderLogoutUrl() { - oAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, "", null, oAuthIdentityProviderDefinition); + oAuthLogoutHandler.constructOAuthProviderLogoutUrl(request, "", "", oAuthIdentityProviderDefinition); } @Test From a692b42159eb2cd230a5c471eb711f2d36a717ab Mon Sep 17 00:00:00 2001 From: d036670 Date: Thu, 26 Sep 2024 15:45:07 +0200 Subject: [PATCH 4/4] Test added --- .../uaa/authentication/SessionResetFilterTests.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SessionResetFilterTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SessionResetFilterTests.java index 5283eab30c5..261e1b93059 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SessionResetFilterTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SessionResetFilterTests.java @@ -22,6 +22,7 @@ import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; @@ -206,6 +207,13 @@ public void testUserNotFound() throws Exception { verify(session, times(1)).invalidate(); } + @Test + public void testIdTokenSetAndGet() { + Assert.assertNull(authentication.getIdpIdToken()); + authentication.setIdpIdToken("token"); + Assert.assertEquals("token", authentication.getIdpIdToken()); + } + protected long dropMilliSeconds(long time) { return ( time / 1000l ) * 1000l; }