Support for client_id parameter in OIDC RP-Initiated logout endpoint (#12202)

Closes #12002


Co-authored-by: Martin Bartoš <mabartos@redhat.com>
This commit is contained in:
Marek Posolda 2022-05-27 14:12:37 +02:00 committed by GitHub
parent 063960aaa3
commit cf386efa40
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 232 additions and 41 deletions

View file

@ -478,8 +478,12 @@ function Keycloak (config) {
kc.createLogoutUrl = function(options) {
var url = kc.endpoints.logout()
+ '?post_logout_redirect_uri=' + encodeURIComponent(adapter.redirectUri(options, false))
+ '&id_token_hint=' + encodeURIComponent(kc.idToken);
+ '?client_id=' + encodeURIComponent(kc.clientId)
+ '&post_logout_redirect_uri=' + encodeURIComponent(adapter.redirectUri(options, false));
if (kc.idToken) {
url += '&id_token_hint=' + encodeURIComponent(kc.idToken);
}
return url;
}

View file

@ -143,6 +143,7 @@ public class LogoutEndpoint {
*
* @param deprecatedRedirectUri Parameter "redirect_uri" is not supported by the specification. It is here just for the backwards compatibility
* @param encodedIdToken Parameter "id_token_hint" as described in the specification.
* @param clientId Parameter "client_id" as described in the specification.
* @param postLogoutRedirectUri Parameter "post_logout_redirect_uri" as described in the specification with the URL to redirect after logout.
* @param state Parameter "state" as described in the specification. Will be used to send "state" when redirecting back to the application after the logout
* @param uiLocales Parameter "ui_locales" as described in the specification. Can be used by the client to display pages in specified locale (if any pages are going to be displayed to the user during logout)
@ -153,6 +154,7 @@ public class LogoutEndpoint {
@NoCache
public Response logout(@QueryParam(OIDCLoginProtocol.REDIRECT_URI_PARAM) String deprecatedRedirectUri, // deprecated
@QueryParam(OIDCLoginProtocol.ID_TOKEN_HINT) String encodedIdToken,
@QueryParam(OIDCLoginProtocol.CLIENT_ID_PARAM) String clientId,
@QueryParam(OIDCLoginProtocol.POST_LOGOUT_REDIRECT_URI_PARAM) String postLogoutRedirectUri,
@QueryParam(OIDCLoginProtocol.STATE_PARAM) String state,
@QueryParam(OIDCLoginProtocol.UI_LOCALES_PARAM) String uiLocales,
@ -166,13 +168,21 @@ public class LogoutEndpoint {
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_PARAMETER, OIDCLoginProtocol.REDIRECT_URI_PARAM);
}
if (postLogoutRedirectUri != null && encodedIdToken == null) {
if (postLogoutRedirectUri != null && encodedIdToken == null && clientId == null) {
event.event(EventType.LOGOUT);
event.error(Errors.INVALID_REQUEST);
logger.warnf("Parameter 'id_token_hint' is required when 'post_logout_redirect_uri' is used.");
logger.warnf("Either the parameter 'client_id' or the parameter 'id_token_hint' is required when 'post_logout_redirect_uri' is used.");
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.MISSING_PARAMETER, OIDCLoginProtocol.ID_TOKEN_HINT);
}
boolean confirmationNeeded = true;
boolean forcedConfirmation = false;
ClientModel client = clientId == null ? null : realm.getClientByClientId(clientId);
if (clientId != null && client == null) {
logger.warnf("Client '%s' not found.", clientId);
forcedConfirmation = true;
}
IDToken idToken = null;
if (encodedIdToken != null) {
try {
@ -185,7 +195,26 @@ public class LogoutEndpoint {
}
}
ClientModel client = (idToken == null || idToken.getIssuedFor() == null) ? null : realm.getClientByClientId(idToken.getIssuedFor());
if (clientId == null) {
// Retrieve client from id_token_hint
client = (idToken == null || idToken.getIssuedFor() == null) ? null : realm.getClientByClientId(idToken.getIssuedFor());
if (client != null) {
confirmationNeeded = false;
}
} else {
// Check client_id and id_token_hint point to the same client
if (idToken != null && idToken.getIssuedFor() != null) {
if (!idToken.getIssuedFor().equals(clientId)) {
event.event(EventType.LOGOUT);
event.client(clientId);
event.error(Errors.INVALID_TOKEN);
logger.warnf("Parameter client_id is different than the client for which ID Token was issued. Parameter client_id: '%s', ID Token issued for: '%s'.", clientId, idToken.getIssuedFor());
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_PARAMETER, OIDCLoginProtocol.ID_TOKEN_HINT);
} else {
confirmationNeeded = false;
}
}
}
if (client != null) {
session.getContext().setClient(client);
}
@ -229,8 +258,22 @@ public class LogoutEndpoint {
LoginFormsProvider loginForm = session.getProvider(LoginFormsProvider.class)
.setAuthenticationSession(logoutSession);
// Client was not sent in id_token_hint or has consentRequired. Logout confirmation screen will be displayed to the user in this case
if (client == null || client.isConsentRequired()) {
// Check if we have session in the browser. If yes and it is different session than referenced by id_token_hint, the confirmation should be displayed
AuthenticationManager.AuthResult authResult = AuthenticationManager.authenticateIdentityCookie(session, realm, false);
if (authResult != null) {
if (idToken != null && idToken.getSessionState() != null && !idToken.getSessionState().equals(authResult.getSession().getId())) {
forcedConfirmation = true;
}
} else {
// Skip confirmation in case that valid redirect URI was setup for given client_id and there is no session in the browser as well as no id_token_hint.
// We can do automatic redirect as there is no logout needed at all for this scenario (Session was probably already logged-out before)
if (encodedIdToken == null && client != null && validatedRedirectUri != null) {
confirmationNeeded = false;
}
}
// Logout confirmation screen will be displayed to the user in this case
if (confirmationNeeded || forcedConfirmation) {
return displayLogoutConfirmationScreen(loginForm, logoutSession);
} else {
return doBrowserLogout(logoutSession);

View file

@ -206,6 +206,13 @@ public class OAuthClient {
public class LogoutUrlBuilder {
private final UriBuilder b = OIDCLoginProtocolService.logoutUrl(UriBuilder.fromUri(baseUrl));
public LogoutUrlBuilder clientId(String clientId) {
if (clientId != null) {
b.queryParam(OIDCLoginProtocol.CLIENT_ID_PARAM, clientId);
}
return this;
}
public LogoutUrlBuilder idTokenHint(String idTokenHint) {
if (idTokenHint != null) {
b.queryParam(OIDCLoginProtocol.ID_TOKEN_HINT, idTokenHint);

View file

@ -118,6 +118,7 @@ import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
import org.keycloak.testsuite.client.resources.TestApplicationResourceUrls;
import org.keycloak.testsuite.client.resources.TestOIDCEndpointsApplicationResource;
import org.keycloak.testsuite.pages.ErrorPage;
import org.keycloak.testsuite.pages.LogoutConfirmPage;
import org.keycloak.testsuite.pages.OAuth2DeviceVerificationPage;
import org.keycloak.testsuite.pages.OAuthGrantPage;
import org.keycloak.testsuite.rest.resource.TestingOIDCEndpointsApplicationResource.AuthorizationEndpointRequestObject;
@ -197,6 +198,9 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest {
@Page
protected ErrorPage errorPage;
@Page
protected LogoutConfirmPage logoutConfirmPage;
@Override
public void addTestRealms(List<RealmRepresentation> testRealms) {
RealmRepresentation realm = loadJson(getClass().getResourceAsStream("/testrealm.json"), RealmRepresentation.class);
@ -2939,7 +2943,6 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest {
} catch (IOException ioe) {
throw new RuntimeException(ioe);
}
String idTokenHint = accessTokenResponse.getIdToken();
assertEquals(200, accessTokenResponse.getStatusCode());
// Check token refresh.
@ -2996,7 +2999,9 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest {
assertEquals(OAuthErrorException.INVALID_GRANT, accessTokenResponse.getError());
// Check frontchannel logout and login.
oauth.idTokenHint(idTokenHint).openLogout();
driver.navigate().to(oauth.getLogoutUrl().build());
logoutConfirmPage.assertCurrent();
logoutConfirmPage.confirmLogout();
loginResponse = oauth.doLogin(TEST_USER_NAME, TEST_USER_PASSWORD);
Assert.assertNull(loginResponse.getError());

View file

@ -48,6 +48,7 @@ import org.keycloak.testsuite.pages.LoginPasswordResetPage;
import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
import org.keycloak.testsuite.pages.LoginTotpPage;
import org.keycloak.testsuite.pages.LoginUsernameOnlyPage;
import org.keycloak.testsuite.pages.LogoutConfirmPage;
import org.keycloak.testsuite.pages.PasswordPage;
import org.keycloak.testsuite.pages.RegisterPage;
import org.keycloak.testsuite.util.FlowUtil;
@ -110,6 +111,9 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl
@Page
protected LoginTotpPage loginTotpPage;
@Page
protected LogoutConfirmPage logoutConfirmPage;
@Page
protected ErrorPage errorPage;
@ -452,7 +456,9 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl
accountTotpPage.removeTotp();
// Logout
oauth.openLogout();
driver.navigate().to(oauth.getLogoutUrl().build());
logoutConfirmPage.assertCurrent();
logoutConfirmPage.confirmLogout();
/* Verify the 'Device Name' is optional when creating the first OTP credential via the login config TOTP page */
@ -490,7 +496,9 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl
.map(WebElement::getText).collect(Collectors.toList()), Matchers.hasItem(""));;
// Logout
oauth.openLogout();
driver.navigate().to(oauth.getLogoutUrl().build());
logoutConfirmPage.assertCurrent();
logoutConfirmPage.confirmLogout();
/* Verify the 'Device Name' is required for each next OTP credential created via the login config TOTP page */
@ -544,7 +552,9 @@ public class ResetCredentialsAlternativeFlowsTest extends AbstractTestRealmKeycl
accountTotpPage.removeTotp();
// Logout
oauth.openLogout();
driver.navigate().to(oauth.getLogoutUrl().build());
logoutConfirmPage.assertCurrent();
logoutConfirmPage.confirmLogout();
// Undo setup changes performed within the test
} finally {

View file

@ -367,8 +367,6 @@ public class OAuthGrantTest extends AbstractKeycloakTest {
String logoutUrl = oauth.getLogoutUrl().idTokenHint(res.getIdToken()).build();
driver.navigate().to(logoutUrl);
logoutConfirmPage.assertCurrent();
logoutConfirmPage.confirmLogout();
events.expectLogout(loginEvent.getSessionId()).removeDetail(Details.REDIRECT_URI).assertEvent();

View file

@ -144,6 +144,37 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest {
assertCurrentUrlEquals(redirectUri);
tokenResponse = loginUser();
String sessionId2 = tokenResponse.getSessionState();
idTokenString = tokenResponse.getIdToken();
assertNotEquals(sessionId, sessionId2);
// Test also "state" parameter is included in the URL after logout. Make sure to use idTokenHint from the last login to match with current browser session
logoutUrl = oauth.getLogoutUrl().postLogoutRedirectUri(redirectUri).idTokenHint(idTokenString).state("something").build();
driver.navigate().to(logoutUrl);
events.expectLogout(sessionId2).detail(Details.REDIRECT_URI, redirectUri).assertEvent();
Assert.assertThat(false, is(isSessionActive(sessionId2)));
assertCurrentUrlEquals(redirectUri + "&state=something");
}
@Test
public void logoutRedirectWithIdTokenHintPointToDifferentSession() {
OAuthClient.AccessTokenResponse tokenResponse = loginUser();
String sessionId = tokenResponse.getSessionState();
String redirectUri = APP_REDIRECT_URI + "?logout";
String idTokenString = tokenResponse.getIdToken();
String logoutUrl = oauth.getLogoutUrl().postLogoutRedirectUri(redirectUri).idTokenHint(idTokenString).build();
driver.navigate().to(logoutUrl);
events.expectLogout(sessionId).detail(Details.REDIRECT_URI, redirectUri).assertEvent();
Assert.assertThat(false, is(isSessionActive(sessionId)));
assertCurrentUrlEquals(redirectUri);
loginPage.open();
loginPage.login("test-user@localhost", "password");
assertTrue(appPage.isCurrent());
@ -151,9 +182,11 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest {
String sessionId2 = events.expectLogin().assertEvent().getSessionId();
assertNotEquals(sessionId, sessionId2);
// Test also "state" parameter is included in the URL after logout
// Using idTokenHint of the 1st session. Logout confirmation is needed in such case. Test also "state" parameter is included in the URL after logout
logoutUrl = oauth.getLogoutUrl().postLogoutRedirectUri(redirectUri).idTokenHint(idTokenString).state("something").build();
driver.navigate().to(logoutUrl);
logoutConfirmPage.assertCurrent();
logoutConfirmPage.confirmLogout();
events.expectLogout(sessionId2).detail(Details.REDIRECT_URI, redirectUri).assertEvent();
Assert.assertThat(false, is(isSessionActive(sessionId2)));
assertCurrentUrlEquals(redirectUri + "&state=something");
@ -237,12 +270,12 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest {
events.assertEmpty();
assertCurrentUrlEquals(APP_REDIRECT_URI);
loginPage.open();
loginPage.login("test-user@localhost", "password");
assertTrue(appPage.isCurrent());
String sessionId2 = events.expectLogin().assertEvent().getSessionId();
// Login again in the browser. Ensure to use newest idTokenHint after logout
tokenResponse = loginUser();
String sessionId2 = tokenResponse.getSessionState();
idTokenString = tokenResponse.getIdToken();
assertNotEquals(sessionId, sessionId2);
logoutUrl = oauth.getLogoutUrl().postLogoutRedirectUri(APP_REDIRECT_URI).idTokenHint(idTokenString).build();
driver.navigate().to(logoutUrl);
events.expectLogout(sessionId2).detail(Details.REDIRECT_URI, APP_REDIRECT_URI).assertEvent();
@ -518,15 +551,9 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest {
String logoutUrl = oauth.getLogoutUrl().postLogoutRedirectUri(APP_REDIRECT_URI).idTokenHint(idTokenString).state("somethingg").build();
driver.navigate().to(logoutUrl);
// Assert logout confirmation page. Session still exists. Assert default language on logout page (English)
logoutConfirmPage.assertCurrent();
Assert.assertEquals("English", logoutConfirmPage.getLanguageDropdownText());
Assert.assertThat(true, is(isSessionActive(tokenResponse.getSessionState())));
events.assertEmpty();
logoutConfirmPage.confirmLogout();
// Logout confirmation page not shown as id_token_hint was included.
// Redirected back to the application with expected "state"
events.expectLogout(tokenResponse.getSessionState()).removeDetail(Details.REDIRECT_URI).assertEvent();
events.expectLogout(tokenResponse.getSessionState()).assertEvent();
Assert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState())));
assertCurrentUrlEquals(APP_REDIRECT_URI + "?state=somethingg");
@ -535,15 +562,13 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest {
}
// Test logout request without "post logout redirect uri" . Also test "ui_locales" parameter works as expected
// Test logout request with only "client_id" parameter. Also test "ui_locales" parameter works as expected
@Test
public void logoutConsentRequiredWithoutPostLogoutRedirectUri() throws IOException {
public void logoutWithUiLocalesAndClientIdParameter() throws IOException {
try (RealmAttributeUpdater updater = new RealmAttributeUpdater(testRealm()).addSupportedLocale("cs").update()) {
oauth.clientId("third-party");
OAuthClient.AccessTokenResponse tokenResponse = loginUser(true);
String idTokenString = tokenResponse.getIdToken();
OAuthClient.AccessTokenResponse tokenResponse = loginUser(false);
String logoutUrl = oauth.getLogoutUrl().idTokenHint(idTokenString).uiLocales("cs").build();
String logoutUrl = oauth.getLogoutUrl().clientId("test-app").uiLocales("cs").build();
driver.navigate().to(logoutUrl);
// Assert logout confirmation page. Session still exists. Assert czech language on logout page
@ -562,19 +587,15 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest {
infoPage.clickBackToApplicationLinkCs();
WaitUtils.waitForPageToLoad();
Assert.assertThat(driver.getCurrentUrl(), endsWith("/app/auth"));
UserResource user = ApiUtil.findUserByUsernameId(testRealm(), "test-user@localhost");
user.revokeConsent("third-party");
}
}
@Test
public void logoutConsentRequiredWithExpiredCode() throws IOException {
oauth.clientId("third-party");
OAuthClient.AccessTokenResponse tokenResponse = loginUser(true);
public void logoutWithClientIdAndExpiredCode() throws IOException {
OAuthClient.AccessTokenResponse tokenResponse = loginUser();
String idTokenString = tokenResponse.getIdToken();
driver.navigate().to(oauth.getLogoutUrl().idTokenHint(idTokenString).build());
driver.navigate().to(oauth.getLogoutUrl().clientId("test-app").build());
// Assert logout confirmation page. Session still exists
logoutConfirmPage.assertCurrent();
@ -597,6 +618,109 @@ public class RPInitiatedLogoutTest extends AbstractTestRealmKeycloakTest {
}
@Test
public void logoutWithClientIdAndWithoutIdTokenHint() {
OAuthClient.AccessTokenResponse tokenResponse = loginUser();
String logoutUrl = oauth.getLogoutUrl().postLogoutRedirectUri(APP_REDIRECT_URI).clientId("test-app").state("somethingg").build();
driver.navigate().to(logoutUrl);
// Assert logout confirmation page as id_token_hint was not sent. Session still exists. Assert default language on logout page (English)
logoutConfirmPage.assertCurrent();
Assert.assertEquals("English", logoutConfirmPage.getLanguageDropdownText());
Assert.assertThat(true, is(isSessionActive(tokenResponse.getSessionState())));
events.assertEmpty();
logoutConfirmPage.confirmLogout();
// Redirected back to the application with expected "state"
events.expectLogout(tokenResponse.getSessionState()).assertEvent();
Assert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState())));
assertCurrentUrlEquals(APP_REDIRECT_URI + "?state=somethingg");
}
@Test
public void logoutWithClientIdIdTokenHintAndPostLogoutRedirectUri() {
OAuthClient.AccessTokenResponse tokenResponse = loginUser();
// Test logout with all of "client_id", "id_token_hint" and "post_logout_redirect_uri". Logout should work without confirmation
String logoutUrl = oauth.getLogoutUrl()
.postLogoutRedirectUri(APP_REDIRECT_URI)
.clientId("test-app")
.idTokenHint(tokenResponse.getIdToken())
.state("somethingg").build();
driver.navigate().to(logoutUrl);
// Logout done and redirected back to the application with expected "state"
events.expectLogout(tokenResponse.getSessionState()).assertEvent();
Assert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState())));
assertCurrentUrlEquals(APP_REDIRECT_URI + "?state=somethingg");
// Test logout only with "client_id" and "post_logout_redirect_uri". Should automatically redirect as there is no logout (No active browser session)
logoutUrl = oauth.getLogoutUrl()
.postLogoutRedirectUri(APP_REDIRECT_URI)
.clientId("test-app")
.state("something2").build();
driver.navigate().to(logoutUrl);
events.assertEmpty();
assertCurrentUrlEquals(APP_REDIRECT_URI + "?state=something2");
}
@Test
public void logoutWithBadClientId() {
OAuthClient.AccessTokenResponse tokenResponse = loginUser();
// Case when client_id points to different client than ID Token.
String logoutUrl = oauth.getLogoutUrl()
.postLogoutRedirectUri(APP_REDIRECT_URI)
.clientId("third-party")
.idTokenHint(tokenResponse.getIdToken()).build();
driver.navigate().to(logoutUrl);
errorPage.assertCurrent();
Assert.assertEquals("Invalid parameter: id_token_hint", errorPage.getError());
events.expectLogoutError(Errors.INVALID_TOKEN).client("third-party").assertEvent();
Assert.assertThat(true, is(isSessionActive(tokenResponse.getSessionState())));
// Case when client_id is non-existing client and redirect uri of different client is used
logoutUrl = oauth.getLogoutUrl()
.postLogoutRedirectUri(APP_REDIRECT_URI)
.clientId("non-existing").build();
driver.navigate().to(logoutUrl);
errorPage.assertCurrent();
Assert.assertEquals("Invalid redirect uri", errorPage.getError());
events.expectLogoutError(Errors.INVALID_REDIRECT_URI).assertEvent();
Assert.assertThat(true, is(isSessionActive(tokenResponse.getSessionState())));
// Case when client_id is non-existing client. Confirmation is needed.
logoutUrl = oauth.getLogoutUrl()
.clientId("non-existing").build();
driver.navigate().to(logoutUrl);
logoutConfirmPage.assertCurrent();
logoutConfirmPage.confirmLogout();
// Info page present. No link "back to the application"
infoPage.assertCurrent();
Assert.assertEquals("You are logged out", infoPage.getInfo());
try {
logoutConfirmPage.clickBackToApplicationLink();
fail();
}
catch (NoSuchElementException ex) {
// expected
}
events.expectLogout(tokenResponse.getSessionState()).removeDetail(Details.REDIRECT_URI).assertEvent();
Assert.assertThat(false, is(isSessionActive(tokenResponse.getSessionState())));
}
@Test
public void testFrontChannelLogoutWithPostLogoutRedirectUri() throws Exception {
ClientsResource clients = adminClient.realm(oauth.getRealm()).clients();