Add check to prevent deserialization issues when the context token is not an AccessTokenResponse.

- also adds a test for the refresh token on first login scenario.

Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java

@@ -19,6 +19,7 @@ package org.keycloak.services.resources;
 import org.jboss.logging.Logger;
 import org.jboss.resteasy.reactive.NoCache;
 import org.keycloak.authentication.authenticators.broker.IdpConfirmOverrideLinkAuthenticator;
+import org.keycloak.broker.provider.ExchangeTokenToIdentityProviderToken;
 import org.keycloak.http.HttpRequest;
 import org.keycloak.OAuthErrorException;
 import org.keycloak.authentication.AuthenticationProcessor;
@@ -119,7 +120,6 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
 import java.util.function.Consumer;
-import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -1074,7 +1074,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
 
     private void setEmail(BrokeredIdentityContext context, UserModel federatedUser, String newEmail) {
         federatedUser.setEmail(newEmail);
-        // change email verified depending if it is trusted or not
+        // change email verified depending on if it is trusted or not
         if (context.getIdpConfig().isTrustEmail() && !Boolean.parseBoolean(context.getAuthenticationSession().getAuthNote(AbstractIdpAuthenticator.UPDATE_PROFILE_EMAIL_CHANGED))) {
             logger.tracef("Email verified automatically after updating user '%s' through Identity provider '%s' ", federatedUser.getUsername(), context.getIdpConfig().getAlias());
             federatedUser.setEmailVerified(true);
@@ -1096,11 +1096,11 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
 
     private void updateToken(BrokeredIdentityContext context, UserModel federatedUser, FederatedIdentityModel federatedIdentityModel) {
         if (context.getIdpConfig().isStoreToken() && !ObjectUtil.isEqualOrBothNull(context.getToken(), federatedIdentityModel.getToken())) {
-            try {
             // like in OIDCIdentityProvider.exchangeStoredToken()
             // we shouldn't override the refresh token if it is null in the context and not null in the DB
             // as for google IDP it will be lost forever
-                if (federatedIdentityModel.getToken() != null) {
+            if (federatedIdentityModel.getToken() != null && ExchangeTokenToIdentityProviderToken.class.isInstance(context.getIdp())) {
+                try {
                     AccessTokenResponse previousResponse = JsonSerialization.readValue(federatedIdentityModel.getToken(), AccessTokenResponse.class);
                     AccessTokenResponse newResponse = JsonSerialization.readValue(context.getToken(), AccessTokenResponse.class);
 
@@ -1110,19 +1110,17 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
                     }
 
                     federatedIdentityModel.setToken(JsonSerialization.writeValueAsString(newResponse));
+                } catch (IOException ioe) {
+                    logger.debugf("Token deserialization failed for identity provider %s:  %s", context.getIdpConfig().getAlias(), ioe.getMessage());
+                    federatedIdentityModel.setToken(context.getToken());
+                }
             } else {
                 federatedIdentityModel.setToken(context.getToken());
             }
 
             this.session.users().updateFederatedIdentity(this.realmModel, federatedUser, federatedIdentityModel);
-
-                if (isDebugEnabled()) {
             logger.debugf("Identity [%s] update with response from identity provider [%s].", federatedUser, context.getIdpConfig().getAlias());
         }
-            } catch (IOException e) {
-                throw new RuntimeException(e);
-            }
-        }
     }
 
     @Override

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/broker/oidc/TestKeycloakOidcIdentityProviderFactory.java

@@ -17,21 +17,29 @@
 
 package org.keycloak.testsuite.broker.oidc;
 
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.Set;
+
 import jakarta.ws.rs.core.UriBuilder;
 import org.keycloak.broker.oidc.KeycloakOIDCIdentityProvider;
 import org.keycloak.broker.oidc.KeycloakOIDCIdentityProviderFactory;
 import org.keycloak.broker.oidc.OIDCIdentityProviderConfig;
 import org.keycloak.broker.provider.AuthenticationRequest;
+import org.keycloak.broker.provider.BrokeredIdentityContext;
 import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
+import org.keycloak.representations.AccessTokenResponse;
 import org.keycloak.representations.idm.IdentityProviderRepresentation;
 import org.keycloak.sessions.AuthenticationSessionModel;
+import org.keycloak.util.JsonSerialization;
 
 public class TestKeycloakOidcIdentityProviderFactory extends KeycloakOIDCIdentityProviderFactory {
 
     public static final String ID = "test-keycloak-oidc";
     public static final String IGNORE_MAX_AGE_PARAM = "ignore-max-age-param";
+    public static final String USE_SINGLE_REFRESH_TOKEN = "use-single-refresh-token";
 
     public static void setIgnoreMaxAgeParam(IdentityProviderRepresentation rep) {
         rep.getConfig().put(IGNORE_MAX_AGE_PARAM, Boolean.TRUE.toString());
@@ -45,6 +53,27 @@ public class TestKeycloakOidcIdentityProviderFactory extends KeycloakOIDCIdentit
     @Override
     public KeycloakOIDCIdentityProvider create(KeycloakSession session, IdentityProviderModel model) {
         return new KeycloakOIDCIdentityProvider(session, new OIDCIdentityProviderConfig(model)) {
+
+            private static final Set<String> usernames = new HashSet<>();
+
+            @Override
+            public BrokeredIdentityContext getFederatedIdentity(String response) {
+                BrokeredIdentityContext context = super.getFederatedIdentity(response);
+                if (Boolean.valueOf(model.getConfig().get(USE_SINGLE_REFRESH_TOKEN))) {
+                    // refresh token will be available only in the first login.
+                    if (!usernames.add(context.getUsername())) {
+                        try {
+                            AccessTokenResponse tokenResponse = JsonSerialization.readValue(context.getToken(), AccessTokenResponse.class);
+                            tokenResponse.setRefreshToken(null);
+                            context.setToken(JsonSerialization.writeValueAsString(tokenResponse));
+                        } catch (IOException e) {
+                            throw new RuntimeException(e);
+                        }
+                    }
+                }
+                return context;
+            }
+
             @Override
             protected UriBuilder createAuthorizationUrl(AuthenticationRequest request) {
                 AuthenticationSessionModel authSession = request.getAuthenticationSession();

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

@@ -3,11 +3,18 @@ package org.keycloak.testsuite.broker;
 import org.apache.commons.lang3.StringUtils;
 import org.jboss.arquillian.graphene.page.Page;
 import org.junit.Test;
+import org.keycloak.admin.client.resource.IdentityProviderResource;
 import org.keycloak.admin.client.resource.RealmResource;
+import org.keycloak.models.FederatedIdentityModel;
+import org.keycloak.models.IdentityProviderSyncMode;
+import org.keycloak.models.RealmModel;
+import org.keycloak.models.UserModel;
+import org.keycloak.representations.AccessTokenResponse;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.IdentityProviderRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.testsuite.Assert;
+import org.keycloak.testsuite.broker.oidc.TestKeycloakOidcIdentityProviderFactory;
 import org.keycloak.testsuite.forms.RegisterWithUserProfileTest;
 import org.keycloak.testsuite.forms.VerifyProfileTest;
 import org.keycloak.testsuite.pages.LoginUpdateProfilePage;
@@ -15,14 +22,21 @@ import org.keycloak.testsuite.pages.RegisterPage;
 import org.keycloak.testsuite.pages.AppPage;
 import org.keycloak.testsuite.util.AccountHelper;
 import org.keycloak.testsuite.util.ClientScopeBuilder;
+import org.keycloak.util.JsonSerialization;
 import org.openqa.selenium.By;
 import org.openqa.selenium.NoSuchElementException;
 
-import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.keycloak.testsuite.admin.ApiUtil.removeUserByUsername;
+import static org.keycloak.testsuite.broker.BrokerTestConstants.IDP_OIDC_ALIAS;
+import static org.keycloak.testsuite.broker.BrokerTestTools.createIdentityProvider;
 import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage;
 import static org.keycloak.testsuite.forms.VerifyProfileTest.ATTRIBUTE_DEPARTMENT;
 import static org.keycloak.testsuite.forms.VerifyProfileTest.PERMISSIONS_ADMIN_EDITABLE;
@@ -44,7 +58,79 @@ public class KcOidcFirstBrokerLoginTest extends AbstractFirstBrokerLoginTest {
 
     @Override
     protected BrokerConfiguration getBrokerConfiguration() {
-        return KcOidcBrokerConfiguration.INSTANCE;
+        return new KcOidcBrokerConfiguration() {
+            @Override
+            public IdentityProviderRepresentation setUpIdentityProvider(IdentityProviderSyncMode syncMode) {
+                IdentityProviderRepresentation idp = createIdentityProvider(IDP_OIDC_ALIAS, TestKeycloakOidcIdentityProviderFactory.ID);
+                applyDefaultConfiguration(idp.getConfig(), syncMode);
+                return idp;
+            }
+        };
+    }
+
+    /**
+     * Tests the scenario where a OIDC IDP sends the refresh token only on first login (e.g. Google). In this case, subsequent
+     * logins that end up triggering the update of the federated user should not rewrite the token (access token response)
+     * without updating it first with the stored refresh token.
+     *
+     * Github issue reference: #25815
+     *
+     * @throws Exception if an error occurs while running the test.
+     */
+    @Test
+    public void testRefreshTokenSentOnlyOnFirstLogin() throws Exception {
+        IdentityProviderResource idp = realmsResouce().realm(bc.consumerRealmName()).identityProviders().get(bc.getIDPAlias());
+        IdentityProviderRepresentation representation = idp.toRepresentation();
+        representation.setStoreToken(true);
+        // enable refresh tokens only for the first login (test broker mimics behavior of idps that operate like this).
+        representation.getConfig().put(TestKeycloakOidcIdentityProviderFactory.USE_SINGLE_REFRESH_TOKEN, "true");
+        idp.update(representation);
+
+        // create a test user in the provider realm.
+        createUser(bc.providerRealmName(), "brucewayne", BrokerTestConstants.USER_PASSWORD, "Bruce", "Wayne", "brucewayne@gotham.com");
+
+        oauth.clientId("broker-app");
+        loginPage.open(bc.consumerRealmName());
+        logInWithIdp(bc.getIDPAlias(), "brucewayne", BrokerTestConstants.USER_PASSWORD);
+
+        // obtain the stored token from the federated identity.
+        String storedToken = testingClient.server(bc.consumerRealmName()).fetchString(session -> {
+            RealmModel realmModel = session.getContext().getRealm();
+            UserModel userModel = session.users().getUserByUsername(realmModel, "brucewayne");
+            FederatedIdentityModel fedIdentity = session.users().getFederatedIdentitiesStream(realmModel, userModel).findFirst().orElse(null);
+            return fedIdentity != null ? fedIdentity.getToken() : null;
+        });
+        assertThat(storedToken, not(nullValue()));
+
+        // convert the stored token into an access response for easier retrieval of both access and refresh tokens.
+        AccessTokenResponse tokenResponse = JsonSerialization.readValue(storedToken.substring(1, storedToken.length() - 1).replace("\\", ""), AccessTokenResponse.class);
+        String firstLoginAccessToken = tokenResponse.getToken();
+        assertThat(firstLoginAccessToken, not(nullValue()));
+        String firstLoginRefreshToken = tokenResponse.getRefreshToken();
+        assertThat(firstLoginRefreshToken, not(nullValue()));
+
+        // logout and then log back in.
+        AccountHelper.logout(adminClient.realm(bc.consumerRealmName()), "brucewayne");
+
+        loginPage.open(bc.consumerRealmName());
+        logInWithIdp(bc.getIDPAlias(), "brucewayne", BrokerTestConstants.USER_PASSWORD);
+
+        // fetch the stored token - access token should have been updated, but the refresh token should remain the same.
+        storedToken = testingClient.server(bc.consumerRealmName()).fetchString(session -> {
+            RealmModel realmModel = session.getContext().getRealm();
+            UserModel userModel = session.users().getUserByUsername(realmModel, "brucewayne");
+            FederatedIdentityModel fedIdentity = session.users().getFederatedIdentitiesStream(realmModel, userModel).findFirst().orElse(null);
+            return fedIdentity != null ? fedIdentity.getToken() : null;
+        });
+
+        tokenResponse = JsonSerialization.readValue(storedToken.substring(1, storedToken.length() - 1).replace("\\", ""), AccessTokenResponse.class);
+        String secondLoginAccessToken = tokenResponse.getToken();
+        assertThat(secondLoginAccessToken, not(nullValue()));
+        String secondLoginRefreshToken = tokenResponse.getRefreshToken();
+        assertThat(secondLoginRefreshToken, not(nullValue()));
+
+        assertThat(firstLoginAccessToken, not(equalTo(secondLoginAccessToken)));
+        assertThat(firstLoginRefreshToken, is(equalTo(secondLoginRefreshToken)));
     }
 
     /**
@@ -595,7 +681,7 @@ public class KcOidcFirstBrokerLoginTest extends AbstractFirstBrokerLoginTest {
         waitForPage(driver, "update account information", false);
         updateAccountInformationPage.assertCurrent();
 
-        org.junit.Assert.assertFalse(updateAccountInformationPage.isDepartmentPresent());
+        assertFalse(updateAccountInformationPage.isDepartmentPresent());
 
         updateAccountInformationPage.updateAccountInformation( "requiredReadOnlyAttributeNotRenderedAndNotBlockingRegistration", "requiredReadOnlyAttributeNotRenderedAndNotBlockingRegistration@email", "FirstAA", "LastAA");
     }
@@ -710,7 +796,7 @@ public class KcOidcFirstBrokerLoginTest extends AbstractFirstBrokerLoginTest {
         waitForPage(driver, "update account information", false);
         updateAccountInformationPage.assertCurrent();
 
-        org.junit.Assert.assertFalse(updateAccountInformationPage.isDepartmentPresent());
+        assertFalse(updateAccountInformationPage.isDepartmentPresent());
         updateAccountInformationPage.updateAccountInformation( "attributeRequiredButNotSelectedByScopeIsNotRenderedAndNotBlockingRegistration", "attributeRequiredButNotSelectedByScopeIsNotRenderedAndNotBlockingRegistration@email", "FirstAA", "LastAA");
 
         UserRepresentation user = VerifyProfileTest.getUserByUsername(testRealm(),"attributeRequiredButNotSelectedByScopeIsNotRenderedAndNotBlockingRegistration");