KEYCLOAK-17748 Optimize validation of redirect URIs in logout endpoint

Reimplementation of KEYCLOAK-17718
This commit is contained in:
Hynek Mlnarik 2021-04-13 07:33:49 +02:00 committed by Hynek Mlnařík
parent 4d776cd780
commit c02a706a86
11 changed files with 86 additions and 12 deletions

View file

@ -568,6 +568,11 @@ public class RealmCacheSession implements CacheRealmProvider {
return getClientDelegate().getAlwaysDisplayInConsoleClientsStream(realm);
}
@Override
public Map<ClientModel, Set<String>> getAllRedirectUrisOfEnabledClients(RealmModel realm) {
return getClientDelegate().getAllRedirectUrisOfEnabledClients(realm);
}
@Override
public void removeClients(RealmModel realm) {
getClientDelegate().removeClients(realm);

View file

@ -279,6 +279,20 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc
return session.roles().getRoleById(client.getRealm(), roles.get(0));
}
@Override
public Map<ClientModel, Set<String>> getAllRedirectUrisOfEnabledClients(RealmModel realm) {
TypedQuery<Map> query = em.createNamedQuery("getAllRedirectUrisOfEnabledClients", Map.class);
query.setParameter("realm", realm.getId());
return query.getResultStream()
.filter(s -> s.get("client") != null)
.collect(
Collectors.groupingBy(
s -> new ClientAdapter(realm, em, session, (ClientEntity) s.get("client")),
Collectors.mapping(s -> (String) s.get("redirectUri"), Collectors.toSet())
)
);
}
@Override
public Stream<RoleModel> getRealmRolesStream(RealmModel realm, Integer first, Integer max) {
TypedQuery<RoleEntity> query = em.createNamedQuery("getRealmRoles", RoleEntity.class);

View file

@ -56,6 +56,7 @@ import java.util.Set;
@NamedQuery(name="searchClientsByClientId", query="select client.id from ClientEntity client where lower(client.clientId) like lower(concat('%',:clientId,'%')) and client.realmId = :realm order by client.clientId"),
@NamedQuery(name="getRealmClientsCount", query="select count(client) from ClientEntity client where client.realmId = :realm"),
@NamedQuery(name="findClientByClientId", query="select client from ClientEntity client where client.clientId = :clientId and client.realmId = :realm"),
@NamedQuery(name="getAllRedirectUrisOfEnabledClients", query="select new map(client as client, r as redirectUri) from ClientEntity client join client.redirectUris r where client.realmId = :realm and client.enabled = true"),
})
public class ClientEntity {

View file

@ -46,6 +46,7 @@ import static org.keycloak.common.util.StackUtil.getShortStackTrace;
import org.keycloak.models.ClientScopeModel;
import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import java.util.HashSet;
import static org.keycloak.utils.StreamsUtil.paginatedStream;
public class MapClientProvider<K> implements ClientProvider {
@ -336,6 +337,22 @@ public class MapClientProvider<K> implements ClientProvider {
.collect(Collectors.toMap(ClientScopeModel::getName, Function.identity()));
}
@Override
public Map<ClientModel, Set<String>> getAllRedirectUrisOfEnabledClients(RealmModel realm) {
ModelCriteriaBuilder<ClientModel> mcb = clientStore.createCriteriaBuilder()
.compare(SearchableFields.REALM_ID, Operator.EQ, realm.getId())
.compare(SearchableFields.ENABLED, Operator.EQ, Boolean.TRUE);
try (Stream<MapClientEntity<K>> st = tx.getUpdatedNotRemoved(mcb)) {
return st
.filter(mce -> mce.getRedirectUris() != null && ! mce.getRedirectUris().isEmpty())
.collect(Collectors.toMap(
mce -> entityToAdapterFunc(realm).apply(mce),
mce -> new HashSet<>(mce.getRedirectUris()))
);
}
}
public void preRemove(RealmModel realm, RoleModel role) {
ModelCriteriaBuilder<ClientModel> mcb = clientStore.createCriteriaBuilder()
.compare(SearchableFields.REALM_ID, Operator.EQ, realm.getId())

View file

@ -340,6 +340,12 @@ public class MapRealmProvider<K> implements RealmProvider {
session.clientScopes().removeClientScopes(realm);
}
@Override
@Deprecated
public Map<ClientModel, Set<String>> getAllRedirectUrisOfEnabledClients(RealmModel realm) {
return session.clients().getAllRedirectUrisOfEnabledClients(realm);
}
@Override
@Deprecated
public void moveGroup(RealmModel realm, GroupModel group, GroupModel toParent) {

View file

@ -96,6 +96,7 @@ public class MapFieldPredicates {
put(CLIENT_PREDICATES, ClientModel.SearchableFields.REALM_ID, MapClientEntity::getRealmId);
put(CLIENT_PREDICATES, ClientModel.SearchableFields.CLIENT_ID, MapClientEntity::getClientId);
put(CLIENT_PREDICATES, ClientModel.SearchableFields.SCOPE_MAPPING_ROLE, MapFieldPredicates::checkScopeMappingRole);
put(CLIENT_PREDICATES, ClientModel.SearchableFields.ENABLED, MapClientEntity::isEnabled);
put(CLIENT_PREDICATES, ClientModel.SearchableFields.ATTRIBUTE, MapFieldPredicates::checkClientAttributes);
put(CLIENT_SCOPE_PREDICATES, ClientScopeModel.SearchableFields.REALM_ID, MapClientScopeEntity::getRealmId);

View file

@ -41,6 +41,7 @@ public interface ClientModel extends ClientScopeModel, RoleContainerModel, Prot
public static final SearchableModelField<ClientModel> ID = new SearchableModelField<>("id", String.class);
public static final SearchableModelField<ClientModel> REALM_ID = new SearchableModelField<>("realmId", String.class);
public static final SearchableModelField<ClientModel> CLIENT_ID = new SearchableModelField<>("clientId", String.class);
public static final SearchableModelField<ClientModel> ENABLED = new SearchableModelField<>("enabled", Boolean.class);
public static final SearchableModelField<ClientModel> SCOPE_MAPPING_ROLE = new SearchableModelField<>("scopeMappingRole", String.class);
/**

View file

@ -20,6 +20,7 @@ import org.keycloak.provider.Provider;
import org.keycloak.storage.client.ClientLookupProvider;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@ -167,4 +168,13 @@ public interface ClientProvider extends ClientLookupProvider, Provider {
* @param clientScope to be unassigned
*/
void removeClientScope(RealmModel realm, ClientModel client, ClientScopeModel clientScope);
/**
* Returns a map of (rootUrl, {validRedirectUris}) for all enabled clients.
* @param realm
* @return
* @deprecated Do not use, this is only to support a deprecated logout endpoint and will vanish with it's removal
*/
@Deprecated
Map<ClientModel, Set<String>> getAllRedirectUrisOfEnabledClients(RealmModel realm);
}

View file

@ -128,9 +128,26 @@ public class LogoutEndpoint {
@QueryParam("state") String state,
@QueryParam("initiating_idp") String initiatingIdp) {
String redirect = postLogoutRedirectUri != null ? postLogoutRedirectUri : redirectUri;
IDToken idToken = null;
if (encodedIdToken != null) {
try {
idToken = tokenManager.verifyIDTokenSignature(session, encodedIdToken);
TokenVerifier.createWithoutSignature(idToken).tokenType(TokenUtil.TOKEN_TYPE_ID).verify();
} catch (OAuthErrorException | VerificationException e) {
event.event(EventType.LOGOUT);
event.error(Errors.INVALID_TOKEN);
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.SESSION_NOT_ACTIVE);
}
}
if (redirect != null) {
String validatedUri = RedirectUtils.verifyRealmRedirectUri(session, redirect);
String validatedUri;
ClientModel client = (idToken == null || idToken.getIssuedFor() == null) ? null : realm.getClientById(idToken.getIssuedFor());
if (client != null) {
validatedUri = RedirectUtils.verifyRedirectUri(session, redirect, client);
} else {
validatedUri = RedirectUtils.verifyRealmRedirectUri(session, redirect);
}
if (validatedUri == null) {
event.event(EventType.LOGOUT);
event.detail(Details.REDIRECT_URI, redirect);
@ -141,17 +158,14 @@ public class LogoutEndpoint {
}
UserSessionModel userSession = null;
IDToken idToken = null;
if (encodedIdToken != null) {
if (idToken != null) {
try {
idToken = tokenManager.verifyIDTokenSignature(session, encodedIdToken);
TokenVerifier.createWithoutSignature(idToken).tokenType(TokenUtil.TOKEN_TYPE_ID).verify();
userSession = session.sessions().getUserSession(realm, idToken.getSessionState());
if (userSession != null) {
checkTokenIssuedAt(idToken, userSession);
}
} catch (OAuthErrorException | VerificationException e) {
} catch (OAuthErrorException e) {
event.event(EventType.LOGOUT);
event.error(Errors.INVALID_TOKEN);
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.SESSION_NOT_ACTIVE);

View file

@ -72,9 +72,10 @@ public class RedirectUtils {
}
private static Set<String> getValidateRedirectUris(KeycloakSession session) {
return session.getContext().getRealm().getClientsStream()
.filter(client -> client.isEnabled() && OIDCLoginProtocol.LOGIN_PROTOCOL.equals(client.getProtocol()) && !client.isBearerOnly() && (client.isStandardFlowEnabled() || client.isImplicitFlowEnabled()))
.map(c -> resolveValidRedirects(session, c.getRootUrl(), c.getRedirectUris()))
RealmModel realm = session.getContext().getRealm();
return session.clientStorageManager().getAllRedirectUrisOfEnabledClients(realm).entrySet().stream()
.filter(me -> me.getKey().isEnabled() && OIDCLoginProtocol.LOGIN_PROTOCOL.equals(me.getKey().getProtocol()) && !me.getKey().isBearerOnly() && (me.getKey().isStandardFlowEnabled() || me.getKey().isImplicitFlowEnabled()))
.map(me -> resolveValidRedirects(session, me.getKey().getRootUrl(), me.getValue()))
.flatMap(Collection::stream)
.collect(Collectors.toSet());
}

View file

@ -34,7 +34,6 @@ import org.keycloak.utils.ServicesUtils;
import java.util.Objects;
import java.util.function.Function;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.keycloak.models.ClientScopeModel;
@ -264,6 +263,11 @@ public class ClientStorageManager implements ClientProvider {
session.clientLocalStorage().removeClientScope(realm, client, clientScope);
}
@Override
public Map<ClientModel, Set<String>> getAllRedirectUrisOfEnabledClients(RealmModel realm) {
return session.clientLocalStorage().getAllRedirectUrisOfEnabledClients(realm);
}
@Override
public void close() {