From 3c4114091f6ce3cbb6b917aa4533daad99c1d628 Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Mon, 5 Dec 2016 16:14:48 +0100 Subject: [PATCH] KEYCLOAK-4035 Composite roles need to be expanded in SAML attribute mapper --- .../org/keycloak/models/utils/RoleUtils.java | 33 ++++++++++++++ .../AbstractUserRoleMappingMapper.java | 28 +----------- .../protocol/saml/mappers/HardcodedRole.java | 9 ++-- .../protocol/saml/mappers/RoleListMapper.java | 44 ++++++++++++------- .../AbstractSAMLServletsAdapterTest.java | 11 +++++ .../resources/adapter-test/demorealm.json | 9 ++++ 6 files changed, 87 insertions(+), 47 deletions(-) diff --git a/server-spi/src/main/java/org/keycloak/models/utils/RoleUtils.java b/server-spi/src/main/java/org/keycloak/models/utils/RoleUtils.java index a03a55d9f0..1dd72673c8 100644 --- a/server-spi/src/main/java/org/keycloak/models/utils/RoleUtils.java +++ b/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 expandCompositeRolesStream(RoleModel role) { + Stream.Builder sb = Stream.builder(); + Set roles = new HashSet<>(); + + Deque 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(); + } + } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/mappers/AbstractUserRoleMappingMapper.java b/services/src/main/java/org/keycloak/protocol/oidc/mappers/AbstractUserRoleMappingMapper.java index 4de3720422..bfedd292e2 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/mappers/AbstractUserRoleMappingMapper.java +++ b/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 expandCompositeRolesStream(RoleModel role) { - Stream.Builder sb = Stream.builder(); - - Deque 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}. diff --git a/services/src/main/java/org/keycloak/protocol/saml/mappers/HardcodedRole.java b/services/src/main/java/org/keycloak/protocol/saml/mappers/HardcodedRole.java index 9efead26fd..9b78e60c94 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/mappers/HardcodedRole.java +++ b/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 configProperties = new ArrayList(); + private static final List 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 config = new HashMap(); - config.put("role", role); + Map config = new HashMap<>(); + config.put(ROLE_ATTRIBUTE, role); mapper.setConfig(config); return mapper; diff --git a/services/src/main/java/org/keycloak/protocol/saml/mappers/RoleListMapper.java b/services/src/main/java/org/keycloak/protocol/saml/mappers/RoleListMapper.java index dd274728e9..5650333c11 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/mappers/RoleListMapper.java +++ b/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 Bill Burke @@ -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 configProperties = new ArrayList(); + private static final List 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()) { - // todo need a role mapping - RoleModel roleModel = clientSession.getRealm().getRoleById(roleId); - AttributeType attributeType = null; + RealmModel realm = clientSession.getRealm(); + List allRoleNames = clientSession.getRoles().stream() + // todo need a role mapping + .map(realm::getRoleById) + .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 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 config = new HashMap(); + Map config = new HashMap<>(); config.put(AttributeStatementHelper.SAML_ATTRIBUTE_NAME, samlAttributeName); if (friendlyName != null) { config.put(AttributeStatementHelper.FRIENDLY_NAME, friendlyName); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java index d6d52f2835..3f9dabb453 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java +++ b/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); diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/demorealm.json b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/demorealm.json index b1f70e2738..072ca402bb 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/demorealm.json +++ b/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"] + } } ] },