fix for service account role management in admin v2 (#48166)

* fix for service account role management in admin v2 fixes: #47966 Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * fixed merge error Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * fix test Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * fixed build error Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * fixed tests Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * Remove unused RealmAdminResource from DefaultClientsApi and DefaultAdminApi Signed-off-by: Peter Zaoral <pzaoral@redhat.com> * Update rest/admin-v2/services/src/main/java/org/keycloak/services/client/DefaultClientService.java Co-authored-by: Peter Zaoral <pepo48@gmail.com> Signed-off-by: Erik Jan de Wit <edewit@redhat.com> * also update the context Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * fix merge error Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * revert change Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * Update rest/admin-v2/services/src/main/java/org/keycloak/rest/admin/api/DefaultAdminApi.java Co-authored-by: Peter Zaoral <pepo48@gmail.com> Signed-off-by: Erik Jan de Wit <edewit@redhat.com> * Update rest/admin-v2/services/src/main/java/org/keycloak/rest/admin/api/DefaultAdminApi.java Co-authored-by: Peter Zaoral <pepo48@gmail.com> Signed-off-by: Erik Jan de Wit <edewit@redhat.com> --------- Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> Signed-off-by: Peter Zaoral <pzaoral@redhat.com> Signed-off-by: Erik Jan de Wit <edewit@redhat.com> Co-authored-by: Peter Zaoral <pzaoral@redhat.com> Co-authored-by: Peter Zaoral <pepo48@gmail.com>
2026-05-26 13:50:48 +00:00 · 2026-05-21 08:30:16 +02:00
parent 492d1f04cd
commit 27262be569
8 changed files with 185 additions and 89 deletions
@@ -20,6 +20,10 @@
            <groupId>org.keycloak</groupId>
            <artifactId>keycloak-quarkus-server</artifactId>
        </dependency>
+        <dependency>
+            <groupId>org.keycloak</groupId>
+            <artifactId>keycloak-core</artifactId>
+        </dependency>
        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-core-deployment</artifactId>
@@ -2,7 +2,6 @@ package org.keycloak.admin.api.client;

 import java.io.InputStream;

-import jakarta.validation.Valid;
 import jakarta.ws.rs.Consumes;
 import jakarta.ws.rs.DELETE;
 import jakarta.ws.rs.GET;
@@ -42,7 +41,7 @@ public interface ClientApi {
        @APIResponse(responseCode = "200", content = @Content(schema = @Schema(implementation = BaseClientRepresentation.class))),
        @APIResponse(responseCode = "201", description = "Created", content = @Content(schema = @Schema(implementation = BaseClientRepresentation.class)))
    })
-    Response createOrUpdateClient(@Valid BaseClientRepresentation client);
+    Response createOrUpdateClient(BaseClientRepresentation client);

    @PATCH
    @Consumes(PatchTypeNames.JSON_MERGE)
@@ -1,14 +1,13 @@
 package org.keycloak.rest.admin.api;

+import jakarta.ws.rs.NotFoundException;
+
 import org.keycloak.admin.api.AdminApi;
 import org.keycloak.admin.api.client.ClientsApi;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
-import org.keycloak.protocol.oidc.TokenManager;
 import org.keycloak.rest.admin.api.client.DefaultClientsApi;
 import org.keycloak.services.resources.admin.AdminRoot;
-import org.keycloak.services.resources.admin.RealmAdminResource;
-import org.keycloak.services.resources.admin.RealmsAdminResource;
 import org.keycloak.services.resources.admin.fgap.AdminPermissionEvaluator;
 import org.keycloak.services.resources.admin.fgap.AdminPermissions;

@@ -17,20 +16,18 @@ public class DefaultAdminApi implements AdminApi {
    private final RealmModel realm;
    private final AdminPermissionEvaluator permissions;

-    // v1 resources
-    private final RealmAdminResource realmAdminResource;
-
    public DefaultAdminApi(KeycloakSession session, String realmName) {
        this.session = session;
        var authInfo = AdminRoot.authenticateRealmAdminRequest(session);
-        this.permissions = AdminPermissions.evaluator(session, authInfo.getRealm(), authInfo);
-        this.realm = session.realms().getRealmByName(realmName);
-        // remove v1 resource once we are not attached to API v1
-        this.realmAdminResource = new RealmsAdminResource(session, authInfo, new TokenManager()).getRealmAdmin(realmName);
+        RealmModel realm = session.realms().getRealmByName(realmName);
+        if (realm == null) throw new NotFoundException("Realm not found.");
+        session.getContext().setRealm(realm);
+        this.realm = realm;
+        this.permissions = AdminPermissions.evaluator(session, realm, authInfo);
    }

    @Override
    public ClientsApi clientsV2() {
-        return new DefaultClientsApi(session, realm, permissions, realmAdminResource);
+        return new DefaultClientsApi(session, realm, permissions);
    }
 }
@@ -20,7 +20,6 @@ import org.keycloak.services.PatchType;
 import org.keycloak.services.ServiceException;
 import org.keycloak.services.client.ClientService;
 import org.keycloak.services.client.DefaultClientService;
-import org.keycloak.services.resources.admin.RealmAdminResource;
 import org.keycloak.services.resources.admin.fgap.AdminPermissionEvaluator;


@@ -34,14 +33,12 @@ public class DefaultClientApi implements ClientApi {
    public DefaultClientApi(@Nonnull KeycloakSession session,
                            @Nonnull RealmModel realm,
                            @Nonnull String clientId,
-                            @Nonnull AdminPermissionEvaluator permissions,
-                            // remove v1 resource once we are not attached to API v1
-                            @Nonnull RealmAdminResource realmAdminResource) {
+                            @Nonnull AdminPermissionEvaluator permissions) {
        this.session = session;
        this.clientId = clientId;
        this.realm = realm;
        this.permissions = permissions;
-        this.clientService = new DefaultClientService(session, realm, permissions, realmAdminResource);
+        this.clientService = new DefaultClientService(session, realm, permissions);
    }

    @GET
@@ -19,7 +19,6 @@ import org.keycloak.representations.admin.v2.BaseClientRepresentation;
 import org.keycloak.services.client.ClientService;
 import org.keycloak.services.client.ClientService.ClientProjectionOptions;
 import org.keycloak.services.client.DefaultClientService;
-import org.keycloak.services.resources.admin.RealmAdminResource;
 import org.keycloak.services.resources.admin.fgap.AdminPermissionEvaluator;

 public class DefaultClientsApi implements ClientsApi {
@@ -29,19 +28,13 @@ public class DefaultClientsApi implements ClientsApi {
    private final RealmModel realm;
    private final ClientService clientService;

-    // v1 resources
-    private final RealmAdminResource realmAdminResource;
-
    public DefaultClientsApi(@Nonnull KeycloakSession session,
                             @Nonnull RealmModel realm,
-                             @Nonnull AdminPermissionEvaluator permissions,
-                             // remove v1 resource once we are not attached to API v1
-                             @Nonnull RealmAdminResource realmAdminResource) {
+                             @Nonnull AdminPermissionEvaluator permissions) {
        this.session = session;
        this.realm = realm;
        this.permissions = permissions;
-        this.realmAdminResource = realmAdminResource;
-        this.clientService = new DefaultClientService(session, realm, permissions, realmAdminResource);
+        this.clientService = new DefaultClientService(session, realm, permissions);
    }
    
    @Override
@@ -71,7 +64,7 @@ public class DefaultClientsApi implements ClientsApi {
    @Override
    public ClientApi client(@PathParam("id") String clientId) {
        enforceAntiPhishingIfClientMissing(clientId);
-        return new DefaultClientApi(session, realm, clientId, permissions, realmAdminResource);
+        return new DefaultClientApi(session, realm, clientId, permissions);
    }

 }
@@ -2,6 +2,7 @@ package org.keycloak.services.client;

 import java.io.IOException;
 import java.io.InputStream;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
@@ -13,7 +14,6 @@ import java.util.stream.Stream;

 import jakarta.annotation.Nonnull;
 import jakarta.validation.groups.Default;
-import jakarta.ws.rs.NotFoundException;
 import jakarta.ws.rs.core.Response;

 import org.keycloak.authorization.fgap.AdminPermissionsSchema;
@@ -26,6 +26,7 @@ import org.keycloak.models.ModelException;
 import org.keycloak.models.ModelValidationException;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RoleModel;
+import org.keycloak.models.UserModel;
 import org.keycloak.models.mapper.ClientModelMapper;
 import org.keycloak.models.mapper.ClientModelMappers;
 import org.keycloak.models.utils.KeycloakModelUtils;
@@ -50,7 +51,6 @@ import org.keycloak.services.clientpolicy.context.AdminClientViewContext;
 import org.keycloak.services.managers.ClientManager;
 import org.keycloak.services.managers.RealmManager;
 import org.keycloak.services.resources.admin.AdminEventBuilder;
-import org.keycloak.services.resources.admin.RealmAdminResource;
 import org.keycloak.services.resources.admin.RoleContainerResource;
 import org.keycloak.services.resources.admin.fgap.AdminPermissionEvaluator;
 import org.keycloak.services.util.ObjectMapperResolver;
@@ -63,6 +63,7 @@ import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.core.JsonToken;
 import com.fasterxml.jackson.databind.JsonMappingException;
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectReader;
 import org.apache.http.HttpEntity;
@@ -80,18 +81,15 @@ public class DefaultClientService implements ClientService {

    private final KeycloakSession session;
    private final AdminPermissionEvaluator permissions;
-    private final RealmAdminResource realmResource;
    private final AdminEventBuilder adminEventBuilder;
    private final JakartaValidatorProvider validator;
    private final RolesService rolesService;

    public DefaultClientService(@Nonnull KeycloakSession session,
                                @Nonnull RealmModel realm,
-                                @Nonnull AdminPermissionEvaluator permissions,
-                                @Nonnull RealmAdminResource realmResource) {
+                                @Nonnull AdminPermissionEvaluator permissions) {
        this.session = session;
        this.permissions = permissions;
-        this.realmResource = realmResource;
        this.adminEventBuilder = new AdminEventV2Builder(realm, permissions.adminAuth(), session, session.getContext().getConnection()).resource(ResourceType.CLIENT);
        this.validator = new HibernateValidatorProvider(new ValidationContext(session, realm));
        this.rolesService = new RolesService(session, realm, permissions, adminEventBuilder);
@@ -148,12 +146,12 @@ public class DefaultClientService implements ClientService {

    @Override
    public BaseClientRepresentation createClient(RealmModel realm, BaseClientRepresentation client) throws ServiceException {
-        return createOrUpdate(realm, null, client, CreateOrUpdateStrategy.ONLY_CREATE).representation();
+        return createOrUpdate(realm, null, client, CreateOrUpdateStrategy.ONLY_CREATE, false).representation();
    }

    @Override
    public CreateOrUpdateResult createOrUpdateClient(RealmModel realm, String clientId, BaseClientRepresentation client) throws ServiceException {
-        return createOrUpdate(realm, clientId, client, CreateOrUpdateStrategy.PUT);
+        return createOrUpdate(realm, clientId, client, CreateOrUpdateStrategy.PUT, false);
    }

    @Override
@@ -189,9 +187,25 @@ public class DefaultClientService implements ClientService {
                .orElseThrow(() -> new ServiceException("Cannot find the specified client", Response.Status.NOT_FOUND));

        BaseClientRepresentation updated;
+        boolean patchExplicitNullSecret = false;
        switch (patchType) {
            case JSON_MERGE -> {
-                try (JsonParser parser = MAPPER.getFactory().createParser(patch)) {
+                final byte[] patchData;
+                try {
+                    patchData = patch.readAllBytes();
+                } catch (IOException e) {
+                    throw new ServiceException("Unknown Error Occurred", Response.Status.INTERNAL_SERVER_ERROR);
+                }
+                try {
+                    JsonNode root = MAPPER.readTree(patchData);
+                    JsonNode authNode = root.get("auth");
+                    if (authNode != null && authNode.has("secret") && authNode.get("secret").isNull()) {
+                        patchExplicitNullSecret = true;
+                    }
+                } catch (IOException e) {
+                    throw new ServiceException(e.getMessage(), Response.Status.BAD_REQUEST);
+                }
+                try (JsonParser parser = MAPPER.getFactory().createParser(patchData)) {
                    final ObjectReader objectReader = MAPPER.readerForUpdating(getOriginalClient.get());
                    JsonToken nextToken = parser.nextToken();
                    if (nextToken != JsonToken.START_OBJECT) {
@@ -213,7 +227,7 @@ public class DefaultClientService implements ClientService {
            default -> throw new ServiceException("Invalid patch type", Response.Status.UNSUPPORTED_MEDIA_TYPE);
        }

-        return createOrUpdate(realm, clientId, updated, CreateOrUpdateStrategy.PATCH).representation();
+        return createOrUpdate(realm, clientId, updated, CreateOrUpdateStrategy.PATCH, patchExplicitNullSecret).representation();
    }

    @Override
@@ -238,7 +252,7 @@ public class DefaultClientService implements ClientService {
        }
    }

-    private CreateOrUpdateResult createOrUpdate(RealmModel realm, String clientId, BaseClientRepresentation client, CreateOrUpdateStrategy strategy) throws ServiceException {
+    private CreateOrUpdateResult createOrUpdate(RealmModel realm, String clientId, BaseClientRepresentation client, CreateOrUpdateStrategy strategy, boolean patchExplicitNullSecret) throws ServiceException {
        validateUnknownFields(client);
        ClientModel model = null;
        if (!strategy.equals(CreateOrUpdateStrategy.ONLY_CREATE)) {
@@ -255,13 +269,12 @@ public class DefaultClientService implements ClientService {
                    case PUT, PATCH -> {
                        // Check permissions, execute validations and trigger client policies
                        permissions.clients().requireConfigure(model);
+                        // Must run before bean validation: PutClient requires a non-blank secret for client-secret methods
+                        generateClientSecretIfNeeded(client, model, strategy, patchExplicitNullSecret);
                        validator.validate(client, strategy.getValidationGroup(), Default.class);
                        var proposedRepresentation = getProposedOldRepresentation(realm, client, mapper);
                        session.clientPolicy().triggerOnEvent(new AdminClientUpdateContext(proposedRepresentation, model, permissions.adminAuth()));

-                        // Generate random secret if applicable
-                        generateClientSecretIfNeeded(client, model);
-
                        // Update model
                        mapper.toModel(client, model);

@@ -286,7 +299,7 @@ public class DefaultClientService implements ClientService {
                model.setProtocol(client.getProtocol());

                // Generate random secret if applicable
-                generateClientSecretIfNeeded(client, model);
+                generateClientSecretIfNeeded(client, model, strategy, patchExplicitNullSecret);
                mapper.toModel(client, model);

                // Validate the fully populated model
@@ -306,7 +319,7 @@ public class DefaultClientService implements ClientService {

        // OIDC specific
        if (client instanceof OIDCClientRepresentation oidcClient) {
-            handleServiceAccount(clientRoles, rolesService.resource(realm), model, oidcClient);
+            handleServiceAccount(model, oidcClient);
        }

        fireAdminEvent(alreadyExists ? OperationType.UPDATE : OperationType.CREATE, mapper.fromModel(model));
@@ -351,11 +364,21 @@ public class DefaultClientService implements ClientService {
    }

    // TODO we should find a way on how to evoke it on the mapper level?
-    private void generateClientSecretIfNeeded(BaseClientRepresentation client, ClientModel model) {
+    private void generateClientSecretIfNeeded(BaseClientRepresentation client, ClientModel model, CreateOrUpdateStrategy strategy, boolean patchExplicitNullSecret) {
        if (client.getProtocol().equals(OIDCClientRepresentation.PROTOCOL)) {
            var auth = ((OIDCClientRepresentation) client).getAuth();
            if (auth != null && isClientSecret(auth.getMethod()) && isBlank(auth.getSecret())) {
-                auth.setSecret(KeycloakModelUtils.generateSecret(model));
+                if (strategy == CreateOrUpdateStrategy.PATCH && patchExplicitNullSecret) {
+                    auth.setSecret(KeycloakModelUtils.generateSecret(model));
+                } else {
+                    // On PUT the client often omits the secret; reuse the persisted secret before bean validation (PutClient).
+                    // On PATCH without explicit JSON null for secret, keep the same semantics (do not rotate).
+                    if (!isBlank(model.getSecret())) {
+                        auth.setSecret(model.getSecret());
+                    } else {
+                        auth.setSecret(KeycloakModelUtils.generateSecret(model));
+                    }
+                }
            }
        }
    }
@@ -402,9 +425,10 @@ public class DefaultClientService implements ClientService {
    /**
     * Declaratively manage service account - enables/disables it and ensures it has exactly the roles specified (realm and client roles)
     * <p>
-     * Reuses API v1 logic
+     * Applies mappings on the {@link UserModel} with the same permission checks as the Admin REST role-mapping resources, but without
+     * routing through nested JAX-RS resources (which are not suited for in-process service calls).
     */
-    protected void handleServiceAccount(RoleContainerResource clientRoleResource, RoleContainerResource realmRoleResource, ClientModel model, OIDCClientRepresentation rep) {
+    protected void handleServiceAccount(ClientModel model, OIDCClientRepresentation rep) {
        boolean serviceAccountEnabled = rep.getLoginFlows().contains(OIDCClientRepresentation.Flow.SERVICE_ACCOUNT);

        ClientManager.updateClientServiceAccount(session, model, serviceAccountEnabled);
@@ -413,49 +437,48 @@ public class DefaultClientService implements ClientService {
            return;
        }

-        var serviceAccountUser = new ClientManager(new RealmManager(session)).getServiceAccountUser(model)
+        UserModel serviceAccountUser = new ClientManager(new RealmManager(session)).getServiceAccountUser(model)
                .orElseThrow(() -> new ServiceException("Cannot find service account user", Response.Status.BAD_REQUEST));
-        var serviceAccountRoleResource = realmResource.users().user(serviceAccountUser.getId()).getRoleMappings();

+        RealmModel realm = model.getRealm();
        Set<String> desiredRoleNames = Optional.ofNullable(rep.getServiceAccountRoles()).orElse(Collections.emptySet());
        Set<RoleModel> currentRoles = serviceAccountUser.getRoleMappingsStream().collect(Collectors.toSet());
        Set<String> currentRoleNames = currentRoles.stream().map(RoleModel::getName).collect(Collectors.toSet());

-        // Get missing roles (in desired but not in current)
-        List<RoleRepresentation> missingRoles = desiredRoleNames.stream()
-                .filter(roleName -> !currentRoleNames.contains(roleName))
-                .map(roleName -> {
-                    try {
-                        return clientRoleResource.getRole(roleName); // client role
-                    } catch (NotFoundException e) {
-                        try {
-                            return realmRoleResource.getRole(roleName); // realm role
-                        } catch (NotFoundException e2) {
-                            throw new ServiceException("Cannot assign role to the service account (field 'serviceAccount.roles') as it does not exist", Response.Status.BAD_REQUEST);
-                        }
-                    }
-                })
-                .toList();
-
-        // Add missing roles (in desired but not in current)
-        if (!missingRoles.isEmpty()) {
-            serviceAccountRoleResource.addRealmRoleMappings(missingRoles);
+        // serviceAccountRoles are plain names; client roles on this client are resolved before realm roles (name collisions favor the client).
+        List<RoleModel> rolesToAdd = new ArrayList<>();
+        for (String roleName : desiredRoleNames) {
+            if (currentRoleNames.contains(roleName)) {
+                continue;
+            }
+            RoleModel clientRole = model.getRole(roleName);
+            RoleModel resolved = clientRole != null ? clientRole : realm.getRole(roleName);
+            if (resolved == null) {
+                throw new ServiceException("Cannot assign role to the service account (field 'serviceAccount.roles') as it does not exist", Response.Status.BAD_REQUEST);
+            }
+            rolesToAdd.add(resolved);
        }

-        // Get extra roles (in current but not in desired)
-        List<RoleRepresentation> extraRoles = currentRoles.stream()
-                .filter(role -> !desiredRoleNames.contains(role.getName()))
-                .map(ModelToRepresentation::toRepresentation)
-                .toList();
-
-        // Remove extra roles (in current but not in desired)
-        if (!extraRoles.isEmpty()) {
-            try {
-                serviceAccountRoleResource.deleteRealmRoleMappings(extraRoles);
-            } catch (NotFoundException e) {
-                throw new ServiceException("Cannot unassign role from the service account (field 'serviceAccount.roles') as it does not exist", Response.Status.BAD_REQUEST);
+        List<RoleModel> rolesToRemove = new ArrayList<>();
+        for (RoleModel role : currentRoles) {
+            if (!desiredRoleNames.contains(role.getName())) {
+                rolesToRemove.add(role);
            }
        }
+
+        if (rolesToAdd.isEmpty() && rolesToRemove.isEmpty()) {
+            return;
+        }
+
+        permissions.users().requireMapRoles(serviceAccountUser);
+        for (RoleModel role : rolesToAdd) {
+            permissions.roles().requireMapRole(role);
+            serviceAccountUser.grantRole(role);
+        }
+        for (RoleModel role : rolesToRemove) {
+            permissions.roles().requireMapRole(role);
+            serviceAccountUser.deleteRoleMapping(role);
+        }
    }

    protected void validateUnknownFields(BaseClientRepresentation rep) {
@@ -19,7 +19,9 @@ package org.keycloak.tests.admin.client.v2;

 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.util.List;
+import java.util.Locale;
 import java.util.Set;
 import java.util.stream.Stream;

@@ -531,6 +533,43 @@ public class ClientApiV2Test extends AbstractClientApiV2Test{
        }
    }

+    @Test
+    public void declarativeServiceAccountClientRoleManagement() {
+        String defaultRealmRoles = "default-roles-%s".formatted(testRealm.getName().toLowerCase());
+        OIDCClientRepresentation rep = new OIDCClientRepresentation();
+        rep.setClientId("sa-client-role-test");
+        rep.setEnabled(true);
+        rep.setRoles(Set.of("my-client-role"));
+        rep.setLoginFlows(Set.of(OIDCClientRepresentation.Flow.SERVICE_ACCOUNT));
+        rep.setServiceAccountRoles(Set.of(defaultRealmRoles, "offline_access"));
+
+        OIDCClientRepresentation.Auth auth = new OIDCClientRepresentation.Auth();
+        auth.setMethod(ClientIdAndSecretAuthenticator.PROVIDER_ID);
+        rep.setAuth(auth);
+
+        try (var response = getClientsApi().createClient(rep)) {
+            assertEquals(201, response.getStatus());
+            OIDCClientRepresentation created = response.readEntity(OIDCClientRepresentation.class);
+            assertThat(created.getRoles(), is(Set.of("my-client-role")));
+            assertThat(created.getServiceAccountRoles(), is(Set.of(defaultRealmRoles, "offline_access")));
+        }
+
+        rep.setServiceAccountRoles(Set.of(defaultRealmRoles, "offline_access", "my-client-role"));
+        try (var response = getClientsApi().client("sa-client-role-test").createOrUpdateClient(rep)) {
+            assertEquals(200, response.getStatus());
+            OIDCClientRepresentation updated = response.readEntity(OIDCClientRepresentation.class);
+            assertThat(updated.getServiceAccountRoles(), is(Set.of(defaultRealmRoles, "offline_access", "my-client-role")));
+        }
+
+        rep.setServiceAccountRoles(Set.of(defaultRealmRoles, "offline_access"));
+        try (var response = getClientsApi().client("sa-client-role-test").createOrUpdateClient(rep)) {
+            assertEquals(200, response.getStatus());
+            OIDCClientRepresentation updated = response.readEntity(OIDCClientRepresentation.class);
+            assertThat(updated.getServiceAccountRoles(), is(Set.of(defaultRealmRoles, "offline_access")));
+            assertThat(updated.getRoles(), is(Set.of("my-client-role")));
+        }
+    }
+
    @Test
    public void versionedClientsApi() throws Exception {
        final var ADMIN_API_URL = "http://localhost:8080/admin/api/master";
@@ -763,8 +802,9 @@ public class ClientApiV2Test extends AbstractClientApiV2Test{

        OIDCClientRepresentation.Auth authWithoutSecret = new OIDCClientRepresentation.Auth();
        authWithoutSecret.setMethod(authenticationMethod);
-        authWithoutSecret.setAdditionalField("secret", null);
-        OIDCClientRepresentation.Auth patchedAuth = getResultingAuthConfigPatch(authWithoutSecret, clientId);
+        authWithoutSecret.setSecret(null);
+        OIDCClientRepresentation.Auth patchedAuth = getResultingAuthConfigPatchRawAuth(clientId,
+                "{\"method\":\"" + authenticationMethod + "\",\"secret\":null}");
        assertThat(patchedAuth, notNullValue());
        String newlyGeneratedSecret = patchedAuth.getSecret();
        assertThat(newlyGeneratedSecret, not(is(createdAuth.getSecret())));
@@ -786,9 +826,7 @@ public class ClientApiV2Test extends AbstractClientApiV2Test{
        assertThat(createdAuth, notNullValue());
        assertThat(createdAuth.getSecret(), is(auth.getSecret()));

-        OIDCClientRepresentation.Auth authWithoutSecret = new OIDCClientRepresentation.Auth();
-        authWithoutSecret.setAdditionalField("secret", null);
-        OIDCClientRepresentation.Auth patchedAuth = getResultingAuthConfigPatch(authWithoutSecret, clientId);
+        OIDCClientRepresentation.Auth patchedAuth = getResultingAuthConfigPatchRawAuth(clientId, "{\"secret\":null}");
        assertThat(patchedAuth, notNullValue());
        String newlyGeneratedSecret = patchedAuth.getSecret();
        assertThat(newlyGeneratedSecret, not(is(createdAuth.getSecret())));
@@ -858,7 +896,7 @@ public class ClientApiV2Test extends AbstractClientApiV2Test{

    @ParameterizedTest
    @ValueSource(strings = { ClientIdAndSecretAuthenticator.PROVIDER_ID, JWTClientSecretAuthenticator.PROVIDER_ID })
-    void expectValidationFailureForUpdatePutWithoutSecret(String authenticationMethod) throws IOException {
+    void putUpdateWithNullSecretReusesPersistedSecret(String authenticationMethod) throws IOException {
        String clientId = authenticationMethod + "-validation-update-put";
        OIDCClientRepresentation.Auth auth = new OIDCClientRepresentation.Auth();
        auth.setMethod(authenticationMethod);
@@ -869,8 +907,9 @@ public class ClientApiV2Test extends AbstractClientApiV2Test{
        assertThat(createdAuth.getSecret(), is(auth.getSecret()));

        auth.setSecret(null);
-        var assertionError = assertThrows(AssertionError.class, () -> getResultingAuthConfigPut(auth, clientId));
-        assertThat(assertionError.getMessage(), Matchers.containsString("was <400>"));
+        OIDCClientRepresentation.Auth putAuth = getResultingAuthConfigPut(auth, clientId);
+        assertThat(putAuth, notNullValue());
+        assertThat(putAuth.getSecret(), is(createdAuth.getSecret()));
    }

    @ParameterizedTest
@@ -926,6 +965,23 @@ public class ClientApiV2Test extends AbstractClientApiV2Test{
        return assertClientEnabledIdDescriptionAndAuth(rep, createdClient);
    }

+    /**
+     * Applies a JSON merge patch with a raw {@code auth} object fragment (must include {@code "secret":null} when rotating secrets).
+     */
+    private OIDCClientRepresentation.Auth getResultingAuthConfigPatchRawAuth(String clientId, String authObjectJson) throws JsonProcessingException {
+        OIDCClientRepresentation rep = new OIDCClientRepresentation();
+        rep.setEnabled(true);
+        rep.setClientId(clientId);
+        rep.setDescription("I'm OIDC Client");
+        rep.setAuth(mapper.readValue(authObjectJson, OIDCClientRepresentation.Auth.class));
+        String body = String.format(Locale.ROOT,
+                "{\"enabled\":true,\"clientId\":\"%s\",\"description\":\"I'm OIDC Client\",\"auth\":%s}",
+                clientId, authObjectJson);
+        OIDCClientRepresentation createdClient = (OIDCClientRepresentation) getClientsApi().client(clientId).patchClient(
+                new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8)));
+        return assertClientEnabledIdDescriptionAndAuth(rep, createdClient);
+    }
+
    private OIDCClientRepresentation getResultingClientRep(OIDCClientRepresentation.Auth auth, String clientId, String... additionalFields) throws IllegalArgumentException {
        OIDCClientRepresentation rep = new OIDCClientRepresentation();
        rep.setEnabled(true);
@@ -228,6 +228,33 @@ public class OIDCClientModelMapperTest {
        }
    }

+    @TestOnServer
+    public void fromModel_mapsServiceAccountClientRoles(KeycloakSession session) {
+        RealmModel realm = session.realms().getRealmByName("master");
+        session.getContext().setRealm(realm);
+
+        ClientModel clientModel = realm.addClient("test-sa-client-role-mapper");
+        try {
+            setupBasicClientModel(clientModel);
+            clientModel.setServiceAccountsEnabled(true);
+            RoleModel clientRole = clientModel.addRole("mapper-client-sa-role");
+
+            String username = "service-account-" + clientModel.getClientId();
+            UserModel serviceAccount = session.users().addUser(realm, username);
+            serviceAccount.setEnabled(true);
+            serviceAccount.setServiceAccountClientLink(clientModel.getId());
+
+            serviceAccount.grantRole(clientRole);
+
+            OIDCClientModelMapper mapper = getModelMapper(session);
+            OIDCClientRepresentation rep = (OIDCClientRepresentation) mapper.fromModel(clientModel);
+
+            assertThat(rep.getServiceAccountRoles(), hasItems("mapper-client-sa-role"));
+        } finally {
+            realm.removeClient(clientModel.getId());
+        }
+    }
+
    @TestOnServer
    public void fromModel_emptyServiceAccountRolesWhenServiceAccountDisabled(KeycloakSession session) {
        RealmModel realm = session.realms().getRealmByName("master");