KEYCLOAK-4035 Composite roles need to be expanded in SAML attribute mapper

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

@@ -20,7 +20,11 @@ package org.keycloak.models.utils;
 import org.keycloak.models.GroupModel;
 import org.keycloak.models.RoleModel;
 
+import java.util.ArrayDeque;
+import java.util.Deque;
+import java.util.HashSet;
 import java.util.Set;
+import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
 /**
@@ -98,4 +102,33 @@ public class RoleUtils {
                 .anyMatch(group -> hasRoleFromGroup(group, targetRole, checkParentGroup));
     }
 
+    /**
+     * Recursively expands composite roles into their composite.
+     * @param role
+     * @return Stream of containing all of the composite roles and their components.
+     */
+    public static Stream<RoleModel> expandCompositeRolesStream(RoleModel role) {
+        Stream.Builder<RoleModel> sb = Stream.builder();
+        Set<RoleModel> roles = new HashSet<>();
+
+        Deque<RoleModel> stack = new ArrayDeque<>();
+        stack.add(role);
+
+        while (! stack.isEmpty()) {
+            RoleModel current = stack.pop();
+            sb.add(current);
+
+            if (current.isComposite()) {
+                current.getComposites().stream()
+                  .filter(r -> ! roles.contains(r))
+                  .forEach(r -> {
+                    roles.add(r);
+                    stack.add(r);
+                  });
+            }
+        }
+
+        return sb.build();
+    }
+
 }

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

@@ -22,10 +22,9 @@ import org.keycloak.models.ProtocolMapperModel;
 import org.keycloak.models.RoleModel;
 import org.keycloak.models.UserModel;
 import org.keycloak.models.UserSessionModel;
+import org.keycloak.models.utils.RoleUtils;
 import org.keycloak.representations.IDToken;
 
-import java.util.ArrayDeque;
-import java.util.Deque;
 import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -54,7 +53,7 @@ abstract class AbstractUserRoleMappingMapper extends AbstractOIDCProtocolMapper
           user.getGroups().stream()
             .flatMap(g -> groupAndItsParentsStream(g))
             .flatMap(g -> g.getRoleMappings().stream()))
-          .flatMap(role -> expandCompositeRolesStream(role));
+          .flatMap(RoleUtils::expandCompositeRolesStream);
     }
 
     /**
@@ -71,29 +70,6 @@ abstract class AbstractUserRoleMappingMapper extends AbstractOIDCProtocolMapper
         return sb.build();
     }
 
-    /**
-     * Recursively expands composite roles into their composite.
-     * @param role
-     * @return Stream of containing all of the composite roles and their components.
-     */
-    private static Stream<RoleModel> expandCompositeRolesStream(RoleModel role) {
-        Stream.Builder<RoleModel> sb = Stream.builder();
-
-        Deque<RoleModel> stack = new ArrayDeque<>();
-        stack.add(role);
-
-        while (! stack.isEmpty()) {
-            RoleModel current = stack.pop();
-            sb.add(current);
-
-            if (current.isComposite()) {
-                stack.addAll(current.getComposites());
-            }
-        }
-
-        return sb.build();
-    }
-
     /**
      * Retrieves all roles of the current user based on direct roles set to the user, its groups and their parent groups.
      * Then it recursively expands all composite roles, and restricts according to the given predicate {@code restriction}.

services/src/main/java/org/keycloak/protocol/saml/mappers/HardcodedRole.java

@@ -35,12 +35,13 @@ import java.util.Map;
 public class HardcodedRole extends AbstractSAMLProtocolMapper {
     public static final String PROVIDER_ID = "saml-hardcode-role-mapper";
     public static final String ATTRIBUTE_VALUE = "attribute.value";
-    private static final List<ProviderConfigProperty> configProperties = new ArrayList<ProviderConfigProperty>();
+    private static final List<ProviderConfigProperty> configProperties = new ArrayList<>();
+    public static final String ROLE_ATTRIBUTE = "role";
 
     static {
         ProviderConfigProperty property;
         property = new ProviderConfigProperty();
-        property.setName("role");
+        property.setName(ROLE_ATTRIBUTE);
         property.setLabel("Role");
         property.setHelpText("Arbitrary role name you want to hardcode.  This role does not have to exist in current realm and can be just any string you need");
         property.setType(ProviderConfigProperty.ROLE_TYPE);
@@ -79,8 +80,8 @@ public class HardcodedRole extends AbstractSAMLProtocolMapper {
         mapper.setName(name);
         mapper.setProtocolMapper(mapperId);
         mapper.setProtocol(SamlProtocol.LOGIN_PROTOCOL);
-       Map<String, String> config = new HashMap<String, String>();
+        Map<String, String> config = new HashMap<>();
-        config.put("role", role);
+        config.put(ROLE_ATTRIBUTE, role);
         mapper.setConfig(config);
         return mapper;
 

services/src/main/java/org/keycloak/protocol/saml/mappers/RoleListMapper.java

@@ -23,8 +23,9 @@ import org.keycloak.models.ClientSessionModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.KeycloakSessionFactory;
 import org.keycloak.models.ProtocolMapperModel;
-import org.keycloak.models.RoleModel;
+import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserSessionModel;
+import org.keycloak.models.utils.RoleUtils;
 import org.keycloak.protocol.ProtocolMapper;
 import org.keycloak.protocol.saml.SamlProtocol;
 import org.keycloak.provider.ProviderConfigProperty;
@@ -35,7 +36,9 @@ import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
@@ -45,7 +48,7 @@ public class RoleListMapper extends AbstractSAMLProtocolMapper implements SAMLRo
     public static final String PROVIDER_ID = "saml-role-list-mapper";
     public static final String SINGLE_ROLE_ATTRIBUTE = "single";
 
-    private static final List<ProviderConfigProperty> configProperties = new ArrayList<ProviderConfigProperty>();
+    private static final List<ProviderConfigProperty> configProperties = new ArrayList<>();
 
     static {
         ProviderConfigProperty property;
@@ -120,11 +123,13 @@ public class RoleListMapper extends AbstractSAMLProtocolMapper implements SAMLRo
 
             ProtocolMapper mapper = (ProtocolMapper)sessionFactory.getProviderFactory(ProtocolMapper.class, mapping.getProtocolMapper());
             if (mapper == null) continue;
+
             if (mapper instanceof SAMLRoleNameMapper) {
                 roleNameMappers.add(new SamlProtocol.ProtocolMapperProcessor<>((SAMLRoleNameMapper) mapper,mapping));
             }
+
             if (mapper instanceof HardcodedRole) {
-                AttributeType attributeType = null;
+                AttributeType attributeType;
                 if (singleAttribute) {
                     if (singleAttributeType == null) {
                         singleAttributeType = AttributeStatementHelper.createAttributeType(mappingModel);
@@ -135,14 +140,26 @@ public class RoleListMapper extends AbstractSAMLProtocolMapper implements SAMLRo
                     attributeType = AttributeStatementHelper.createAttributeType(mappingModel);
                     roleAttributeStatement.addAttribute(new AttributeStatementType.ASTChoiceType(attributeType));
                 }
-                attributeType.addAttributeValue(mapping.getConfig().get("role"));
+
+                attributeType.addAttributeValue(mapping.getConfig().get(HardcodedRole.ROLE_ATTRIBUTE));
             }
         }
 
-        for (String roleId : clientSession.getRoles()) {
+        RealmModel realm = clientSession.getRealm();
+        List<String> allRoleNames = clientSession.getRoles().stream()
           // todo need a role mapping
-            RoleModel roleModel = clientSession.getRealm().getRoleById(roleId);
+          .map(realm::getRoleById)
-            AttributeType attributeType = null;
+          .filter(Objects::nonNull)
+          .flatMap(RoleUtils::expandCompositeRolesStream)
+          .map(roleModel -> roleNameMappers.stream()
+            .map(entry -> entry.mapper.mapName(entry.model, roleModel))
+            .filter(Objects::nonNull)
+            .findFirst()
+            .orElse(roleModel.getName())
+          ).collect(Collectors.toList());
+
+        for (String roleName : allRoleNames) {
+            AttributeType attributeType;
             if (singleAttribute) {
                 if (singleAttributeType == null) {
                     singleAttributeType = AttributeStatementHelper.createAttributeType(mappingModel);
@@ -153,14 +170,7 @@ public class RoleListMapper extends AbstractSAMLProtocolMapper implements SAMLRo
                 attributeType = AttributeStatementHelper.createAttributeType(mappingModel);
                 roleAttributeStatement.addAttribute(new AttributeStatementType.ASTChoiceType(attributeType));
             }
-            String roleName = roleModel.getName();
+
-            for (SamlProtocol.ProtocolMapperProcessor<SAMLRoleNameMapper> entry : roleNameMappers) {
-                String newName = entry.mapper.mapName(entry.model, roleModel);
-                if (newName != null) {
-                    roleName = newName;
-                    break;
-                }
-            }
             attributeType.addAttributeValue(roleName);
         }
 
@@ -172,7 +182,7 @@ public class RoleListMapper extends AbstractSAMLProtocolMapper implements SAMLRo
         mapper.setProtocolMapper(PROVIDER_ID);
         mapper.setProtocol(SamlProtocol.LOGIN_PROTOCOL);
         mapper.setConsentRequired(false);
-        Map<String, String> config = new HashMap<String, String>();
+        Map<String, String> config = new HashMap<>();
         config.put(AttributeStatementHelper.SAML_ATTRIBUTE_NAME, samlAttributeName);
         if (friendlyName != null) {
             config.put(AttributeStatementHelper.FRIENDLY_NAME, friendlyName);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java

@@ -22,6 +22,7 @@ import org.jboss.arquillian.graphene.page.Page;
 import org.jboss.shrinkwrap.api.spec.WebArchive;
 import org.junit.Assert;
 import org.junit.Test;
+
 import org.keycloak.admin.client.resource.ClientResource;
 import org.keycloak.admin.client.resource.ProtocolMappersResource;
 import org.keycloak.admin.client.resource.RoleScopeResource;
@@ -71,6 +72,7 @@ import org.keycloak.testsuite.auth.page.login.SAMLIDPInitiatedLogin;
 import org.keycloak.testsuite.page.AbstractPage;
 import org.keycloak.testsuite.util.IOUtil;
 import org.keycloak.testsuite.util.UserBuilder;
+
 import org.openqa.selenium.By;
 import org.w3c.dom.Document;
 import org.xml.sax.SAXException;
@@ -104,6 +106,7 @@ import static org.junit.Assert.*;
 import static org.keycloak.representations.idm.CredentialRepresentation.PASSWORD;
 import static org.keycloak.testsuite.AbstractAuthTest.createUserRepresentation;
 import static org.keycloak.testsuite.admin.ApiUtil.createUserAndResetPasswordWithAdminClient;
+import static org.keycloak.testsuite.admin.Users.setPasswordFor;
 import static org.keycloak.testsuite.auth.page.AuthRealm.SAMLSERVLETDEMO;
 import static org.keycloak.testsuite.util.IOUtil.loadRealm;
 import static org.keycloak.testsuite.util.IOUtil.loadXML;
@@ -529,6 +532,14 @@ public abstract class AbstractSAMLServletsAdapterTest extends AbstractServletsAd
         testSuccessfulAndUnauthorizedLogin(salesMetadataServletPage, testRealmSAMLPostLoginPage);
     }
 
+    @Test
+    public void salesPostTestCompositeRoleForUser() {
+        UserRepresentation topGroupUser = createUserRepresentation("topGroupUser", "top@redhat.com", "", "", true);
+        setPasswordFor(topGroupUser, PASSWORD);
+
+        assertSuccessfulLogin(salesPostServletPage, topGroupUser, testRealmSAMLPostLoginPage, "principal=topgroupuser");
+    }
+
     @Test
     public void salesPostTest() {
         testSuccessfulAndUnauthorizedLogin(salesPostServletPage, testRealmSAMLPostLoginPage);

testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/demorealm.json

@@ -49,6 +49,7 @@
                 { "type" : "password",
                     "value" : "password" }
             ],
+            "realmRoles": [ "realm-composite-role" ],
             "groups": [
                 "/top"
             ]
@@ -75,6 +76,14 @@
             {
                 "name": "admin",
                 "description": "Administrator privileges"
+            },
+            {
+                "name": "realm-composite-role",
+                "description": "Realm composite role containing user role",
+                "composite": true,
+                "composites": {
+                    "realm": ["user"]
+                }
             }
         ]
     },