From 5ce41a462b772ec583339991e2600b34c7836996 Mon Sep 17 00:00:00 2001 From: Douglas Palmer Date: Thu, 23 Nov 2023 07:35:54 -0800 Subject: [PATCH] NPE in HardcodedUserSessionAttributeMapper on Token Exchange Closes #11996 Signed-off-by: Douglas Palmer --- .../provider/BrokeredIdentityContext.java | 24 +++++++++++++++---- .../java/org/keycloak/models/Constants.java | 2 ++ .../mappers/ClaimToUserSessionNoteMapper.java | 7 +----- .../HardcodedUserSessionAttributeMapper.java | 2 +- .../oidc/DefaultTokenExchangeProvider.java | 3 +-- 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java index b83bb53e30..dc18b6dee3 100755 --- a/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java @@ -18,6 +18,7 @@ package org.keycloak.broker.provider; import org.keycloak.models.Constants; import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.UserSessionModel; import org.keycloak.sessions.AuthenticationSessionModel; import java.util.ArrayList; @@ -48,7 +49,6 @@ public class BrokeredIdentityContext { private IdentityProviderModel idpConfig; private IdentityProvider idp; private Map contextData = new HashMap<>(); - private Map claims = new HashMap<>(); private AuthenticationSessionModel authenticationSession; public BrokeredIdentityContext(String id) { @@ -162,12 +162,26 @@ public class BrokeredIdentityContext { this.contextData = contextData; } - public Map getClaims() { - return claims; + private Map getSessionNotes() { + HashMap sessionNotes = (HashMap) this.contextData.get(Constants.MAPPER_SESSION_NOTES); + if (sessionNotes == null) { + sessionNotes = new HashMap<>(); + this.contextData.put(Constants.MAPPER_SESSION_NOTES, sessionNotes); + } + return sessionNotes; } - public void setClaims(Map claims) { - this.claims = claims; + public void setSessionNote(String key, String value) { + if(authenticationSession != null) { + authenticationSession.setUserSessionNote(key, value); + } + else { + getSessionNotes().put(key, value); + } + } + + public void addSessionNotesToUserSession(UserSessionModel userSession) { + getSessionNotes().forEach((k, v) -> userSession.setNote(k, v)); } // Set the attribute, which will be available on "Update profile" page and in authenticators diff --git a/server-spi-private/src/main/java/org/keycloak/models/Constants.java b/server-spi-private/src/main/java/org/keycloak/models/Constants.java index c54646115e..8559abad3b 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/Constants.java +++ b/server-spi-private/src/main/java/org/keycloak/models/Constants.java @@ -106,6 +106,8 @@ public final class Constants { // Groups already assigned by a mapper when updating brokered users. public static final String MAPPER_GRANTED_GROUPS = "MAPPER_GRANTED_GROUPS"; + public static final String MAPPER_SESSION_NOTES = "MAPPER_SESSION_NOTES"; + // Indication to admin-rest-endpoint that realm keys should be re-generated public static final String GENERATE = "GENERATE"; diff --git a/services/src/main/java/org/keycloak/broker/oidc/mappers/ClaimToUserSessionNoteMapper.java b/services/src/main/java/org/keycloak/broker/oidc/mappers/ClaimToUserSessionNoteMapper.java index 440994ccd5..9606882bb1 100644 --- a/services/src/main/java/org/keycloak/broker/oidc/mappers/ClaimToUserSessionNoteMapper.java +++ b/services/src/main/java/org/keycloak/broker/oidc/mappers/ClaimToUserSessionNoteMapper.java @@ -127,12 +127,7 @@ public class ClaimToUserSessionNoteMapper extends AbstractClaimMapper { : valueEquals(value, claimValue); if (claimValuesMatch) { - if(context.getAuthenticationSession() != null) { - context.getAuthenticationSession().setUserSessionNote(claim.getKey(), claimValue); - } - else { - context.getClaims().put(claim.getKey(), claimValue); - } + context.setSessionNote(claim.getKey(), claimValue); } } } diff --git a/services/src/main/java/org/keycloak/broker/provider/HardcodedUserSessionAttributeMapper.java b/services/src/main/java/org/keycloak/broker/provider/HardcodedUserSessionAttributeMapper.java index 530e3318cd..a541524d89 100755 --- a/services/src/main/java/org/keycloak/broker/provider/HardcodedUserSessionAttributeMapper.java +++ b/services/src/main/java/org/keycloak/broker/provider/HardcodedUserSessionAttributeMapper.java @@ -114,6 +114,6 @@ public class HardcodedUserSessionAttributeMapper extends AbstractIdentityProvide private void setHardcodedUserSessionAttribute(IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { String attribute = mapperModel.getConfig().get(ATTRIBUTE); String attributeValue = mapperModel.getConfig().get(ATTRIBUTE_VALUE); - context.getAuthenticationSession().setUserSessionNote(attribute, attributeValue); + context.setSessionNote(attribute, attributeValue); } } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/DefaultTokenExchangeProvider.java b/services/src/main/java/org/keycloak/protocol/oidc/DefaultTokenExchangeProvider.java index 49977e36f6..89b51bc1b0 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/DefaultTokenExchangeProvider.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/DefaultTokenExchangeProvider.java @@ -86,7 +86,6 @@ import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.MultivaluedMap; import jakarta.ws.rs.core.Response; -import java.util.UUID; /** * Default token exchange implementation @@ -515,7 +514,7 @@ public class DefaultTokenExchangeProvider implements TokenExchangeProvider { userSession.setNote(IdentityProvider.EXTERNAL_IDENTITY_PROVIDER, externalIdpModel.get().getAlias()); userSession.setNote(IdentityProvider.FEDERATED_ACCESS_TOKEN, subjectToken); - context.getClaims().forEach((k, v) -> userSession.setNote(k, v)); + context.addSessionNotesToUserSession(userSession); return exchangeClientToClient(user, userSession, null, false); }