[KEYCLOAK-7450] - Match subject when validating id_token returned from external OP

services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java

@@ -365,6 +365,10 @@ public class OIDCIdentityProvider extends AbstractOAuth2IdentityProvider<OIDCIde
 
         try {
             BrokeredIdentityContext identity = extractIdentity(tokenResponse, accessToken, idToken);
+            
+            if (!identity.getId().equals(idToken.getSubject())) {
+                throw new IdentityBrokerException("Mismatch between the subject in the id_token and the subject from the user_info endpoint");
+            }
 
             identity.getContextData().put(BROKER_NONCE_PARAM, idToken.getOtherClaims().get(OIDCLoginProtocol.NONCE_PARAM));
             

services/src/main/java/org/keycloak/protocol/oidc/endpoints/UserInfoEndpoint.java

@@ -203,11 +203,17 @@ public class UserInfoEndpoint {
         ClientSessionContext clientSessionCtx = DefaultClientSessionContext.fromClientSessionScopeParameter(clientSession, session);
 
         AccessToken userInfo = new AccessToken();
+        
+        userInfo.subject(userModel.getId());
+        
         tokenManager.transformUserInfoAccessToken(session, userInfo, userSession, clientSessionCtx);
 
         Map<String, Object> claims = new HashMap<>();
-        claims.put("sub", userModel.getId());
         claims.putAll(userInfo.getOtherClaims());
+        // we always set the subject to the correct value and ignore any mapper (not directly related to subject mapping such as 
+        // pseudo-subjects). the endpoint should always return a valid subject identifier.
+        // any attempt to customize the value of this field should be done through a different claim
+        claims.put("sub", userInfo.getSubject());
 
         Response.ResponseBuilder responseBuilder;
         OIDCAdvancedConfigWrapper cfg = OIDCAdvancedConfigWrapper.fromClientModel(clientModel);

services/src/main/java/org/keycloak/protocol/oidc/mappers/AbstractPairwiseSubMapper.java

@@ -90,7 +90,7 @@ public abstract class AbstractPairwiseSubMapper extends AbstractOIDCProtocolMapp
     }
 
     protected void setUserInfoTokenSubject(IDToken token, String pairwiseSub) {
-        token.getOtherClaims().put("sub", pairwiseSub);
+        token.setSubject(pairwiseSub);
     }
 
     @Override

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/UserInfoClientUtil.java

@@ -50,6 +50,10 @@ public class UserInfoClientUtil {
     }
 
     public static UserInfo testSuccessfulUserInfoResponse(Response response, String expectedUsername, String expectedEmail) {
+        return testSuccessfulUserInfoResponse(response, null, expectedUsername, expectedEmail);
+    }
+
+    public static UserInfo testSuccessfulUserInfoResponse(Response response, String userId, String expectedUsername, String expectedEmail) {
         Assert.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus());
         Assert.assertEquals(response.getHeaderString(HttpHeaders.CONTENT_TYPE), MediaType.APPLICATION_JSON);
 
@@ -59,6 +63,9 @@ public class UserInfoClientUtil {
 
         Assert.assertNotNull(userInfo);
         Assert.assertNotNull(userInfo.getSubject());
+        if (userId != null) {
+            Assert.assertEquals(userId, userInfo.getSubject());
+        }
         Assert.assertEquals(expectedEmail, userInfo.getEmail());
         Assert.assertEquals(expectedUsername, userInfo.getPreferredUsername());
         return userInfo;

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerSubMatchIntrospectionest.java

@@ -0,0 +1,72 @@
+package org.keycloak.testsuite.broker;
+
+import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage;
+import static org.keycloak.testsuite.util.ProtocolMapperUtil.createHardcodedClaim;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.keycloak.jose.jws.JWSInput;
+import org.keycloak.jose.jws.JWSInputException;
+import org.keycloak.representations.IDToken;
+import org.keycloak.representations.idm.ClientRepresentation;
+import org.keycloak.representations.idm.ProtocolMapperRepresentation;
+import org.keycloak.testsuite.arquillian.SuiteContext;
+import org.keycloak.testsuite.util.ClientBuilder;
+import org.keycloak.testsuite.util.OAuthClient;
+
+public class KcOidcBrokerSubMatchIntrospectionest extends AbstractBrokerTest {
+
+    @Override
+    protected BrokerConfiguration getBrokerConfiguration() {
+        return new KcOidcBrokerConfiguration() {
+            @Override
+            public List<ClientRepresentation> createConsumerClients(SuiteContext suiteContext) {
+                List<ClientRepresentation> clients = new ArrayList<>(super.createConsumerClients(suiteContext));
+                
+                clients.add(ClientBuilder.create().clientId("consumer-client")
+                        .publicClient()
+                        .redirectUris("http://localhost:8180/auth/realms/master/app/auth/*", "https://localhost:8543/auth/realms/master/app/auth/*")
+                        .publicClient().build());
+                
+                return clients;
+            }
+
+            @Override
+            public List<ClientRepresentation> createProviderClients(SuiteContext suiteContext) {
+                List<ClientRepresentation> clients = super.createProviderClients(suiteContext);
+                List<ProtocolMapperRepresentation> mappers = new ArrayList<>();
+                
+                mappers.add(createHardcodedClaim("sub-override", "sub", "overriden", "String", true, true));
+                
+                clients.get(0).setProtocolMappers(mappers);
+                
+                return clients;
+            }
+        };
+    }
+
+    @Override
+    public void testLogInAsUserInIDP() {
+        driver.navigate().to(getAccountUrl(bc.consumerRealmName()));
+
+        oauth.realm(bc.consumerRealmName());
+        oauth.clientId("consumer-client");
+
+        log.debug("Clicking social " + bc.getIDPAlias());
+        loginPage.clickSocial(bc.getIDPAlias());
+        waitForPage(driver, "log in to", true);
+
+        log.debug("Logging in");
+        loginPage.login(bc.getUserLogin(), bc.getUserPassword());
+        errorPage.assertCurrent();
+    }
+
+    @Ignore
+    @Override
+    public void loginWithExistingUser() {
+    }
+}

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/UserInfoTest.java

@@ -37,6 +37,7 @@ import org.keycloak.jose.jws.crypto.RSAProvider;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.representations.AccessToken;
 import org.keycloak.representations.idm.RoleRepresentation;
+import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.testsuite.util.KeycloakModelUtils;
 import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
 import org.keycloak.protocol.oidc.OIDCLoginProtocolService;
@@ -594,7 +595,8 @@ public class UserInfoTest extends AbstractKeycloakTest {
                 .detail(Details.SIGNATURE_REQUIRED, "false")
                 .client(expectedClientId)
                 .assertEvent();
-        UserInfoClientUtil.testSuccessfulUserInfoResponse(response, "test-user@localhost", "test-user@localhost");
+        UserRepresentation user = adminClient.realm("test").users().search("test-user@localhost").get(0);
+        UserInfoClientUtil.testSuccessfulUserInfoResponse(response, user.getId(), "test-user@localhost", "test-user@localhost");
     }
 
     private void testSuccessSignedResponse(Algorithm sigAlg) throws Exception {