KEYCLOAK-16006 User should not be required to re-authenticate after revoking consent to an application

This commit is contained in:
mposolda 2021-02-09 20:34:37 +01:00 committed by Marek Posolda
parent 37cb1ba310
commit 0058011265
11 changed files with 128 additions and 26 deletions

View file

@ -497,18 +497,13 @@ public class TokenManager {
} }
public static void dettachClientSession(UserSessionProvider sessions, RealmModel realm, AuthenticatedClientSessionModel clientSession) { public static void dettachClientSession(AuthenticatedClientSessionModel clientSession) {
UserSessionModel userSession = clientSession.getUserSession(); UserSessionModel userSession = clientSession.getUserSession();
if (userSession == null) { if (userSession == null) {
return; return;
} }
clientSession.detachFromUserSession(); clientSession.detachFromUserSession();
// TODO: Might need optimization to prevent loading client sessions from cache in getAuthenticatedClientSessions()
if (userSession.getAuthenticatedClientSessions().isEmpty()) {
sessions.removeUserSession(realm, userSession);
}
} }

View file

@ -228,7 +228,17 @@ public class TokenRevocationEndpoint {
.map(userSession -> userSession.getAuthenticatedClientSessionByClient(client.getId())) .map(userSession -> userSession.getAuthenticatedClientSessionByClient(client.getId()))
.filter(Objects::nonNull) .filter(Objects::nonNull)
.collect(Collectors.toList()) // collect to avoid concurrent modification as dettachClientSession removes the user sessions. .collect(Collectors.toList()) // collect to avoid concurrent modification as dettachClientSession removes the user sessions.
.forEach(clientSession -> TokenManager.dettachClientSession(session.sessions(), realm, clientSession)); .forEach(clientSession -> {
UserSessionModel userSession = clientSession.getUserSession();
TokenManager.dettachClientSession(clientSession);
if (userSession != null) {
// TODO: Might need optimization to prevent loading client sessions from cache in getAuthenticatedClientSessions()
if (userSession.getAuthenticatedClientSessions().isEmpty()) {
session.sessions().removeUserSession(realm, userSession);
}
}
});
} }
private void revokeAccessToken() { private void revokeAccessToken() {

View file

@ -555,7 +555,7 @@ public class AuthenticationManager {
.forEach(clientSession -> { .forEach(clientSession -> {
backchannelLogoutClientSession(session, realm, clientSession, null, uriInfo, headers); backchannelLogoutClientSession(session, realm, clientSession, null, uriInfo, headers);
clientSession.setAction(AuthenticationSessionModel.Action.LOGGED_OUT.name()); clientSession.setAction(AuthenticationSessionModel.Action.LOGGED_OUT.name());
TokenManager.dettachClientSession(session.sessions(), realm, clientSession); TokenManager.dettachClientSession(clientSession);
}); });
} }

View file

@ -0,0 +1,51 @@
/*
* Copyright 2020 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
package org.keycloak.services.managers;
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class UserConsentManager {
/**
* Revoke consent of given user to given client
*
* @param session
* @param client
* @param user
* @return true if either consent or offlineToken was revoked
*/
public static boolean revokeConsentToClient(KeycloakSession session, ClientModel client, UserModel user) {
RealmModel realm = session.getContext().getRealm();
boolean revokedConsent = session.users().revokeConsentForClient(realm, user.getId(), client.getId());
boolean revokedOfflineToken = new UserSessionManager(session).revokeOfflineToken(user, client);
if (revokedConsent) {
// Logout clientSessions for this user and client
AuthenticationManager.backchannelLogoutUserFromClient(session, realm, user, client, session.getContext().getUri(), session.getContext().getRequestHeaders());
}
return revokedConsent || revokedOfflineToken;
}
}

View file

@ -65,6 +65,7 @@ import org.keycloak.services.managers.AppAuthManager;
import org.keycloak.services.managers.Auth; import org.keycloak.services.managers.Auth;
import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.services.managers.AuthenticationSessionManager; import org.keycloak.services.managers.AuthenticationSessionManager;
import org.keycloak.services.managers.UserConsentManager;
import org.keycloak.services.managers.UserSessionManager; import org.keycloak.services.managers.UserSessionManager;
import org.keycloak.services.messages.Messages; import org.keycloak.services.messages.Messages;
import org.keycloak.services.resources.AbstractSecuredLocalService; import org.keycloak.services.resources.AbstractSecuredLocalService;
@ -465,11 +466,7 @@ public class AccountFormService extends AbstractSecuredLocalService {
// Revoke grant in UserModel // Revoke grant in UserModel
UserModel user = auth.getUser(); UserModel user = auth.getUser();
session.users().revokeConsentForClient(realm, user.getId(), client.getId()); UserConsentManager.revokeConsentToClient(session, client, user);
new UserSessionManager(session).revokeOfflineToken(user, client);
// Logout clientSessions for this user and client
AuthenticationManager.backchannelLogoutUserFromClient(session, realm, user, client, session.getContext().getUri(), headers);
event.event(EventType.REVOKE_GRANT).client(auth.getClient()).user(auth.getUser()).detail(Details.REVOKED_CLIENT, client.getClientId()).success(); event.event(EventType.REVOKE_GRANT).client(auth.getClient()).user(auth.getUser()).detail(Details.REVOKED_CLIENT, client.getClientId()).success();
setReferrerOnPage(); setReferrerOnPage();

View file

@ -41,6 +41,7 @@ import org.keycloak.representations.account.ConsentScopeRepresentation;
import org.keycloak.representations.account.UserRepresentation; import org.keycloak.representations.account.UserRepresentation;
import org.keycloak.services.ErrorResponse; import org.keycloak.services.ErrorResponse;
import org.keycloak.services.managers.Auth; import org.keycloak.services.managers.Auth;
import org.keycloak.services.managers.UserConsentManager;
import org.keycloak.services.managers.UserSessionManager; import org.keycloak.services.managers.UserSessionManager;
import org.keycloak.services.messages.Messages; import org.keycloak.services.messages.Messages;
import org.keycloak.services.resources.account.resources.ResourcesService; import org.keycloak.services.resources.account.resources.ResourcesService;
@ -286,8 +287,7 @@ public class AccountRestService {
return ErrorResponse.error(msg, Response.Status.NOT_FOUND); return ErrorResponse.error(msg, Response.Status.NOT_FOUND);
} }
session.users().revokeConsentForClient(realm, user.getId(), client.getId()); UserConsentManager.revokeConsentToClient(session, client, user);
new UserSessionManager(session).revokeOfflineToken(user, client);
event.success(); event.success();
return Response.noContent().build(); return Response.noContent().build();

View file

@ -65,6 +65,7 @@ import org.keycloak.services.ForbiddenException;
import org.keycloak.services.ServicesLogger; import org.keycloak.services.ServicesLogger;
import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.services.managers.BruteForceProtector; import org.keycloak.services.managers.BruteForceProtector;
import org.keycloak.services.managers.UserConsentManager;
import org.keycloak.services.managers.UserSessionManager; import org.keycloak.services.managers.UserSessionManager;
import org.keycloak.services.resources.LoginActionsService; import org.keycloak.services.resources.LoginActionsService;
import org.keycloak.services.resources.account.AccountFormService; import org.keycloak.services.resources.account.AccountFormService;
@ -477,15 +478,9 @@ public class UserResource {
if (client == null) { if (client == null) {
throw new NotFoundException("Client not found"); throw new NotFoundException("Client not found");
} }
boolean revokedConsent = session.users().revokeConsentForClient(realm, user.getId(), client.getId()); boolean revokedConsent = UserConsentManager.revokeConsentToClient(session, client, user);
boolean revokedOfflineToken = new UserSessionManager(session).revokeOfflineToken(user, client);
if (revokedConsent) { if (!revokedConsent) {
// Logout clientSessions for this user and client
AuthenticationManager.backchannelLogoutUserFromClient(session, realm, user, client, session.getContext().getUri(), headers);
}
if (!revokedConsent && !revokedOfflineToken) {
throw new NotFoundException("Consent nor offline token not found"); throw new NotFoundException("Consent nor offline token not found");
} }
adminEvent.operation(OperationType.ACTION).resourcePath(session.getContext().getUri()).success(); adminEvent.operation(OperationType.ACTION).resourcePath(session.getContext().getUri()).success();

View file

@ -149,7 +149,7 @@ public class OfflineServletsAdapterTest extends AbstractServletsAdapterTest {
.client("account").detail(Details.REVOKED_CLIENT, "offline-client").assertEvent(); .client("account").detail(Details.REVOKED_CLIENT, "offline-client").assertEvent();
// Assert refresh doesn't work now (increase time one more time) // Assert refresh doesn't work now (increase time one more time)
setAdapterAndServerTimeOffset(9999); setAdapterAndServerTimeOffset(19999);
offlineTokenPage.navigateTo(); offlineTokenPage.navigateTo();
assertCurrentUrlDoesntStartWith(offlineTokenPage); assertCurrentUrlDoesntStartWith(offlineTokenPage);
loginPage.assertCurrent(); loginPage.assertCurrent();

View file

@ -318,9 +318,10 @@ public class ConsentsTest extends AbstractKeycloakTest {
Map<String, Object> consent = consents.get(0); Map<String, Object> consent = consents.get(0);
Assert.assertEquals("Consent should be given to " + CLIENT_ID, CLIENT_ID, consent.get("clientId")); Assert.assertEquals("Consent should be given to " + CLIENT_ID, CLIENT_ID, consent.get("clientId"));
// list sessions // list sessions. Single client should be in user session
List<UserSessionRepresentation> sessions = userResource.getUserSessions(); List<UserSessionRepresentation> sessions = userResource.getUserSessions();
Assert.assertEquals("There should be one active session", 1, sessions.size()); Assert.assertEquals("There should be one active session", 1, sessions.size());
Assert.assertEquals("There should be one client in user session", 1, sessions.get(0).getClients().size());
// revoke consent // revoke consent
userResource.revokeConsent(CLIENT_ID); userResource.revokeConsent(CLIENT_ID);
@ -331,7 +332,8 @@ public class ConsentsTest extends AbstractKeycloakTest {
// list sessions // list sessions
sessions = userResource.getUserSessions(); sessions = userResource.getUserSessions();
Assert.assertEquals("There should be no active session", 0, sessions.size()); Assert.assertEquals("There should be one active session", 1, sessions.size());
Assert.assertEquals("There should be no client in user session", 0, sessions.get(0).getClients().size());
} }
@Test @Test

View file

@ -461,4 +461,48 @@ public class OAuthGrantTest extends AbstractKeycloakTest {
Assert.assertEquals("Email address", displayedScopes.get(1)); Assert.assertEquals("Email address", displayedScopes.get(1));
} }
// KEYCLOAK-16006 - tests that after revoke consent from single client, the SSO session is still valid and not automatically logged-out
@Test
public void oauthGrantUserNotLoggedOutAfterConsentRevoke() throws Exception {
// Login
oauth.clientId(THIRD_PARTY_APP);
oauth.doLoginGrant("test-user@localhost", "password");
// Confirm consent screen
grantPage.assertCurrent();
grantPage.assertGrants(OAuthGrantPage.PROFILE_CONSENT_TEXT, OAuthGrantPage.EMAIL_CONSENT_TEXT, OAuthGrantPage.ROLES_CONSENT_TEXT);
grantPage.accept();
Assert.assertTrue(oauth.getCurrentQuery().containsKey(OAuth2Constants.CODE));
EventRepresentation loginEvent = events.expectLogin()
.client(THIRD_PARTY_APP)
.detail(Details.CONSENT, Details.CONSENT_VALUE_CONSENT_GRANTED)
.assertEvent();
String sessionId = loginEvent.getSessionId();
// Revoke consent with admin REST API
adminClient.realm(REALM_NAME).users().get(loginEvent.getUserId()).revokeConsent(THIRD_PARTY_APP);
// Make sure that after refresh, consent page is displayed and user doesn't need to re-authenticate. Just accept consent screen again
oauth.openLoginForm();
grantPage.assertCurrent();
grantPage.assertGrants(OAuthGrantPage.PROFILE_CONSENT_TEXT, OAuthGrantPage.EMAIL_CONSENT_TEXT, OAuthGrantPage.ROLES_CONSENT_TEXT);
grantPage.accept();
loginEvent = events.expectLogin()
.client(THIRD_PARTY_APP)
.detail(Details.CONSENT, Details.CONSENT_VALUE_CONSENT_GRANTED)
.assertEvent();
//String codeId = loginEvent.getDetails().get(Details.CODE_ID);
String sessionId2 = loginEvent.getSessionId();
Assert.assertEquals(sessionId, sessionId2);
// Revert consent
adminClient.realm(REALM_NAME).users().get(loginEvent.getUserId()).revokeConsent(THIRD_PARTY_APP);
}
} }

View file

@ -113,6 +113,14 @@ public class TokenRevocationTest extends AbstractKeycloakTest {
isTokenDisabled(tokenResponse1, "test-app"); isTokenDisabled(tokenResponse1, "test-app");
isTokenEnabled(tokenResponse2, "test-app-scope"); isTokenEnabled(tokenResponse2, "test-app-scope");
// Revoke second token and assert no sessions for testUser
response = oauth.doTokenRevoke(tokenResponse2.getRefreshToken(), "refresh_token", "password");
assertThat(response, Matchers.statusCodeIsHC(Status.OK));
userSessions = testUser.getUserSessions();
assertEquals(0, userSessions.size());
} }
@Test @Test