From c15753266ff7e86af82ff2a4a82fc26f0cbcc16f Mon Sep 17 00:00:00 2001 From: Maria Arias de Reyna Date: Tue, 19 Sep 2023 17:09:45 +0200 Subject: [PATCH] fix(Closes #21236): Adding client-id to logout event --- .../protocol/oidc/OIDCLoginProtocol.java | 1 + .../oidc/endpoints/LogoutEndpoint.java | 1 + .../org/keycloak/testsuite/AssertEvents.java | 2 +- .../servlet/DemoServletsAdapterTest.java | 1 + .../testsuite/client/ClientRedirectTest.java | 2 +- .../testsuite/forms/ResetPasswordTest.java | 6 +++++- .../testsuite/oauth/LegacyLogoutTest.java | 8 ++++---- .../keycloak/testsuite/oauth/LogoutTest.java | 5 ++++- .../testsuite/oauth/OAuthGrantTest.java | 2 +- .../oauth/RPInitiatedLogoutTest.java | 20 ++++++++++++------- .../webauthn/WebAuthnIdlessTest.java | 4 ++++ .../WebAuthnRegisterAndLoginTest.java | 3 +++ 12 files changed, 39 insertions(+), 16 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java index fbbc5e78017..5d845a47c97 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java @@ -384,6 +384,7 @@ public class OIDCLoginProtocol implements LoginProtocol { @Override public Response finishBrowserLogout(UserSessionModel userSession, AuthenticationSessionModel logoutSession) { event.event(EventType.LOGOUT); + event.client(logoutSession.getClient()); String redirectUri = logoutSession.getAuthNote(OIDCLoginProtocol.LOGOUT_REDIRECT_URI); if (redirectUri != null) { diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java index a07172e38e2..c91267d7a25 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java @@ -232,6 +232,7 @@ public class LogoutEndpoint { } if (client != null) { session.getContext().setClient(client); + event.client(client); } String validatedRedirectUri = null; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AssertEvents.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AssertEvents.java index c03327296a7..cde4c4d185d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AssertEvents.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AssertEvents.java @@ -175,7 +175,7 @@ public class AssertEvents implements TestRule { } public ExpectedEvent expectLogout(String sessionId) { - return expect(EventType.LOGOUT).client((String) null) + return expect(EventType.LOGOUT) .detail(Details.REDIRECT_URI, Matchers.equalTo(DEFAULT_REDIRECT_URI)) .session(sessionId); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java index 87bbc6c6402..d34b4322594 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/DemoServletsAdapterTest.java @@ -1100,6 +1100,7 @@ public class DemoServletsAdapterTest extends AbstractServletsAdapterTest { assertEvents.expectLogout(null) .realm(realm.getId()) .user(userId) + .client("account") .session(AssertEvents.isUUID()) .removeDetail(Details.REDIRECT_URI) .assertEvent(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRedirectTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRedirectTest.java index 1e03c830da6..22b40954f68 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRedirectTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRedirectTest.java @@ -119,7 +119,7 @@ public class ClientRedirectTest extends AbstractTestRealmKeycloakTest { log.debug("Current URL: " + driver.getCurrentUrl()); log.debug("check logout_error"); - events.expectLogoutError(OAuthErrorException.INVALID_REDIRECT_URI).assertEvent(); + events.expectLogoutError(OAuthErrorException.INVALID_REDIRECT_URI).client(AssertEvents.DEFAULT_CLIENT_ID).assertEvent(); assertThat(driver.getCurrentUrl(), is(not(equalTo("http://example.org/redirected")))); } finally { log.debug("removing disabled-client"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java index 6f82c4b4e34..e87d5dfe84d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java @@ -1293,7 +1293,11 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest { logoutConfirmPage.assertCurrent(); logoutConfirmPage.confirmLogout(); - events.expectLogout(sessionId).user(user.getId()).removeDetail(Details.REDIRECT_URI).assertEvent(); + events.expectLogout(sessionId) + .client("account") + .user(user.getId()) + .removeDetail(Details.REDIRECT_URI) + .assertEvent(); } BrowserTabUtil util = BrowserTabUtil.getInstanceAndSetEnv(driver); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LegacyLogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LegacyLogoutTest.java index c8770262b0b..516a4a33201 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LegacyLogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LegacyLogoutTest.java @@ -144,7 +144,7 @@ public class LegacyLogoutTest extends AbstractTestRealmKeycloakTest { logoutConfirmPage.confirmLogout(); // Redirected back to the application with expected state - events.expectLogout(sessionId).removeDetail(Details.REDIRECT_URI).assertEvent(); + events.expectLogout(sessionId).client("account").removeDetail(Details.REDIRECT_URI).assertEvent(); assertThat(false, is(isSessionActive(sessionId))); assertCurrentUrlEquals(APP_REDIRECT_URI); } @@ -165,7 +165,7 @@ public class LegacyLogoutTest extends AbstractTestRealmKeycloakTest { logoutConfirmPage.confirmLogout(); // Redirected back to the application with expected state - events.expectLogout(sessionId).removeDetail(Details.REDIRECT_URI).assertEvent(); + events.expectLogout(sessionId).client("account").removeDetail(Details.REDIRECT_URI).assertEvent(); assertThat(false, is(isSessionActive(sessionId))); assertCurrentUrlEquals(APP_REDIRECT_URI); } @@ -230,7 +230,7 @@ public class LegacyLogoutTest extends AbstractTestRealmKeycloakTest { logoutConfirmPage.confirmLogout(); // Redirected back to the application with expected state - events.expectLogout(sessionId).removeDetail(Details.REDIRECT_URI).assertEvent(); + events.expectLogout(sessionId).client("account").removeDetail(Details.REDIRECT_URI).assertEvent(); MatcherAssert.assertThat(false, is(isSessionActive(sessionId))); assertCurrentUrlEquals(APP_REDIRECT_URI); } @@ -248,7 +248,7 @@ public class LegacyLogoutTest extends AbstractTestRealmKeycloakTest { String logoutUrl = oauth.getLogoutUrl().postLogoutRedirectUri(APP_REDIRECT_URI).build(); driver.navigate().to(logoutUrl); - events.expectLogout(sessionId).detail(Details.REDIRECT_URI, APP_REDIRECT_URI).assertEvent(); + events.expectLogout(sessionId).client("account").detail(Details.REDIRECT_URI, APP_REDIRECT_URI).assertEvent(); assertThat(false, is(isSessionActive(sessionId))); assertCurrentUrlEquals(APP_REDIRECT_URI); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LogoutTest.java index b7cef62c9bd..cffcf158e4b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LogoutTest.java @@ -293,7 +293,10 @@ public class LogoutTest extends AbstractKeycloakTest { } // Assert logout event triggered for backchannel logout - events.expectLogout(sessionId).detail(Details.REDIRECT_URI, oauth.APP_AUTH_ROOT).assertEvent(); + events.expectLogout(sessionId) + .client(AssertEvents.DEFAULT_CLIENT_ID) + .detail(Details.REDIRECT_URI, oauth.APP_AUTH_ROOT) + .assertEvent(); assertNotNull(testingClient.testApp().getAdminLogoutAction()); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthGrantTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthGrantTest.java index 648a6c165fc..2519d7d3d3b 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthGrantTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthGrantTest.java @@ -360,7 +360,7 @@ public class OAuthGrantTest extends AbstractKeycloakTest { String logoutUrl = oauth.getLogoutUrl().idTokenHint(res.getIdToken()).build(); driver.navigate().to(logoutUrl); - events.expectLogout(loginEvent.getSessionId()).removeDetail(Details.REDIRECT_URI).assertEvent(); + events.expectLogout(loginEvent.getSessionId()).client(THIRD_PARTY_APP).removeDetail(Details.REDIRECT_URI).assertEvent(); // login again to check whether the Dynamic scope and only the dynamic scope is requested again oauth.scope("foo-dynamic-scope:withparam"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RPInitiatedLogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RPInitiatedLogoutTest.java index 19fc9df2b7f..914c5d4c79f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RPInitiatedLogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RPInitiatedLogoutTest.java @@ -452,13 +452,19 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { // Completely invalid redirect uri driver.navigate().to(oauth.getLogoutUrl().postLogoutRedirectUri("https://invalid").idTokenHint(idTokenString).build()); errorPage.assertCurrent(); - events.expectLogoutError(OAuthErrorException.INVALID_REDIRECT_URI).detail(Details.REDIRECT_URI, "https://invalid").assertEvent(); + events.expectLogoutError(OAuthErrorException.INVALID_REDIRECT_URI) + .client(AssertEvents.DEFAULT_CLIENT_ID) + .detail(Details.REDIRECT_URI, "https://invalid") + .assertEvent(); // Redirect uri of different client in the realm should fail as well String rootUrlClientRedirectUri = UriUtils.getOrigin(APP_REDIRECT_URI) + "/foo/bar"; driver.navigate().to(oauth.getLogoutUrl().postLogoutRedirectUri(rootUrlClientRedirectUri).idTokenHint(idTokenString).build()); errorPage.assertCurrent(); - events.expectLogoutError(OAuthErrorException.INVALID_REDIRECT_URI).detail(Details.REDIRECT_URI, rootUrlClientRedirectUri).assertEvent(); + events.expectLogoutError(OAuthErrorException.INVALID_REDIRECT_URI) + .client(AssertEvents.DEFAULT_CLIENT_ID) + .detail(Details.REDIRECT_URI, rootUrlClientRedirectUri) + .assertEvent(); // Session still authenticated MatcherAssert.assertThat(true, is(isSessionActive(tokenResponse.getSessionState()))); @@ -510,7 +516,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { // expected } - events.expectLogout(tokenResponse.getSessionState()).removeDetail(Details.REDIRECT_URI).assertEvent(); + events.expectLogout(tokenResponse.getSessionState()).client("account").removeDetail(Details.REDIRECT_URI).assertEvent(); MatcherAssert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState()))); } @@ -600,7 +606,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { // Logout confirmation page not shown as id_token_hint was included. // Redirected back to the application with expected "state" - events.expectLogout(tokenResponse.getSessionState()).assertEvent(); + events.expectLogout(tokenResponse.getSessionState()).client("third-party").assertEvent(); MatcherAssert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState()))); assertCurrentUrlEquals(APP_REDIRECT_URI + "?state=somethingg"); @@ -762,7 +768,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { // expected } - events.expectLogout(tokenResponse.getSessionState()).removeDetail(Details.REDIRECT_URI).assertEvent(); + events.expectLogout(tokenResponse.getSessionState()).client("account").removeDetail(Details.REDIRECT_URI).assertEvent(); MatcherAssert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState()))); } @@ -837,7 +843,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { Assert.assertEquals("Deutsch", logoutConfirmPage.getLanguageDropdownText()); logoutConfirmPage.confirmLogout(); WaitUtils.waitForPageToLoad(); - events.expectLogout(tokenResponse.getSessionState()).removeDetail(Details.REDIRECT_URI).assertEvent(); + events.expectLogout(tokenResponse.getSessionState()).client("account").removeDetail(Details.REDIRECT_URI).assertEvent(); // Remove ui_locales from logout request. Default locale should be set tokenResponse = loginUser(); @@ -846,7 +852,7 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest { Assert.assertEquals("English", logoutConfirmPage.getLanguageDropdownText()); logoutConfirmPage.confirmLogout(); WaitUtils.waitForPageToLoad(); - events.expectLogout(tokenResponse.getSessionState()).removeDetail(Details.REDIRECT_URI).assertEvent(); + events.expectLogout(tokenResponse.getSessionState()).client("account").removeDetail(Details.REDIRECT_URI).assertEvent(); } } diff --git a/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnIdlessTest.java b/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnIdlessTest.java index 190e05bab47..2d1d52b8342 100644 --- a/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnIdlessTest.java +++ b/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnIdlessTest.java @@ -248,6 +248,7 @@ public class WebAuthnIdlessTest extends AbstractWebAuthnVirtualTest { events.expectLogout(sessionId) .removeDetail(Details.REDIRECT_URI) .user(userId) + .client("account") .assertEvent(); return credentialId; } @@ -304,6 +305,7 @@ public class WebAuthnIdlessTest extends AbstractWebAuthnVirtualTest { events.expectLogout(sessionId) .removeDetail(Details.REDIRECT_URI) .user(userId) + .client("account") .assertEvent(); } @@ -334,6 +336,7 @@ public class WebAuthnIdlessTest extends AbstractWebAuthnVirtualTest { events.expectLogout(sessionId) .removeDetail(Details.REDIRECT_URI) .user(userId) + .client("account") .assertEvent(); } @@ -367,6 +370,7 @@ public class WebAuthnIdlessTest extends AbstractWebAuthnVirtualTest { events.expectLogout(sessionId) .removeDetail(Details.REDIRECT_URI) .user(userId) + .client("account") .assertEvent(); } else { diff --git a/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnRegisterAndLoginTest.java b/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnRegisterAndLoginTest.java index b1c2b5ff35e..bb660e1b65d 100644 --- a/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnRegisterAndLoginTest.java +++ b/testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnRegisterAndLoginTest.java @@ -151,6 +151,7 @@ public class WebAuthnRegisterAndLoginTest extends AbstractWebAuthnVirtualTest { events.expectLogout(sessionId) .removeDetail(Details.REDIRECT_URI) .user(userId) + .client("account") .assertEvent(); // login by user @@ -183,6 +184,7 @@ public class WebAuthnRegisterAndLoginTest extends AbstractWebAuthnVirtualTest { // confirm logout event events.expectLogout(sessionId) .removeDetail(Details.REDIRECT_URI) + .client("account") .user(userId) .assertEvent(); } finally { @@ -257,6 +259,7 @@ public class WebAuthnRegisterAndLoginTest extends AbstractWebAuthnVirtualTest { events.expectLogout(sessionID) .removeDetail(Details.REDIRECT_URI) .user(userId) + .client("account") .assertEvent(); // Password + WebAuthn security key