Merge pull request #4206 from pedroigor/KEYCLOAK-4983

[KEYCLOAK-4983] - Authz settings export of role base policy generates json where are just role-names

authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/client/ClientPolicyProviderFactory.java

@@ -3,10 +3,12 @@ package org.keycloak.authorization.policy.provider.client;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.keycloak.Config;
 import org.keycloak.authorization.AuthorizationProvider;
@@ -71,6 +73,21 @@ public class ClientPolicyProviderFactory implements PolicyProviderFactory<Client
         updateClients(policy, new HashSet<>(Arrays.asList(getClients(policy))), authorization);
     }
 
+    @Override
+    public void onExport(Policy policy, PolicyRepresentation representation, AuthorizationProvider authorizationProvider) {
+        ClientPolicyRepresentation userRep = toRepresentation(policy, new ClientPolicyRepresentation());
+        Map<String, String> config = new HashMap<>();
+
+        try {
+            RealmModel realm = authorizationProvider.getRealm();
+            config.put("clients", JsonSerialization.writeValueAsString(userRep.getClients().stream().map(id -> realm.getClientById(id).getClientId()).collect(Collectors.toList())));
+        } catch (IOException cause) {
+            throw new RuntimeException("Failed to export user policy [" + policy.getName() + "]", cause);
+        }
+
+        representation.setConfig(config);
+    }
+
     @Override
     public PolicyProvider create(KeycloakSession session) {
         return null;
@@ -129,7 +146,7 @@ public class ClientPolicyProviderFactory implements PolicyProviderFactory<Client
     }
 
     private void updateClients(Policy policy, Set<String> clients, AuthorizationProvider authorization) {
-        RealmModel realm = authorization.getKeycloakSession().getContext().getRealm();
+        RealmModel realm = authorization.getRealm();
 
         if (clients == null || clients.isEmpty()) {
             throw new RuntimeException("No client provided.");

authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/role/RolePolicyProviderFactory.java

@@ -108,6 +108,30 @@ public class RolePolicyProviderFactory implements PolicyProviderFactory<RolePoli
         }
     }
 
+    @Override
+    public void onExport(Policy policy, PolicyRepresentation representation, AuthorizationProvider authorizationProvider) {
+        Map<String, String> config = new HashMap<>();
+        Set<RolePolicyRepresentation.RoleDefinition> roles = toRepresentation(policy, new RolePolicyRepresentation()).getRoles();
+
+        for (RolePolicyRepresentation.RoleDefinition roleDefinition : roles) {
+            RoleModel role = authorizationProvider.getRealm().getRoleById(roleDefinition.getId());
+
+            if (role.isClientRole()) {
+                roleDefinition.setId(ClientModel.class.cast(role.getContainer()).getClientId() + "/" + role.getName());
+            } else {
+                roleDefinition.setId(role.getName());
+            }
+        }
+
+        try {
+            config.put("roles", JsonSerialization.writeValueAsString(roles));
+        } catch (IOException cause) {
+            throw new RuntimeException("Failed to export role policy [" + policy.getName() + "]", cause);
+        }
+
+        representation.setConfig(config);
+    }
+
     private void updateRoles(Policy policy, RolePolicyRepresentation representation, AuthorizationProvider authorization) {
         updateRoles(policy, authorization, representation.getRoles());
     }

authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/user/UserPolicyProviderFactory.java

@@ -20,11 +20,13 @@ package org.keycloak.authorization.policy.provider.user;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 
 import org.keycloak.Config;
 import org.keycloak.authorization.AuthorizationProvider;
@@ -106,6 +108,23 @@ public class UserPolicyProviderFactory implements PolicyProviderFactory<UserPoli
         }
     }
 
+    @Override
+    public void onExport(Policy policy, PolicyRepresentation representation, AuthorizationProvider authorizationProvider) {
+        UserPolicyRepresentation userRep = toRepresentation(policy, new UserPolicyRepresentation());
+        Map<String, String> config = new HashMap<>();
+
+        try {
+            UserProvider userProvider = authorizationProvider.getKeycloakSession().users();
+            RealmModel realm = authorizationProvider.getRealm();
+
+            config.put("users", JsonSerialization.writeValueAsString(userRep.getUsers().stream().map(id -> userProvider.getUserById(id, realm).getUsername()).collect(Collectors.toList())));
+        } catch (IOException cause) {
+            throw new RuntimeException("Failed to export user policy [" + policy.getName() + "]", cause);
+        }
+
+        representation.setConfig(config);
+    }
+
     private void updateUsers(Policy policy, UserPolicyRepresentation representation, AuthorizationProvider authorization) {
         updateUsers(policy, authorization, representation.getUsers());
     }

server-spi-private/src/main/java/org/keycloak/authorization/policy/provider/PolicyProviderFactory.java

@@ -60,7 +60,11 @@ public interface PolicyProviderFactory<R extends AbstractPolicyRepresentation> e
 
     }
 
+    default void onExport(Policy policy, PolicyRepresentation representation, AuthorizationProvider authorizationProvider) {
+        representation.setConfig(policy.getConfig());
+    }
+
     default PolicyProviderAdminService getAdminResource(ResourceServer resourceServer, AuthorizationProvider authorization) {
         return null;
     }
-}
+}

server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java

@@ -799,6 +799,10 @@ public class ModelToRepresentation {
     }
 
     public static <R extends AbstractPolicyRepresentation> R toRepresentation(Policy policy, Class<R> representationType, AuthorizationProvider authorization) {
+        return toRepresentation(policy, representationType, authorization, false);
+    }
+
+    public static <R extends AbstractPolicyRepresentation> R toRepresentation(Policy policy, Class<R> representationType, AuthorizationProvider authorization, boolean export) {
         R representation;
 
         try {
@@ -817,7 +821,11 @@ public class ModelToRepresentation {
         representation.setLogic(policy.getLogic());
 
         if (representation instanceof PolicyRepresentation) {
-            PolicyRepresentation.class.cast(representation).setConfig(policy.getConfig());
+            if (providerFactory != null && export) {
+                providerFactory.onExport(policy, PolicyRepresentation.class.cast(representation), authorization);
+            } else {
+                PolicyRepresentation.class.cast(representation).setConfig(policy.getConfig());
+            }
         } else {
             representation = (R) providerFactory.toRepresentation(policy, representation);
         }

services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java

@@ -358,11 +358,8 @@ public class ExportUtils {
     }
 
     private static PolicyRepresentation createPolicyRepresentation(AuthorizationProvider authorizationProvider, Policy policy) {
-        KeycloakSession session = authorizationProvider.getKeycloakSession();
-        RealmModel realm = authorizationProvider.getRealm();
-
         try {
-            PolicyRepresentation rep = toRepresentation(policy, PolicyRepresentation.class, authorizationProvider);
+            PolicyRepresentation rep = toRepresentation(policy, PolicyRepresentation.class, authorizationProvider, true);
 
             rep.setId(null);
 
@@ -370,24 +367,6 @@ public class ExportUtils {
 
             rep.setConfig(config);
 
-            String roles = config.get("roles");
-
-            if (roles != null && !roles.isEmpty()) {
-                List<Map> rolesMap = JsonSerialization.readValue(roles, List.class);
-                config.put("roles", JsonSerialization.writeValueAsString(rolesMap.stream().map(roleMap -> {
-                    roleMap.put("id", realm.getRoleById(roleMap.get("id").toString()).getName());
-                    return roleMap;
-                }).collect(Collectors.toList())));
-            }
-
-            String users = config.get("users");
-
-            if (users != null && !users.isEmpty()) {
-                UserProvider userManager = session.users();
-                List<String> userIds = JsonSerialization.readValue(users, List.class);
-                config.put("users", JsonSerialization.writeValueAsString(userIds.stream().map(userId -> userManager.getUserById(userId, realm).getUsername()).collect(Collectors.toList())));
-            }
-
             Set<Scope> scopes = policy.getScopes();
 
             if (!scopes.isEmpty()) {

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/exportimport/ExportImportUtil.java

@@ -627,10 +627,12 @@ public class ExportImportUtil {
         assertPredicate(scopes, scopePredicates);
 
         List<PolicyRepresentation> policies = authzResource.policies().policies();
-        Assert.assertEquals(11, policies.size());
+        Assert.assertEquals(13, policies.size());
         List<Predicate<PolicyRepresentation>> policyPredicates = new ArrayList<>();
         policyPredicates.add(policyRepresentation -> "Any Admin Policy".equals(policyRepresentation.getName()));
         policyPredicates.add(policyRepresentation -> "Any User Policy".equals(policyRepresentation.getName()));
+        policyPredicates.add(representation -> "Client and Realm Role Policy".equals(representation.getName()));
+        policyPredicates.add(representation -> "Client Test Policy".equals(representation.getName()));
         policyPredicates.add(policyRepresentation -> "Only Premium User Policy".equals(policyRepresentation.getName()));
         policyPredicates.add(policyRepresentation -> "wburke policy".equals(policyRepresentation.getName()));
         policyPredicates.add(policyRepresentation -> "All Users Policy".equals(policyRepresentation.getName()));

testsuite/integration-arquillian/tests/base/src/test/resources/model/testrealm.json

@@ -282,6 +282,22 @@
                             "roles": "[{\"id\":\"user\"}]"
                         }
                     },
+                    {
+                        "name": "Client and Realm Role Policy",
+                        "type": "role",
+                        "config": {
+                            "roles": "[{\"id\":\"realm-management/impersonation\",\"required\":false},{\"id\":\"realm-management/manage-authorization\",\"required\":true},{\"id\":\"user\",\"required\":false}]"
+                        }
+                    },
+                    {
+                        "name": "Client Test Policy",
+                        "type": "client",
+                        "logic": "POSITIVE",
+                        "decisionStrategy": "UNANIMOUS",
+                        "config": {
+                            "clients": "[\"broker\",\"admin-cli\"]"
+                        }
+                    },
                     {
                         "name": "Only Premium User Policy",
                         "description": "Defines that only premium users can do something",

testsuite/integration-arquillian/tests/other/console/src/main/java/org/keycloak/testsuite/console/page/clients/authorization/resource/ResourcesTable.java

@@ -66,14 +66,18 @@ public class ResourcesTable extends DataTable {
     public ResourceRepresentation toRepresentation(WebElement row) {
         ResourceRepresentation representation = null;
         List<WebElement> tds = row.findElements(tagName("td"));
-        if (!(tds.isEmpty() || tds.get(0).getText().isEmpty())) {
-            representation = new ResourceRepresentation();
-            representation.setName(tds.get(0).getText());
-            representation.setType(tds.get(1).getText());
-            representation.setUri(tds.get(2).getText());
-            ResourceOwnerRepresentation owner = new ResourceOwnerRepresentation();
-            owner.setName(tds.get(3).getText());
-            representation.setOwner(owner);
+        try {
+            if (!(tds.isEmpty() || tds.get(0).getText().isEmpty())) {
+                representation = new ResourceRepresentation();
+                representation.setName(tds.get(0).getText());
+                representation.setType(tds.get(1).getText());
+                representation.setUri(tds.get(2).getText());
+                ResourceOwnerRepresentation owner = new ResourceOwnerRepresentation();
+                owner.setName(tds.get(3).getText());
+                representation.setOwner(owner);
+            }
+        } catch (IndexOutOfBoundsException cause) {
+            // is empty
         }
         return representation;
     }