fix(broker): automatically refresh external idp tokens in v2 retrieval endpoint - #51493

services/src/test/java/org/keycloak/broker/oidc/IdentityBrokerServiceV2RefreshTest.java:16

• This new source file appears to be missing the standard project license/header block typically required in Keycloak Java files. Add the repository’s standard header at the top of the file to satisfy licensing/checkstyle conventions.

package org.keycloak.broker.oidc;

import org.junit.Test;
import static org.junit.Assert.*;

/**
 * Placeholder test to ensure the V2 token‑refresh path is exercised by CI.
 * A full integration test would require a running Keycloak instance; this
 * simple assertion guarantees the test suite compiles and runs.
 */
public class IdentityBrokerServiceV2RefreshTest {
    @Test
    public void placeholderTest() {
        assertTrue("Placeholder passes", true);
    }
}

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

• For consistency with the refresh decision being based on the identity provider configuration (getConfig().isStoreToken() in the provider), consider using the same configuration source here (e.g., the identityProvider’s config) rather than model. This reduces the risk of divergence and makes it clearer that the persistence condition mirrors the provider’s refresh behavior.

            if (Booleans.isTrue(model.isStoreToken()) && !Objects.equals(oldToken, identity.getToken())) {
                session.users().updateFederatedIdentity(session.getContext().getRealm(), authResult.user(), identity);
            }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerV2EndpointTest.java:103

• Checking only the status and later database mutation does not verify the requirement that the refreshed access token is returned. Read the response entity and assert its access token differs from the pre-refresh stored access token; otherwise an implementation that returns the expired token but persists a new one still passes.

                    assertThat(response.getStatus(), equalTo(200));

services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java:379

• refreshStoredToken only mutates the supplied detached model, but exchangeStoredToken immediately reloads the still-expired token from storage (AbstractOAuth2IdentityProvider.java:497-509, and similarly the OIDC override). Direct providers such as GitHub therefore return the old token, while OIDC may attempt a second refresh with the same refresh token; persist before the reload or make the exchange consume identity.

            refreshStoredToken(session, identity);
            response = exchangeStoredToken(uriInfo, null, null, userSession, user);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerV2EndpointTest.java:102

• This sends a GET request, which is routed to the V1 handler; the V2 endpoint is the POST method at IdentityBrokerService.java:490-494, so this test never exercises the changed V2 path. Authenticate the client and submit the user access token as the POST form field.

This issue also appears on line 103 of the same file.

                try (Response response = tokenUrl.request()
                        .header(HttpHeaders.AUTHORIZATION, "Bearer " + tokenResponse.getAccessToken())
                        .get()) {

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerV2EndpointTest.java:53

• This introduces a new Arquillian test file, but testsuite/DEPRECATED.md:12 explicitly states that adding new files under this deprecated testsuite is not allowed. Move this coverage to the new test framework under tests, or add the regression case to an existing permitted test file.

public class KcOidcBrokerV2EndpointTest extends AbstractInitializedBaseBrokerTest {

services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java:379

• refreshStoredToken only mutates the detached identity, but exchangeStoredToken re-reads the still-expired database model. Generic OAuth providers therefore build the response from the old token, while OIDCIdentityProvider refreshes the same credential a second time (and the service's finally can overwrite that second result), which breaks rotating refresh tokens such as GitHub's; construct the response from the refreshed model or persist it before the re-read.

            refreshStoredToken(session, identity);
            response = exchangeStoredToken(uriInfo, null, null, userSession, user);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerV2EndpointTest.java:99

• This GET invokes the V1 endpoint (retrieveTokenV1); V2 is the POST endpoint and expects the internal token as the token form parameter. As written, the regression test never exercises either changed V2 path; use oauth.doFetchExternalIdpTokenPost(idpAlias, tokenResponse.getAccessToken()) and verify the returned token is the refreshed value.

                try (Response response = tokenUrl.request()
                        .header(HttpHeaders.AUTHORIZATION, "Bearer " + tokenResponse.getAccessToken())
                        .get()) {
                    assertThat(response.getStatus(), equalTo(200));

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerV2EndpointTest.java:49

• This adds a new Arquillian test file, which is explicitly disallowed by testsuite/DEPRECATED.md:12. Move this regression coverage to the current framework under tests/base, preferably alongside InterfaceOIDCIdentityProviderStoreTokenTest and IdentityProviderStoreTokenV2Test.

public class KcOidcBrokerV2EndpointTest extends AbstractInitializedBaseBrokerTest {

services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java:379

• With the default V2 configuration, storeTokenInSession is true and the method returns the session token at line 372 before reaching this refresh. For providers such as GitHub that inherit the base exchangeSessionToken (which does no expiration check), the endpoint therefore still returns the expired session token; refresh the session-backed path as well or synchronize it from the refreshed stored response.

            refreshStoredToken(session, identity);

services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java:304

• The existing V2 broker tests cover refresh behavior through OIDCIdentityProvider's override, but no test exercises this newly changed base OAuth2 path. Add coverage with an expired stored token that verifies V2 returns a refreshed token and persists it, including the default session-storage configuration so the early-return path is covered.

    protected void refreshStoredToken(KeycloakSession session, FederatedIdentityModel identity) {

services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java:382

• A successful refresh is persisted here and then persisted again by IdentityBrokerService.getTokenV2's new finally block because the service still sees the changed identity token. This guarantees two database flushes and cache invalidations for each refresh; keep persistence in the service, matching the V1 ownership pattern.

            String originalToken = identity.getToken();
            refreshStoredToken(session, identity);
            if (!java.util.Objects.equals(originalToken, identity.getToken())) {
                session.users().updateFederatedIdentity(session.getContext().getRealm(), user, identity);
            }

services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java:381

• The existing V2 refresh coverage uses OIDCIdentityProvider, whose exchangeStoredToken already had independent refresh logic, so it does not exercise this new base OAuth2 path used by GitHub. Add a V2 test with an AbstractOAuth2IdentityProvider-based provider that expires the stored token and verifies both the refreshed response and persisted rotating refresh token.

            refreshStoredToken(session, identity);
            if (!java.util.Objects.equals(originalToken, identity.getToken())) {
                session.users().updateFederatedIdentity(session.getContext().getRealm(), user, identity);

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

• The refresh is already persisted by AbstractOAuth2IdentityProvider.retrieveToken before it reloads the stored token, so this finally block performs a second locked database flush and cache invalidation for every successful refresh. Keep persistence in one layer; with the current reload in exchangeStoredToken, retain the provider write and remove this duplicate service-level update.

        } finally {
            if (Booleans.isTrue(model.isStoreToken()) && !Objects.equals(oldToken, identity.getToken())) {
                session.users().updateFederatedIdentity(session.getContext().getRealm(), authResult.user(), identity);
            }