From 3527e593f26978e60492c026c1cc22910c8cd20b Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Mon, 6 Nov 2023 14:02:04 -0800 Subject: [PATCH 1/7] backfill test: RP initiated logout - Add test coverage on the existing behavior described in https://github.com/cloudfoundry/uaa/issues/2589 where UAA attempts to log the user out of the external OIDC provider after a successful UAA logout (this is called the RP initiated logout) [#184752215] --- .../identity/uaa/integration/feature/OIDCLoginIT.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java index c806fdeb2b3..6b1b647b647 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java @@ -88,6 +88,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; @@ -181,6 +182,7 @@ public void setUp() throws Exception { config.setTokenKeyUrl(new URL(urlBase + "/token_key")); config.setIssuer(urlBase + "/oauth/token"); config.setUserInfoUrl(new URL(urlBase + "/userinfo")); + config.setLogoutUrl(new URL(urlBase + "/logout.do")); config.setShowLinkText(true); config.setLinkText("My OIDC Provider"); @@ -558,6 +560,15 @@ public void testResponseTypeRequired() { assertThat(webDriver.getCurrentUrl(), containsString("error_description=Missing%20response_type%20in%20authorization%20request")); } + @Test + public void successfulUaaLogoutTriggersExternalOIDCProviderLogout() { + validateSuccessfulOIDCLogin(zoneUrl, testAccounts.getUserName(), testAccounts.getPassword()); + + String externalOIDCProviderLoginPage = baseUrl; + webDriver.get(externalOIDCProviderLoginPage); + Assert.assertThat("URL validation failed", webDriver.getCurrentUrl(), endsWith("/login")); + } + private String getRefreshTokenResponse(ServerRunning serverRunning, String refreshToken) { MultiValueMap formData = new LinkedMultiValueMap<>(); formData.add("client_id", zoneClient.getClientId()); From c4b2118274bada555aa69968ebb4b31b2a0f42ca Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Mon, 6 Nov 2023 15:54:38 -0800 Subject: [PATCH 2/7] feat: add config to control "OIDC RP-initiated logout" - New config "performRpInitiatedLogout" (default to true to preserve existing behavior) added to /identity-providers API and uaa.yml. It is a flag controlling whether to log out of the external provider after a successful UAA logout per [OIDC RP-Initiated Logout](https://openid.net/specs/openid-connect-rpinitiated-1_0.html)" - doc changes [more context: https://github.com/cloudfoundry/uaa/issues/2589] [#184752215] --- ...actExternalOAuthIdentityProviderDefinition.java | 11 +++++++++++ .../ZoneAwareWhitelistLogoutHandler.java | 7 ++++--- .../provider/oauth/ExternalOAuthLogoutHandler.java | 7 +++++++ .../provider/oauth/OauthIDPWrapperFactoryBean.java | 3 +++ .../ZoneAwareWhitelistLogoutHandlerTests.java | 11 ++++++++++- .../oauth/ExternalOAuthLogoutHandlerTest.java | 11 +++++++++++ ...hIdentityProviderDefinitionFactoryBeanTest.java | 14 ++++++++++++++ uaa/slateCustomizations/source/index.html.md.erb | 2 +- .../uaa/integration/feature/OIDCLoginIT.java | 12 ++++++++++++ .../providers/IdentityProviderEndpointDocs.java | 4 ++++ 10 files changed, 77 insertions(+), 5 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/provider/AbstractExternalOAuthIdentityProviderDefinition.java b/model/src/main/java/org/cloudfoundry/identity/uaa/provider/AbstractExternalOAuthIdentityProviderDefinition.java index ce40fd96bf0..1a121524313 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/provider/AbstractExternalOAuthIdentityProviderDefinition.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/provider/AbstractExternalOAuthIdentityProviderDefinition.java @@ -46,6 +46,7 @@ public enum OAuthGroupMappingMode { private String userPropagationParameter; private OAuthGroupMappingMode groupMappingMode; private boolean pkce = true; + private boolean performRpInitiatedLogout = true; public URL getAuthUrl() { return authUrl; @@ -214,6 +215,14 @@ public boolean isPkce() { return this.pkce; } + public boolean isPerformRpInitiatedLogout() { + return performRpInitiatedLogout; + } + + public void setPerformRpInitiatedLogout(boolean performRpInitiatedLogout) { + this.performRpInitiatedLogout = performRpInitiatedLogout; + } + @JsonIgnore public Class getParameterizedClass() { ParameterizedType parameterizedType = @@ -247,6 +256,7 @@ public boolean equals(Object o) { if (!Objects.equals(userPropagationParameter, that.userPropagationParameter)) return false; if (!Objects.equals(groupMappingMode, that.groupMappingMode)) return false; if (pkce != that.pkce) return false; + if (performRpInitiatedLogout != that.performRpInitiatedLogout) return false; return Objects.equals(responseType, that.responseType); } @@ -271,6 +281,7 @@ public int hashCode() { result = 31 * result + (groupMappingMode != null ? groupMappingMode.hashCode() : 0); result = 31 * result + (responseType != null ? responseType.hashCode() : 0); result = 31 * result + (pkce ? 1 : 0); + result = 31 * result + (performRpInitiatedLogout ? 1 : 0); return result; } } 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 8eebec26c62..dabc6263e42 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 @@ -47,11 +47,12 @@ public ZoneAwareWhitelistLogoutHandler(MultitenantClientServices clientDetailsSe public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException, ServletException { AbstractExternalOAuthIdentityProviderDefinition oauthConfig = externalOAuthLogoutHandler.getOAuthProviderForAuthentication(authentication); String logoutUrl = externalOAuthLogoutHandler.getLogoutUrl(oauthConfig); + Boolean shouldPerformRpInitiatedLogout = externalOAuthLogoutHandler.getPerformRpInitiatedLogout(oauthConfig); - if (logoutUrl == null) { - getZoneHandler().onLogoutSuccess(request, response, authentication); - } else { + if (shouldPerformRpInitiatedLogout && logoutUrl != null) { externalOAuthLogoutHandler.onLogoutSuccess(request, response, authentication); + } else { + getZoneHandler().onLogoutSuccess(request, response, authentication); } } 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 d331938cb91..0b07251ca91 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 @@ -123,4 +123,11 @@ private String getZoneDefaultUrl() { } return config.getLinks().getLogout().getRedirectUrl(); } + + public Boolean getPerformRpInitiatedLogout(AbstractExternalOAuthIdentityProviderDefinition oauthConfig) { + if (oauthConfig == null) { + return false; + } + return oauthConfig.isPerformRpInitiatedLogout(); + } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java index 887f957c844..198dd94286a 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java @@ -166,6 +166,9 @@ protected void setCommonProperties(Map idpDefinitionMap, Abstrac if (idpDefinitionMap.get("clientAuthInBody") instanceof Boolean) { idpDefinition.setClientAuthInBody((boolean)idpDefinitionMap.get("clientAuthInBody")); } + if (idpDefinitionMap.get("performRpInitiatedLogout") instanceof Boolean) { + idpDefinition.setPerformRpInitiatedLogout((boolean)idpDefinitionMap.get("performRpInitiatedLogout")); + } } private static Map parseAdditionalParameters(Map idpDefinitionMap) { 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 451bfef7099..62ea712e817 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 @@ -163,12 +163,21 @@ public void test_external_client_redirect() { } @Test - public void test_exteral_logout() throws ServletException, IOException { + public void test_external_logout() throws ServletException, IOException { when(oAuthLogoutHandler.getLogoutUrl(null)).thenReturn(""); + when(oAuthLogoutHandler.getPerformRpInitiatedLogout(null)).thenReturn(true); handler.onLogoutSuccess(request, response, null); verify(oAuthLogoutHandler, times(1)).onLogoutSuccess(request, response, null); } + @Test + public void test_does_not_external_logout() throws ServletException, IOException { + when(oAuthLogoutHandler.getLogoutUrl(null)).thenReturn(""); + when(oAuthLogoutHandler.getPerformRpInitiatedLogout(null)).thenReturn(false); + handler.onLogoutSuccess(request, response, null); + verify(oAuthLogoutHandler, times(0)).onLogoutSuccess(request, response, null); + } + @Test public void test_logout() throws ServletException, IOException { handler.onLogoutSuccess(request, response, null); 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 1c6a99f7969..a0ea05121ad 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 @@ -134,4 +134,15 @@ void getOAuthProviderForAuthentication() { void getNullOAuthProviderForAuthentication() { assertEquals(null, oAuthLogoutHandler.getOAuthProviderForAuthentication(null)); } + + @Test + void getPerformRpInitiatedLogout() { + oAuthIdentityProviderDefinition.setPerformRpInitiatedLogout(true); + assertEquals(true, oAuthLogoutHandler.getPerformRpInitiatedLogout(oAuthIdentityProviderDefinition)); + + oAuthIdentityProviderDefinition.setPerformRpInitiatedLogout(false); + assertEquals(false, oAuthLogoutHandler.getPerformRpInitiatedLogout(oAuthIdentityProviderDefinition)); + + assertEquals(false, oAuthLogoutHandler.getPerformRpInitiatedLogout(null)); + } } \ No newline at end of file diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIdentityProviderDefinitionFactoryBeanTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIdentityProviderDefinitionFactoryBeanTest.java index ff7a1ec05b4..b0ed32d74d1 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIdentityProviderDefinitionFactoryBeanTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIdentityProviderDefinitionFactoryBeanTest.java @@ -234,4 +234,18 @@ public void testNoAdditionalParametersInConfig() { Map receivedParameters = ((OIDCIdentityProviderDefinition) factoryBean.getProviders().get(0).getProvider().getConfig()).getAdditionalAuthzParameters(); assertEquals(0, receivedParameters.size()); } + + @Test + public void testPerformRpInitiatedLogoutTrue() { + idpDefinitionMap.put("performRpInitiatedLogout", true); + factoryBean.setCommonProperties(idpDefinitionMap, providerDefinition); + assertTrue(providerDefinition.isPerformRpInitiatedLogout()); + } + + @Test + public void testPerformRpInitiatedLogoutFalse() { + idpDefinitionMap.put("performRpInitiatedLogout", false); + factoryBean.setCommonProperties(idpDefinitionMap, providerDefinition); + assertFalse(providerDefinition.isPerformRpInitiatedLogout()); + } } diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb index 181da452557..e681802d83e 100644 --- a/uaa/slateCustomizations/source/index.html.md.erb +++ b/uaa/slateCustomizations/source/index.html.md.erb @@ -635,7 +635,7 @@ _Error Codes_ ## Logout.do The logout endpoint is meant to be used by applications to log the user out of the UAA session. UAA will only log a user out of the UAA session if they also hit this endpoint, and may also perform Single Logout with SAML providers if configured to do so. -UAA will also log users out of OIDC proxied authenticated sessions based on [OpenID Connect Session Management](https://openid.net/specs/openid-connect-session-1_0.html). +UAA may also log users out of OIDC proxied authenticated sessions based on [OpenID Connect Session Management](https://openid.net/specs/openid-connect-session-1_0.html) if configured to do so. The recommendation for application authors is to: * provide a local logout feature specific to the client application diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java index 6b1b647b647..5b512df12c6 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java @@ -569,6 +569,18 @@ public void successfulUaaLogoutTriggersExternalOIDCProviderLogout() { Assert.assertThat("URL validation failed", webDriver.getCurrentUrl(), endsWith("/login")); } + @Test + public void successfulUaaLogoutDoesNotTriggerExternalOIDCProviderLogout_whenConfiguredNotTo() { + identityProvider.getConfig().setPerformRpInitiatedLogout(false); + updateProvider(); + + validateSuccessfulOIDCLogin(zoneUrl, testAccounts.getUserName(), testAccounts.getPassword()); + + String externalOIDCProviderLoginPage = baseUrl; + webDriver.get(externalOIDCProviderLoginPage); + Assert.assertThat(webDriver.getPageSource(), containsString("Where to?")); + } + private String getRefreshTokenResponse(ServerRunning serverRunning, String refreshToken) { MultiValueMap formData = new LinkedMultiValueMap<>(); formData.add("client_id", zoneClient.getClientId()); diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointDocs.java index b03b9c75605..45cd5ccf382 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointDocs.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointDocs.java @@ -457,6 +457,7 @@ void createOAuthIdentityProvider() throws Exception { definition.setAttributeMappings(getAttributeMappingMap()); definition.setUserPropagationParameter("username"); definition.setPkce(true); + definition.setPerformRpInitiatedLogout(true); identityProvider.setConfig(definition); identityProvider.setSerializeConfigRaw(true); @@ -478,6 +479,7 @@ void createOAuthIdentityProvider() throws Exception { fieldWithPath("config.responseType").optional("code").type(STRING).description("Response type for the authorize request, will be sent to OAuth server, defaults to `code`"), fieldWithPath("config.clientAuthInBody").optional(false).type(BOOLEAN).description("Sends the client credentials in the token retrieval call as body parameters instead of a Basic Authorization header."), fieldWithPath("config.pkce").optional(true).type(BOOLEAN).description("A flag controlling whether PKCE (RFC 7636) is active in authorization code flow when requesting tokens from the external provider."), + fieldWithPath("config.performRpInitiatedLogout").optional(true).type(BOOLEAN).description("A flag controlling whether to log out of the external provider after a successful UAA logout per [OIDC RP-Initiated Logout](https://openid.net/specs/openid-connect-rpinitiated-1_0.html)"), fieldWithPath("config.issuer").optional(null).type(STRING).description("The OAuth 2.0 token issuer. This value is used to validate the issuer inside the token."), fieldWithPath("config.userPropagationParameter").optional("username").type(STRING).description("Name of the request parameter that is used to pass a known username when redirecting to this identity provider from the account chooser"), fieldWithPath("config.attributeMappings.user_name").optional("sub").type(STRING).description("Map `user_name` to the attribute for user name in the provider assertion or token. The default for OpenID Connect is `sub`"), @@ -528,6 +530,7 @@ void createOidcIdentityProvider() throws Exception { definition.setRelyingPartySecret("secret"); definition.setShowLinkText(false); definition.setPkce(true); + definition.setPerformRpInitiatedLogout(true); definition.setAttributeMappings(getAttributeMappingMap()); definition.setUserPropagationParameter("username"); definition.setExternalGroupsWhitelist(Collections.singletonList("uaa.user")); @@ -555,6 +558,7 @@ void createOidcIdentityProvider() throws Exception { fieldWithPath("config.checkTokenUrl").optional(null).type(OBJECT).description("Reserved for future OAuth/OIDC use."), fieldWithPath("config.clientAuthInBody").optional(false).type(BOOLEAN).description("Only effective if relyingPartySecret is defined. Sends the client credentials in the token retrieval call as body parameters instead of a Basic Authorization header. It is recommended to set `jwtClientAuthentication:true` instead."), fieldWithPath("config.pkce").optional(true).type(BOOLEAN).description("A flag controlling whether PKCE (RFC 7636) is active in authorization code flow when requesting tokens from the external provider."), + fieldWithPath("config.performRpInitiatedLogout").optional(true).type(BOOLEAN).description("A flag controlling whether to log out of the external provider after a successful UAA logout per [OIDC RP-Initiated Logout](https://openid.net/specs/openid-connect-rpinitiated-1_0.html)"), fieldWithPath("config.userInfoUrl").optional(null).type(OBJECT).description("Reserved for future OIDC use. This can be left blank if a discovery URL is provided. If both are provided, this property overrides the discovery URL."), fieldWithPath("config.logoutUrl").optional(null).type(OBJECT).description("OIDC logout endpoint. This can be left blank if a discovery URL is provided. If both are provided, this property overrides the discovery URL."), fieldWithPath("config.responseType").optional("code").type(STRING).description("Response type for the authorize request, defaults to `code`, but can be `code id_token` if the OIDC server can return an id_token as a query parameter in the redirect."), From d02a9ffb7eb89eae521d0f20d22ae9e9842873c8 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 7 Nov 2023 15:05:04 -0800 Subject: [PATCH 3/7] refactor: makes test more readible - to improve readibility, in test setup, explicitly set "performRpInitiatedLogout" config to "true" (even though that is the current default) [#186127119] --- .../identity/uaa/integration/feature/OIDCLoginIT.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java index 5b512df12c6..dc5985fba32 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java @@ -561,7 +561,10 @@ public void testResponseTypeRequired() { } @Test - public void successfulUaaLogoutTriggersExternalOIDCProviderLogout() { + public void successfulUaaLogoutTriggersExternalOIDCProviderLogout_whenConfiguredTo() { + identityProvider.getConfig().setPerformRpInitiatedLogout(true); + updateProvider(); + validateSuccessfulOIDCLogin(zoneUrl, testAccounts.getUserName(), testAccounts.getPassword()); String externalOIDCProviderLoginPage = baseUrl; From 00a3af26670c70b123273514d2a814c27141ce87 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 7 Nov 2023 16:38:33 -0800 Subject: [PATCH 4/7] refactor: simplify type - no need to use the Object Boolean when primitive boolean will do [#184752215] --- .../uaa/authentication/ZoneAwareWhitelistLogoutHandler.java | 2 +- .../identity/uaa/provider/oauth/ExternalOAuthLogoutHandler.java | 2 +- 2 files changed, 2 insertions(+), 2 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 dabc6263e42..4d3013589c5 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 @@ -47,7 +47,7 @@ public ZoneAwareWhitelistLogoutHandler(MultitenantClientServices clientDetailsSe public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException, ServletException { AbstractExternalOAuthIdentityProviderDefinition oauthConfig = externalOAuthLogoutHandler.getOAuthProviderForAuthentication(authentication); String logoutUrl = externalOAuthLogoutHandler.getLogoutUrl(oauthConfig); - Boolean shouldPerformRpInitiatedLogout = externalOAuthLogoutHandler.getPerformRpInitiatedLogout(oauthConfig); + boolean shouldPerformRpInitiatedLogout = externalOAuthLogoutHandler.getPerformRpInitiatedLogout(oauthConfig); if (shouldPerformRpInitiatedLogout && logoutUrl != null) { externalOAuthLogoutHandler.onLogoutSuccess(request, response, authentication); 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 0b07251ca91..67e10bc4d34 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 @@ -124,7 +124,7 @@ private String getZoneDefaultUrl() { return config.getLinks().getLogout().getRedirectUrl(); } - public Boolean getPerformRpInitiatedLogout(AbstractExternalOAuthIdentityProviderDefinition oauthConfig) { + public boolean getPerformRpInitiatedLogout(AbstractExternalOAuthIdentityProviderDefinition oauthConfig) { if (oauthConfig == null) { return false; } From fe69e18940b3fd11c353abaaf4211bee980be8f1 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Thu, 9 Nov 2023 11:15:25 -0800 Subject: [PATCH 5/7] refactor: improve test failure output [#184752215] --- .../identity/uaa/integration/feature/OIDCLoginIT.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java index dc5985fba32..14ad2cfdfdc 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/OIDCLoginIT.java @@ -569,7 +569,8 @@ public void successfulUaaLogoutTriggersExternalOIDCProviderLogout_whenConfigured String externalOIDCProviderLoginPage = baseUrl; webDriver.get(externalOIDCProviderLoginPage); - Assert.assertThat("URL validation failed", webDriver.getCurrentUrl(), endsWith("/login")); + Assert.assertThat("Did not land on the external OIDC provider login page (as an unauthenticated user).", + webDriver.getCurrentUrl(), endsWith("/login")); } @Test @@ -581,7 +582,8 @@ public void successfulUaaLogoutDoesNotTriggerExternalOIDCProviderLogout_whenConf String externalOIDCProviderLoginPage = baseUrl; webDriver.get(externalOIDCProviderLoginPage); - Assert.assertThat(webDriver.getPageSource(), containsString("Where to?")); + Assert.assertThat("Did not land on the external OIDC provider home page (as an authenticated user).", + webDriver.getPageSource(), containsString("Where to?")); } private String getRefreshTokenResponse(ServerRunning serverRunning, String refreshToken) { From ace865b20c7ec585be73587a0f48f6241db0c90d Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Wed, 15 Nov 2023 09:47:53 -0800 Subject: [PATCH 6/7] improve test coverage - based on Sonar scan suggestion [#184752215] --- .../ZoneAwareWhitelistLogoutHandlerTests.java | 8 ++++++++ 1 file changed, 8 insertions(+) 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 62ea712e817..0817f932cea 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 @@ -178,6 +178,14 @@ public void test_does_not_external_logout() throws ServletException, IOException verify(oAuthLogoutHandler, times(0)).onLogoutSuccess(request, response, null); } + @Test + public void test_does_not_external_logout_when_logout_url_is_null() throws ServletException, IOException { + when(oAuthLogoutHandler.getLogoutUrl(null)).thenReturn(null); + when(oAuthLogoutHandler.getPerformRpInitiatedLogout(null)).thenReturn(true); + handler.onLogoutSuccess(request, response, null); + verify(oAuthLogoutHandler, times(0)).onLogoutSuccess(request, response, null); + } + @Test public void test_logout() throws ServletException, IOException { handler.onLogoutSuccess(request, response, null); From c52a0b9e69593f0110c60ccda4f01527eae5ee40 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Wed, 15 Nov 2023 12:08:34 -0800 Subject: [PATCH 7/7] refactor: address issue "Generic types should not be used raw (without type parameters)." - based on Sonar scan suggestion [#184752215] --- .../uaa/authentication/ZoneAwareWhitelistLogoutHandler.java | 3 ++- .../uaa/provider/oauth/ExternalOAuthLogoutHandler.java | 2 +- 2 files changed, 3 insertions(+), 2 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 4d3013589c5..66a2ef4e2a7 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 @@ -17,6 +17,7 @@ import org.cloudfoundry.identity.uaa.oauth.KeyInfoService; import org.cloudfoundry.identity.uaa.provider.AbstractExternalOAuthIdentityProviderDefinition; +import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.oauth.ExternalOAuthLogoutHandler; import org.cloudfoundry.identity.uaa.zone.MultitenantClientServices; import org.cloudfoundry.identity.uaa.zone.IdentityZoneConfiguration; @@ -45,7 +46,7 @@ public ZoneAwareWhitelistLogoutHandler(MultitenantClientServices clientDetailsSe @Override public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException, ServletException { - AbstractExternalOAuthIdentityProviderDefinition oauthConfig = externalOAuthLogoutHandler.getOAuthProviderForAuthentication(authentication); + AbstractExternalOAuthIdentityProviderDefinition oauthConfig = externalOAuthLogoutHandler.getOAuthProviderForAuthentication(authentication); String logoutUrl = externalOAuthLogoutHandler.getLogoutUrl(oauthConfig); boolean shouldPerformRpInitiatedLogout = externalOAuthLogoutHandler.getPerformRpInitiatedLogout(oauthConfig); 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 67e10bc4d34..417e2f40aa4 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 @@ -124,7 +124,7 @@ private String getZoneDefaultUrl() { return config.getLinks().getLogout().getRedirectUrl(); } - public boolean getPerformRpInitiatedLogout(AbstractExternalOAuthIdentityProviderDefinition oauthConfig) { + public boolean getPerformRpInitiatedLogout(AbstractExternalOAuthIdentityProviderDefinition oauthConfig) { if (oauthConfig == null) { return false; }