libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

Role mappers must return a single value when they are not multivalued

Browse source 
Closes #20218
This commit is contained in:
Pedro Igor 2023-08-31 09:50:51 -03:00 committed by Marek Posolda
parent 2870ecfb78
commit 13e5a02b9f
4 changed files with 77 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc
View file

@ -35,3 +35,14 @@ If you are using a different encoding, convert the files to UTF-8.
The Oracle Database JDBC driver is no longer part of the Keycloak distribution.
Administrators will need to install a version of the Oracle Driver matching their environment
as described in the configuring the database {section}.
= Changes to the value format of claims mapped by the realm and client role mappers
Before this release, both realm (`User Realm Role`) and client (`User Client Role`) protocol mappers
were mapping a stringfied JSON array when the `Multivalued` setting was disabled.
However, the `Multivalued` setting indicates whether the claim should be mapped as a list or, if disabled, only a single value
from the same list of values.
In this release, the role and client mappers now map to a single value from the effective roles of a user when
they are marked as single-valued (`Multivalued` disabled).

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

@ -29,6 +29,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
@ -70,15 +71,7 @@ abstract class AbstractUserRoleMappingMapper extends AbstractOIDCProtocolMapper
realmRoleNames = rolesToAdd;
}
Object claimValue = realmRoleNames;
boolean multiValued = "true".equals(mappingModel.getConfig().get(ProtocolMapperUtils.MULTIVALUED));
if (!multiValued) {
claimValue = realmRoleNames.toString();
}
//OIDCAttributeMapperHelper.mapClaim(token, mappingModel, claimValue);
mapClaim(token, mappingModel, claimValue, clientId);
mapClaim(token, mappingModel, realmRoleNames, clientId);
}
@ -99,7 +92,16 @@ abstract class AbstractUserRoleMappingMapper extends AbstractOIDCProtocolMapper
if (clientId != null) {
// case when clientId contains dots
clientId = DOT_PATTERN.matcher(clientId).replaceAll(DOT_REPLACEMENT);
protocolClaim = CLIENT_ID_PATTERN.matcher(protocolClaim).replaceAll(clientId);
Matcher matcher = CLIENT_ID_PATTERN.matcher(protocolClaim);
if (matcher.find()) {
protocolClaim = matcher.replaceAll(clientId);
}
if (!(protocolClaim.endsWith("roles") || protocolClaim.startsWith(clientId) || protocolClaim.endsWith(clientId))) {
// the claim name does not reference the current client, do not map roles
// or if the claim does not end with roles suffix, do not map roles.
// the role suffix is used to move roles to a single location other than the default location (e.g.: realm_access and resource_access claims)
return;
}
}
List<String> split = splitClaimPath(protocolClaim);

83
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OIDCProtocolMappersTest.java
View file

@ -79,6 +79,7 @@ import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.in;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.isEmptyOrNullString;
@ -581,8 +582,8 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
// Verify attribute is filled
Map<String, Object> roleMappings = (Map<String, Object>)idToken.getOtherClaims().get("roles-custom");
assertThat(roleMappings.keySet(), containsInAnyOrder("realm", "test-app"));
String realmRoleMappings = (String) roleMappings.get("realm");
String testAppMappings = (String) roleMappings.get("test-app");
List<String> realmRoleMappings = (List<String>) roleMappings.get("realm");
List<String> testAppMappings = (List<String>) roleMappings.get("test-app");
assertRolesString(realmRoleMappings,
"pref.user", // from direct assignment in user definition
"pref.offline_access" // from direct assignment in user definition
@ -808,7 +809,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
// Verify attribute is filled AND it is filled only once
Map<String, Object> roleMappings = (Map<String, Object>)idToken.getOtherClaims().get("roles-custom");
assertThat(roleMappings.keySet(), containsInAnyOrder(clientId));
String testAppScopeMappings = (String) roleMappings.get(clientId);
List<String> testAppScopeMappings = (List<String>) roleMappings.get(clientId);
assertRolesString(testAppScopeMappings,
"customer-user" // from assignment to level2group or level2group2. It is filled just once
);
@ -837,8 +838,8 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
// Verify attribute is filled
Map<String, Object> roleMappings = (Map<String, Object>)idToken.getOtherClaims().get("roles-custom");
assertThat(roleMappings.keySet(), containsInAnyOrder("realm", clientId));
String realmRoleMappings = (String) roleMappings.get("realm");
String testAppMappings = (String) roleMappings.get(clientId);
List<String> realmRoleMappings = (List<String>) roleMappings.get("realm");
List<String> testAppMappings = (List<String>) roleMappings.get(clientId);
assertRolesString(realmRoleMappings,
"pref.admin", // from direct assignment to /roleRichGroup/level2group
"pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup
@ -882,8 +883,8 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
// Verify attribute is filled
Map<String, Object> roleMappings = (Map<String, Object>)idToken.getOtherClaims().get("roles-custom");
assertThat(roleMappings.keySet(), containsInAnyOrder("realm"));
String realmRoleMappings = (String) roleMappings.get("realm");
String testAppAuthzMappings = (String) roleMappings.get(clientId);
List<String> realmRoleMappings = (List<String>) roleMappings.get("realm");
List<String> testAppAuthzMappings = (List<String>) roleMappings.get(clientId);
assertRolesString(realmRoleMappings,
"pref.admin", // from direct assignment to /roleRichGroup/level2group
"pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup
@ -915,8 +916,8 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
// Verify attribute is filled
Map<String, Object> roleMappings = (Map<String, Object>)idToken.getOtherClaims().get("roles-custom");
assertThat(roleMappings.keySet(), containsInAnyOrder("realm", clientId));
String realmRoleMappings = (String) roleMappings.get("realm");
String testAppScopeMappings = (String) roleMappings.get(clientId);
List<String> realmRoleMappings = (List<String>) roleMappings.get("realm");
List<String> testAppScopeMappings = (List<String>) roleMappings.get(clientId);
assertRolesString(realmRoleMappings,
"pref.admin", // from direct assignment to /roleRichGroup/level2group
"pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup
@ -949,8 +950,8 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
// Verify attribute is filled
Map<String, Object> roleMappings = (Map<String, Object>)idToken.getOtherClaims().get("roles-custom");
assertThat(roleMappings.keySet(), containsInAnyOrder("realm", clientId));
String realmRoleMappings = (String) roleMappings.get("realm");
String testAppScopeMappings = (String) roleMappings.get(clientId);
List<String> realmRoleMappings = (List<String>) roleMappings.get("realm");
List<String> testAppScopeMappings = (List<String>) roleMappings.get(clientId);
assertRolesString(realmRoleMappings,
"pref.admin", // from direct assignment to /roleRichGroup/level2group
"pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup
@ -965,6 +966,40 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
deleteMappers(protocolMappers);
}
@Test
public void testSingleValuedRoleMapping() throws Exception {
String clientId = "test-app-scope";
ProtocolMapperRepresentation realmMapper = ProtocolMapperUtil.createUserRealmRoleMappingMapper("pref.", "Realm roles mapper", "roles-custom.realm", true, true, false);
ProtocolMapperRepresentation clientMapper = ProtocolMapperUtil.createUserClientRoleMappingMapper(null, null, "Client roles mapper", "roles-custom.test-app-scope", true, true, false);
ProtocolMappersResource protocolMappers = ApiUtil.findClientResourceByClientId(adminClient.realm("test"), clientId).getProtocolMappers();
protocolMappers.createMapper(Arrays.asList(realmMapper, clientMapper));
// Login user
ClientManager.realm(adminClient.realm("test")).clientId(clientId).directAccessGrant(true);
oauth.clientId(clientId);
OAuthClient.AccessTokenResponse response = browserLogin("password", "rich.roles@redhat.com", "password");
IDToken idToken = oauth.verifyIDToken(response.getIdToken());
// Verify attribute is filled
Map<String, Object> roleMappings = (Map<String, Object>)idToken.getOtherClaims().get("roles-custom");
assertThat(roleMappings.keySet(), containsInAnyOrder("realm", clientId));
String realmRoleMappings = (String) roleMappings.get("realm");
String testAppScopeMappings = (String) roleMappings.get(clientId);
assertSingleValuedRolesString(realmRoleMappings,
"pref.admin", // from direct assignment to /roleRichGroup/level2group
"pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup
"pref.customer-user-premium"
);
assertSingleValuedRolesString(testAppScopeMappings,
"test-app-allowed-by-scope", // from direct assignment to roleRichUser, present as scope allows it
"test-app-disallowed-by-scope" // from direct assignment to /roleRichGroup/level2group, present as scope allows it
);
// Revert
deleteMappers(protocolMappers);
}
@Test
public void testUserGroupRoleToAttributeMappersScopedWithDifferentClient() throws Exception {
final String clientId = "test-app-scope";
@ -988,8 +1023,8 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
Map<String, Object> roleMappings = (Map<String, Object>) idToken.getOtherClaims().get("roles-custom");
assertNotNull(roleMappings);
assertThat(roleMappings.keySet(), containsInAnyOrder("realm", diffClient));
String realmRoleMappings = (String) roleMappings.get("realm");
String testAppScopeMappings = (String) roleMappings.get(diffClient);
List<String> realmRoleMappings = (List<String>) roleMappings.get("realm");
List<String> testAppScopeMappings = (List<String>) roleMappings.get(diffClient);
assertRolesString(realmRoleMappings,
"pref.admin",
"pref.user",
@ -1769,26 +1804,12 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
Assert.assertNames(actualRoleList, expectedRoles);
}
private void assertRolesString(String actualRoleString, String...expectedRoles) {
assertThat(actualRoleString.matches("^\\[.*\\]$"), is(true));
String[] roles = actualRoleString.substring(1, actualRoleString.length() - 1).split(",\\s*");
if (expectedRoles == null || expectedRoles.length == 0) {
assertThat(roles, arrayContainingInAnyOrder(""));
} else {
assertThat(roles, arrayContainingInAnyOrder(expectedRoles));
}
private void assertRolesString(List<String> actualRoleString, String...expectedRoles) {
assertThat(actualRoleString, containsInAnyOrder(expectedRoles));
}
private ProtocolMapperRepresentation makeMapper(String name, String mapperType, Map<String, String> config) {
ProtocolMapperRepresentation rep = new ProtocolMapperRepresentation();
rep.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL);
rep.setName(name);
rep.setProtocolMapper(mapperType);
rep.setConfig(config);
return rep;
private void assertSingleValuedRolesString(String actualRoleString, String... expectedRoles) {
assertThat(actualRoleString, is(in(expectedRoles)));
}
private OAuthClient.AccessTokenResponse browserLogin(String clientSecret, String username, String password) {

4
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/ProtocolMapperUtil.java
View file

@ -118,7 +118,7 @@ public class ProtocolMapperUtil {
String tokenClaimName,
boolean accessToken, boolean idToken) {
return createUserRealmRoleMappingMapper(realmRolePrefix, name, tokenClaimName, accessToken, idToken, false);
return createUserRealmRoleMappingMapper(realmRolePrefix, name, tokenClaimName, accessToken, idToken, true);
}
public static ProtocolMapperRepresentation createUserRealmRoleMappingMapper(String realmRolePrefix,
@ -134,7 +134,7 @@ public class ProtocolMapperUtil {
String tokenClaimName,
boolean accessToken, boolean idToken) {
return createUserClientRoleMappingMapper(clientId, clientRolePrefix, name, tokenClaimName, accessToken, idToken, false);
return createUserClientRoleMappingMapper(clientId, clientRolePrefix, name, tokenClaimName, accessToken, idToken, true);
}
public static ProtocolMapperRepresentation createUserClientRoleMappingMapper(String clientId, String clientRolePrefix,