8000 Add validation for blocked session note names in UserSessionNoteMapper by jimmychakkalakal · Pull Request #53559 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ With {project_name}, you can store tokens and responses from the authentication
Application code can retrieve these tokens and responses to import extra user information or to request the external IDP securely. For example, an application can use the Google token to use other Google services and REST APIs.

For the details on how to configure and use this functionality, please see the link:{developerguide_link}#_identity-brokering-apis-tokens[section for retrieving external IDP tokens from {developerguide_name}].

This endpoint is the only supported way for an application to obtain the tokens of an external IDP. Identity Brokering API V1 requires the `read-token` role on the `broker` client, while V2 requires client authentication and permission for the client to retrieve tokens from the specified IDP alias.
{project_name} stores those tokens as notes on the user session, but a *User Session Note* mapper cannot copy them into a token claim or a SAML attribute.
Such a mapper is rejected when the client is created or updated, and any mapper that already exists is ignored when a token is issued.
37 changes: 37 additions & 0 deletions docs/documentation/upgrading/topics/changes/changes-27_0_0.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,43 @@ If users from a realm with a cross-realm trust log in with a username and passwo
For the LDAP provider the realm comes from the Kerberos principal attribute of the user, so list every realm found there, for example the child domains of an Active Directory forest.
SPNEGO authentication is not affected.

=== Session note mappers no longer expose internal credentials or user models

When a user authenticates through an external identity provider, {project_name} stores the tokens issued by that provider as notes on the user session.
The supported way to retrieve them is the broker token endpoint; its authorization requirements depend on the enabled Identity Brokering API version.
Previously, the *User Session Note* mapper copied any session note into a token claim without checking what the note held.
An administrator who only held the `manage` permission on a single client could therefore configure a mapper for `FEDERATED_ACCESS_TOKEN` and receive the upstream token of every user who logged in to that client, bypassing the broker token endpoint's authorization checks entirely.
The same applied to the SAML *User Session Note* mapper, which copied the note into a SAML attribute.

Both mappers now refuse to read the session notes that hold identity provider tokens: `FEDERATED_ACCESS_TOKEN`, `FEDERATED_REFRESH_TOKEN`, `FEDERATED_TOKEN_EXPIRATION`, and `FEDERATED_ID_TOKEN`, together with the per-provider forms such as `FEDERATED_ACCESS_TOKEN:my-idp`.
They also refuse to read `keycloak.userModel`, which holds the serialized user model when the experimental transient-users feature is used.
This internal representation includes user attributes, role and group identifiers, and consent data, and must not be copied wholesale to application tokens.
The documented Kerberos GSS credential delegation mapper and ordinary application session notes remain supported.

Creating or updating a client with such a mapper is rejected with a `400 Bad Request`, and so is importing a realm that contains one.
A mapper that already exists, because it was configured before upgrading, is retained but no longer produces a claim or an attribute, and {project_name} logs a warning naming the mapper and the client.

Existing deployments continue to start because the mappers are read from the database rather than imported, but a client with such a mapper cannot be updated until the mapper is removed.
If you provision realms from an exported JSON file, remove the mapper from the file before importing it again.

After upgrading, if any of your applications read an identity provider token from a token claim or a SAML attribute, change them to call the broker token endpoint; its authorization requirements depend on the enabled Identity Brokering API version.
For more details, see the link:{adminguide_link}#retrieving-external-idp-tokens[Retrieving external IDP tokens] section of the {adminguide_name}.

=== Protocol mappers are validated when a client or a client scope is written as a whole

Previously, a protocol mapper was validated only when it was written through the mapper endpoints of the Admin REST API.
A mapper sent as part of a client or a client scope, such as during client registration, partial import, or realm import, was stored without being validated, so a mapper rejected by one path was accepted by another.

Starting with this release, every protocol mapper of a client is validated whenever the client is created or updated as a whole, using the same rules as the mapper endpoints. Client scope creation and realm import also validate their protocol mappers.
A mapper with an invalid configuration is now rejected with a `400 Bad Request`, where it was previously stored and silently ignored, or failed later at runtime.

The validation covers all the mappers of the client, not only the one being changed.
A client that carries a mapper stored before the upgrade whose configuration is no longer valid cannot be updated at all, even for an unrelated field, until that mapper is corrected or removed.
Updating an existing client scope does not revalidate its stored mappers.

Existing deployments continue to start, because mappers that are read from the database at startup are not validated.
If you provision realms from an exported JSON file, import the file once after upgrading to find out whether it contains a mapper that is now rejected.

// ------------------------ Notable changes ------------------------ //
== Notable changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.LDAPConstants;
import org.keycloak.models.ModelException;
import org.keycloak.models.ModelValidationException;
import org.keycloak.models.OAuth2DeviceConfig;
import org.keycloak.models.OTPPolicy;
import org.keycloak.models.OrganizationModel;
Expand All @@ -85,12 +86,14 @@
import org.keycloak.models.utils.DefaultKeyProviders;
import org.keycloak.models.utils.DefaultRequiredActions;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.models.utils.ProtocolMapperValidationUtil;
import org.keycloak.models.utils.RepresentationToModel;
import org.keycloak.organization.OrganizationProvider;
import org.keycloak.organization.validation.OrganizationsValidation;
import org.keycloak.partialimport.PartialImportResults;
import org.keycloak.protocol.LoginProtocol;
import org.keycloak.protocol.LoginProtocolFactory;
import org.keycloak.protocol.ProtocolMapperConfigException;
import org.keycloak.protocol.oidc.OIDCConfigAttributes;
import org.keycloak.representations.idm.ApplicationRepresentation;
import org.keycloak.representations.idm.AuthenticationExecutionExportRepresentation;
Expand Down Expand Up @@ -479,7 +482,21 @@ public void importRealm(RealmRepresentation rep, RealmModel newRealm, Runnable u

importUserFederationProvidersAndMappers(session, rep, newRealm);


// Mapper validation may resolve other clients or realm components. Validate only after those
// dependencies exist, but before importing users, which can commit batches independently.
for (ClientModel client : createdClients.values()) {
ValidationUtil.validateClient(session, client, false, r -> {
throw new ModelValidationException("Invalid client " + client.getClientId() + ": " + r.getAllErrorsAsString());
});
}
for (ClientScopeModel clientScope : clientScopes.values()) {
try {
ProtocolMapperValidationUtil.validateProtocolMappers(session, newRealm, clientScope);
} catch (ProtocolMapperConfigException ex) {
logger.warnf("Invalid protocol mapper configuration in client scope '%s': %s", clientScope.getName(), ex.getMessage());
throw new ModelValidationException(ProtocolMapperValidationUtil.INVALID_CONFIG_MESSAGE);
}
}

// create users and their role mappings and social mappings

Expand Down Expand Up @@ -609,10 +626,6 @@ private static Map<String, ClientModel> createClients(KeycloakSession session, M
addSamlEncryptionAttributes(app);
}
appMap.put(app.getClientId(), app);

ValidationUtil.validateClient(session, app, false, r -> {
throw new RuntimeException("Invalid client " + app.getClientId() + ": " + r.getAllErrorsAsString());
});
}
return appMap;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Copyright 2016 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.models.utils;

import java.util.HashMap;
import java.util.Map;

import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ProtocolMapperContainerModel;
import org.keycloak.models.ProtocolMapperModel;
import org.keycloak.models.RealmModel;
import org.keycloak.protocol.ProtocolMapper;
import org.keycloak.protocol.ProtocolMapperConfigException;

/**
* Validates the protocol mappers of a container on the paths that write a whole container at
* once, such as a client scope created inline or a realm import, which do not go through the per-mapper admin
* endpoint and so never reach {@link ProtocolMapper#validateConfig}.
*/
public class ProtocolMapperValidationUtil {

public static final String INVALID_CONFIG_MESSAGE = "Invalid protocol mapper configuration";

private ProtocolMapperValidationUtil() {
// utility class
}

public static void validateProtocolMappers(KeycloakSession session, RealmModel realm, ProtocolMapperContainerModel container) throws ProtocolMapperConfigException {
for (ProtocolMapperModel model : container.getProtocolMappersStream().toList()) {
validateProtocolMapper(session, realm, container, model);
}
}

public static void validateProtocolMapper(KeycloakSession session, RealmModel realm, ProtocolMapperContainerModel container,
ProtocolMapperModel model) throws ProtocolMapperConfigException {
ProtocolMapper mapper = (ProtocolMapper) session.getKeycloakSessionFactory()
.getProviderFactory(ProtocolMapper.class, model.getProtocolMapper());
// An import must not fail just because a third-party mapper provider is absent.
if (mapper == null) {
return;
}

// A mapper may carry no configuration at all, and RepresentationToModel preserves that as null. Validators
// read the configuration without expecting that, so normalize it and let the mapper report its own error
// instead of throwing a NullPointerException, which the admin endpoints would report as a 500.
if (model.getConfig() == null) {
model.setConfig(new HashMap<>());
}

Map<String, String> originalConfig = new HashMap<>(model.getConfig());
mapper.validateConfig(session, realm, container, model);
// Validators can initialize configuration, for example the pairwise subject mapper's salt.
// Models read from the container are detached, so these changes must be saved explicitly.
if (!originalConfig.equals(model.getConfig())) {
container.updateProtocolMapper(model);
Comment thread
jimmychakkalakal marked this conversation as resolved.
Resolved
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,23 @@ public void create(RealmModel realm, KeycloakSession session, ClientRepresentati
OIDCAdvancedConfigWrapper.fromClientModel(client).setPostLogoutRedirectUris(Collections.singletonList("+"));
}
RepresentationToModel.importAuthorizationSettings(clientRep, client, session);
}

ValidationUtil.validateClient(session, client, true, r -> {
session.getTransactionManager().setRollbackOnly();
throw ErrorResponse.error("Failed to import client " + client.getClientId() + ": " + r.getAllErrorsAsString(), Response.Status.BAD_REQUEST);
});
@Override
public PartialImportResults doImport(PartialImportRepresentation rep, RealmModel realm, KeycloakSession session) {
PartialImportResults results = super.doImport(rep, realm, session);
// A mapper may refer to another client in this import. Do not validate until all clients exist.
for (PartialImportResult result : results.getResults()) {
if (result.getAction() == Action.SKIPPED) {
continue;
}
ClientModel client = realm.getClientById(result.getId());
ValidationUtil.validateClient(session, client, true, r -> {
session.getTransactionManager().setRollbackOnly();
throw ErrorResponse.error("Failed to import client " + client.getClientId() + ": " + r.getAllErrorsAsString(), Response.Status.BAD_REQUEST);
});
}
return results;
}

public static boolean isInternalClient(String clientId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,19 @@
import java.util.AbstractMap;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Stream;

import org.keycloak.broker.oidc.AbstractOAuth2IdentityProvider;
import org.keycloak.broker.oidc.OIDCIdentityProvider;
import org.keycloak.broker.provider.UserAuthenticationIdentityProvider;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientSessionContext;
import org.keycloak.models.Constants;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.models.ProtocolMapperModel;
Expand Down Expand Up @@ -93,6 +99,12 @@ public class ProtocolMapperUtils {

private static final HashMap<String, Method> ACCESSORS = new HashMap<>();

public static final Set<String> BROKER_CREDENTIAL_NOTES = Set.of(
UserAuthenticationIdentityProvider.FEDERATED_ACCESS_TOKEN,
AbstractOAuth2IdentityProvider.FEDERATED_REFRESH_TOKEN,
AbstractOAuth2IdentityProvider.FEDERATED_TOKEN_EXPIRATION,
OIDCIdentityProvider.FEDERATED_ID_TOKEN);

// This caches known methods to avoid generating unnecessary failed lookups and exception at runtime which are expensive
static {
for (Method method : UserModel.class.getMethods()) {
Expand Down Expand Up @@ -185,4 +197,13 @@ public static int compare(Entry<ProtocolMapperModel, ProtocolMapper> entry) {
public static boolean isEnabled(KeycloakSession session, ProtocolMapperModel mapper) {
return session.getKeycloakSessionFactory().getProviderFactory(ProtocolMapper.class, mapper.getProtocolMapper()) != null;
}

/** Internal credentials and serialized user state must not be exposed by generic session note mappers. */
public static boolean isRestrictedUserSessionNote(String noteName) {
if (noteName == null) return false;
if (Constants.SESSION_NOTE_LIGHTWEIGHT_USER.equals(noteName)) return true;
String upper = noteName.toUpperCase(Locale.ROOT);
return BROKER_CREDENTIAL_NOTES.stream()
.anyMatch(n -> upper.equals(n) || upper.startsWith(n + ":"));
}
}
Loading
Loading
0