KEYCLOAK-14577 OIDCIdentityProvider incorrectly sets firstName and la…

…stName in BrokeredIdentityContext
dayantrivani · Feb 18, 2021 · 00ee6bb · 00ee6bb
1 parent 056b52f
commit 00ee6bb
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 5 deletions.
diff --git a/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java b/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java
@@ -17,6 +17,7 @@
 package org.keycloak.broker.oidc;
 
 import com.fasterxml.jackson.databind.JsonNode;
+
 import org.jboss.logging.Logger;
 import org.keycloak.OAuth2Constants;
 import org.keycloak.OAuthErrorException;
@@ -25,7 +26,6 @@
 import org.keycloak.broker.provider.BrokeredIdentityContext;
 import org.keycloak.broker.provider.ExchangeExternalToken;
 import org.keycloak.broker.provider.IdentityBrokerException;
-import org.keycloak.broker.provider.util.IdentityBrokerState;
 import org.keycloak.broker.provider.util.SimpleHttp;
 import org.keycloak.common.util.Base64Url;
 import org.keycloak.common.util.Time;
@@ -45,14 +45,12 @@
 import org.keycloak.models.UserSessionModel;
 import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
-import org.keycloak.protocol.oidc.utils.PkceUtils;
 import org.keycloak.representations.AccessTokenResponse;
 import org.keycloak.representations.IDToken;
 import org.keycloak.representations.JsonWebToken;
 import org.keycloak.services.ErrorPage;
 import org.keycloak.services.ErrorResponseException;
 import org.keycloak.services.managers.AuthenticationManager;
-import org.keycloak.services.managers.ClientSessionCode;
 import org.keycloak.services.messages.Messages;
 import org.keycloak.services.resources.IdentityBrokerService;
 import org.keycloak.services.resources.RealmsResource;
@@ -69,6 +67,7 @@
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriBuilder;
 import javax.ws.rs.core.UriInfo;
+
 import java.io.IOException;
 import java.security.PublicKey;
 
@@ -656,11 +655,26 @@ protected BrokeredIdentityContext extractIdentityFromProfile(EventBuilder event,
 
         String name = getJsonProperty(userInfo, "name");
         String preferredUsername = getUsernameFromUserInfo(userInfo);
+        String givenName = getJsonProperty(userInfo, "given_name");
+        String familyName = getJsonProperty(userInfo, "family_name");
         String email = getJsonProperty(userInfo, "email");
+
         AbstractJsonUserAttributeMapper.storeUserProfileForMapper(identity, userInfo, getConfig().getAlias());
 
         identity.setId(id);
-        identity.setName(name);
+
+        if (givenName != null) {
+            identity.setFirstName(givenName);
+        }
+
+        if (familyName != null) {
+            identity.setLastName(familyName);
+        }
+
+        if (givenName == null && familyName == null) {
+            identity.setName(name);
+        }
+
         identity.setEmail(email);
 
         identity.setBrokerUserId(getConfig().getAlias() + "." + id);

diff --git a/.../src/test/java/org/keycloak/testsuite/adapter/servlet/BrokerLinkAndTokenExchangeTest.java b/.../src/test/java/org/keycloak/testsuite/adapter/servlet/BrokerLinkAndTokenExchangeTest.java
@@ -100,6 +100,7 @@ public class BrokerLinkAndTokenExchangeTest extends AbstractServletsAdapterTest
     public static final String PARENT_IDP = "parent-idp";
     public static final String PARENT_USERNAME = "parent";
     public static final String PARENT2_USERNAME = "parent2";
+    public static final String PARENT3_USERNAME = "parent3";
     public static final String UNAUTHORIZED_CHILD_CLIENT = "unauthorized-child-client";
     public static final String PARENT_CLIENT = "parent-client";
 
@@ -221,7 +222,13 @@ public void addIdpUser() {
         user.setUsername(PARENT2_USERNAME);
         user.setEnabled(true);
         createUserAndResetPasswordWithAdminClient(realm, user, "password");
-
+        user = new UserRepresentation();
+        user.setUsername(PARENT3_USERNAME);
+        user.setFirstName("firstname");
+        user.setLastName("lastname");
+        user.setEmail("email");
+        user.setEnabled(true);
+        createUserAndResetPasswordWithAdminClient(realm, user, "password");        
     }
 
     private String childUserId = null;
@@ -700,7 +707,55 @@ public void testExternalExchange() throws Exception {
             httpClient.close();
         }
     }
+
+    /**
+     * KEYCLOAK-14577, see also KEYCLOAK-10932
+     */
+    @Test
+    public void testExternalExchange_extractIdentityFromProfile() throws Exception {
+        RealmResource childRealm = adminClient.realms().realm(CHILD_IDP);
+
+        String accessToken = oauth.doGrantAccessTokenRequest(PARENT_IDP, PARENT3_USERNAME, "password", null, PARENT_CLIENT, "password").getAccessToken();
+        Assert.assertEquals(0, adminClient.realm(CHILD_IDP).getClientSessionStats().size());
+
+        Client httpClient = AdminClientUtil.createResteasyClient();
+        try {
+            WebTarget exchangeUrl = childTokenExchangeWebTarget(httpClient);
+            IdentityProviderRepresentation rep = adminClient.realm(CHILD_IDP).identityProviders().get(PARENT_IDP).toRepresentation();
+            rep.getConfig().put(OIDCIdentityProviderConfig.VALIDATE_SIGNATURE, String.valueOf(false));
+            adminClient.realm(CHILD_IDP).identityProviders().get(PARENT_IDP).update(rep);
 
+            AccessToken token;
+            try (Response response = exchangeUrl.request()
+                    .header(HttpHeaders.AUTHORIZATION, BasicAuthHelper.createHeader(ClientApp.DEPLOYMENT_NAME, "password"))
+                    .post(Entity.form(
+                            new Form()
+                                    .param(OAuth2Constants.GRANT_TYPE, OAuth2Constants.TOKEN_EXCHANGE_GRANT_TYPE)
+                                    .param(OAuth2Constants.SUBJECT_TOKEN, accessToken)
+                                    .param(OAuth2Constants.SUBJECT_TOKEN_TYPE, OAuth2Constants.JWT_TOKEN_TYPE)
+                                    .param(OAuth2Constants.SUBJECT_ISSUER, PARENT_IDP)
+                                    .param(OAuth2Constants.SCOPE, OAuth2Constants.SCOPE_OPENID)
+                    ))) {
+                Assert.assertEquals(200, response.getStatus());
+
+                AccessTokenResponse tokenResponse = response.readEntity(AccessTokenResponse.class);
+                JWSInput jws = new JWSInput(tokenResponse.getToken());
+                token = jws.readJsonContent(AccessToken.class);
+            }
+
+            Assert.assertNotNull(token);
+            Assert.assertNotNull(token.getSubject());
+            Assert.assertEquals(PARENT3_USERNAME, token.getPreferredUsername());
+            Assert.assertEquals("firstname", token.getGivenName());
+            Assert.assertEquals("lastname", token.getFamilyName());
+            Assert.assertEquals("email", token.getEmail());
+
+            // cleanup remove the user
+            childRealm.users().get(token.getSubject()).remove();
+        } finally {
+            httpClient.close();
+        }
+    }
 
     public void logoutAll() {
         String logoutUri = OIDCLoginProtocolService.logoutUrl(authServerPage.createUriBuilder()).build(CHILD_IDP).toString();