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

[KEYCLOAK-18748] - Do not remove attributes when declarative provider is enabled

Browse source
This commit is contained in:
Pedro Igor 2021-07-14 09:43:55 -03:00
parent dc1c9b944f
commit f1face6973
7 changed files with 221 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java
View file

@ -55,7 +55,7 @@ public class DefaultAttributes extends HashMap<String, List<String>> implements
*/
public static final String READ_ONLY_ATTRIBUTE_KEY = "kc.read.only";
private final UserProfileContext context;
protected final UserProfileContext context;
private final KeycloakSession session;
private final Map<String, AttributeMetadata> metadataByAttribute;
protected final UserModel user;
@ -72,10 +72,20 @@ public class DefaultAttributes extends HashMap<String, List<String>> implements
@Override
public boolean isReadOnly(String attributeName) {
return isReadOnlyFromMetadata(attributeName) || isReadOnlyInternalAttribute(attributeName);
if (isReadOnlyFromMetadata(attributeName) || isReadOnlyInternalAttribute(attributeName)) {
return true;
}
return getMetadata(attributeName) == null;
}
private boolean isReadOnlyFromMetadata(String attributeName) {
/**
* Checks whether an attribute is marked as read only by looking at its metadata.
*
* @param attributeName the attribute name
* @return @return {@code true} if the attribute is readonly. Otherwise, returns {@code false}
*/
protected boolean isReadOnlyFromMetadata(String attributeName) {
AttributeMetadata attributeMetadata = metadataByAttribute.get(attributeName);
if (attributeMetadata == null) {
@ -168,13 +178,14 @@ public class DefaultAttributes extends HashMap<String, List<String>> implements
@Override
public Map<String, List<String>> getReadable() {
if(user == null)
return null;
Map<String, List<String>> attributes = new HashMap<>(user.getAttributes());
Map<String, List<String>> attributes = new HashMap<>(this);
if (attributes.isEmpty()) {
return null;
for (String name : nameSet()) {
AttributeMetadata metadata = getMetadata(name);
if (metadata == null || !metadata.canView(createAttributeContext(metadata))) {
attributes.remove(name);
}
}
return attributes;
@ -300,8 +311,7 @@ public class DefaultAttributes extends HashMap<String, List<String>> implements
}
protected boolean isIncludeAttributeIfNotProvided(AttributeMetadata metadata) {
// user api expects that attributes are not updated if not provided when in legacy mode
return UserProfileContext.USER_API.equals(context);
return !metadata.canEdit(createAttributeContext(metadata));
}
/**
@ -313,7 +323,7 @@ public class DefaultAttributes extends HashMap<String, List<String>> implements
* @param name the name of the attribute
* @return
*/
private boolean isSupportedAttribute(String name) {
protected boolean isSupportedAttribute(String name) {
if (READ_ONLY_ATTRIBUTE_KEY.equals(name)) {
return false;
}
@ -327,11 +337,6 @@ public class DefaultAttributes extends HashMap<String, List<String>> implements
return true;
}
// attributes managed using forms with a pre-defined prefix are supported
if (name.startsWith(Constants.USER_ATTRIBUTES_PREFIX)) {
return true;
}
if (isReadOnly(name)) {
return true;
}
@ -340,7 +345,16 @@ public class DefaultAttributes extends HashMap<String, List<String>> implements
return isRootAttribute(name);
}
private boolean isReadOnlyInternalAttribute(String attributeName) {
/**
* <p>Returns whether an attribute is read only based on the provider configuration (using provider config),
* usually related to internal attributes managed by the server.
*
* <p>For user-defined attributes, it should be preferable to use the user profile configuration.
*
* @param attributeName the attribute name
* @return {@code true} if the attribute is readonly. Otherwise, returns {@code false}
*/
protected boolean isReadOnlyInternalAttribute(String attributeName) {
// read-only can be configured through the provider so we try to validate global validations
AttributeMetadata readonlyMetadata = metadataByAttribute.get(READ_ONLY_ATTRIBUTE_KEY);

48
services/src/main/java/org/keycloak/userprofile/DeclarativeAttributes.java
View file

@ -1,48 +0,0 @@
package org.keycloak.userprofile;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.UserModel;
import org.keycloak.userprofile.AttributeMetadata;
import org.keycloak.userprofile.DeclarativeUserProfileProvider;
import org.keycloak.userprofile.DefaultAttributes;
import org.keycloak.userprofile.UserProfileContext;
import org.keycloak.userprofile.UserProfileMetadata;
/**
* Temporary implementation of {@link org.keycloak.userprofile.Attributes}. It should be removed once
* the {@link DeclarativeUserProfileProvider} becomes the default.
*
* @author <a href="mailto:psilva@redhat.com">Pedro Igor</a>
*/
public class DeclarativeAttributes extends DefaultAttributes {
public DeclarativeAttributes(UserProfileContext context, Map<String, ?> attributes,
UserModel user, UserProfileMetadata profileMetadata,
KeycloakSession session) {
super(context, attributes, user, profileMetadata, session);
}
@Override
public Map<String, List<String>> getReadable() {
Map<String, List<String>> attributes = new HashMap<>(this);
for (String name : nameSet()) {
AttributeMetadata metadata = getMetadata(name);
if (metadata == null || !metadata.canView(createAttributeContext(metadata))) {
attributes.remove(name);
}
}
return attributes;
}
@Override
protected boolean isIncludeAttributeIfNotProvided(AttributeMetadata metadata) {
return !metadata.canEdit(createAttributeContext(metadata));
}
}

4
services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProvider.java
View file

@ -120,10 +120,10 @@ public class DeclarativeUserProfileProvider extends AbstractUserProfileProvider<
@Override
protected Attributes createAttributes(UserProfileContext context, Map<String, ?> attributes,
UserModel user, UserProfileMetadata metadata) {
if (!isEnabled(session)) {
if (isEnabled(session)) {
return new DefaultAttributes(context, attributes, user, metadata, session);
}
return new DeclarativeAttributes(context, attributes, user, metadata, session);
return new LegacyAttributes(context, attributes, user, metadata, session);
}
@Override

60
services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java Normal file
View file

@ -0,0 +1,60 @@
package org.keycloak.userprofile;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.keycloak.models.Constants;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.UserModel;
/**
* Enables legacy support when managing attributes without the declarative provider.
*
* @author <a href="mailto:psilva@redhat.com">Pedro Igor</a>
*/
public class LegacyAttributes extends DefaultAttributes {
public LegacyAttributes(UserProfileContext context, Map<String, ?> attributes, UserModel user,
UserProfileMetadata profileMetadata, KeycloakSession session) {
super(context, attributes, user, profileMetadata, session);
}
@Override
protected boolean isSupportedAttribute(String name) {
if (super.isSupportedAttribute(name)) {
return true;
}
if (name.startsWith(Constants.USER_ATTRIBUTES_PREFIX)) {
return true;
}
return false;
}
@Override
public boolean isReadOnly(String attributeName) {
return isReadOnlyFromMetadata(attributeName) || isReadOnlyInternalAttribute(attributeName);
}
@Override
public Map<String, List<String>> getReadable() {
if(user == null)
return null;
Map<String, List<String>> attributes = new HashMap<>(user.getAttributes());
if (attributes.isEmpty()) {
return null;
}
return attributes;
}
@Override
protected boolean isIncludeAttributeIfNotProvided(AttributeMetadata metadata) {
// user api expects that attributes are not updated if not provided when in legacy mode
return UserProfileContext.USER_API.equals(context);
}
}

10
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileWithUserProfileTest.java
View file

@ -510,6 +510,16 @@ public class RequiredActionUpdateProfileWithUserProfileTest extends RequiredActi
assertEquals("LastCC", user.getLastName());
}
@Test
public void updateProfileWithoutRemoveCustomAttributes() {
setUserProfileConfiguration("{\"attributes\": ["
+ "{\"name\": \"firstName\"," + PERMISSIONS_ALL + ", \"required\": {}},"
+ "{\"name\": \"lastName\"," + PERMISSIONS_ALL + ", \"required\": {}},"
+ "{\"name\": \"custom\"," + PERMISSIONS_ALL + "}"
+ "]}");
super.updateProfileWithoutRemoveCustomAttributes();
}
protected void setUserProfileConfiguration(String configuration) {
VerifyProfileTest.setUserProfileConfiguration(testRealm(), configuration);
}

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

@ -33,6 +33,7 @@ import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson;
import static org.keycloak.testsuite.admin.ApiUtil.findClientByClientId;
import static org.keycloak.testsuite.admin.ApiUtil.findClientResourceByClientId;
import static org.keycloak.testsuite.admin.ApiUtil.findUserByUsernameId;
import static org.keycloak.testsuite.forms.VerifyProfileTest.PERMISSIONS_ALL;
import static org.keycloak.testsuite.util.ProtocolMapperUtil.createAddressMapper;
import static org.keycloak.testsuite.util.ProtocolMapperUtil.createClaimMapper;
import static org.keycloak.testsuite.util.ProtocolMapperUtil.createHardcodedClaim;
@ -82,6 +83,7 @@ import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
import org.keycloak.testsuite.forms.VerifyProfileTest;
import org.keycloak.testsuite.updaters.ClientAttributeUpdater;
import org.keycloak.testsuite.updaters.ProtocolMappersUpdater;
import org.keycloak.testsuite.util.ClientManager;
@ -103,6 +105,25 @@ public class OIDCProtocolMappersUserProfileTest extends OIDCProtocolMappersTest
testRealm.getAttributes().put(REALM_USER_PROFILE_ENABLED, Boolean.TRUE.toString());
}
@Before
public void onBefore() {
VerifyProfileTest.setUserProfileConfiguration(adminClient.realm("test"), "{\"attributes\": ["
+ "{\"name\": \"firstName\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"lastName\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"group-value\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"street\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"departments\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"locality\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"region_some\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"postal_code\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"country\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"formatted\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"phone\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"json-attribute\"," + PERMISSIONS_ALL + "},"
+ "{\"name\": \"json-attribute-multi\"," + PERMISSIONS_ALL + "}"
+ "]}");
}
@Test
public void testMappingFromAttribute() {
ClientResource app = findClientResourceByClientId(adminClient.realm("test"), "test-app");

98
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java
View file

@ -45,6 +45,7 @@ import org.junit.Assert;
import org.junit.Test;
import org.keycloak.component.ComponentModel;
import org.keycloak.component.ComponentValidationException;
import org.keycloak.models.Constants;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
@ -78,6 +79,8 @@ import org.keycloak.validate.validators.LengthValidator;
@AuthServerContainerExclude(AuthServerContainerExclude.AuthServer.REMOTE)
public class UserProfileTest extends AbstractUserProfileTest {
protected static final String ATT_ADDRESS = "address";
@Override
public void configureTestRealm(RealmRepresentation testRealm) {
super.configureTestRealm(testRealm);
@ -351,8 +354,26 @@ public class UserProfileTest extends AbstractUserProfileTest {
getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testCreateAndUpdateUser);
}
private static void testCreateAndUpdateUser(KeycloakSession session) {
private static void testCreateAndUpdateUser(KeycloakSession session) throws IOException {
UserProfileProvider provider = getDynamicUserProfileProvider(session);
UPConfig config = JsonSerialization.readValue(provider.getConfiguration(), UPConfig.class);
UPAttribute attribute = new UPAttribute();
attribute.setName("address");
UPAttributePermissions permissions = new UPAttributePermissions();
permissions.setEdit(new HashSet<>(Arrays.asList("admin", "user")));
attribute.setPermissions(permissions);
config.addAttribute(attribute);
attribute = new UPAttribute();
attribute.setName("business.address");
permissions = new UPAttributePermissions();
permissions.setEdit(new HashSet<>(Arrays.asList("admin", "user")));
attribute.setPermissions(permissions);
config.addAttribute(attribute);
provider.setConfiguration(JsonSerialization.writeValueAsString(config));
Map<String, Object> attributes = new HashMap<>();
String userName = org.keycloak.models.utils.KeycloakModelUtils.generateId();
@ -441,7 +462,80 @@ public class UserProfileTest extends AbstractUserProfileTest {
assertTrue(profile.getAttributes().isReadOnly("department"));
}
protected static final String ATT_ADDRESS = "address";
@Test
public void testDoNotUpdateUndefinedAttributes() {
getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testDoNotUpdateUndefinedAttributes);
}
private static void testDoNotUpdateUndefinedAttributes(KeycloakSession session) {
Map<String, Object> attributes = new HashMap<>();
attributes.put(UserModel.USERNAME, org.keycloak.models.utils.KeycloakModelUtils.generateId());
attributes.put("address", Arrays.asList("fixed-address"));
attributes.put("department", Arrays.asList("sales"));
attributes.put("phone", Arrays.asList("fixed-phone"));
UserProfileProvider provider = getDynamicUserProfileProvider(session);
provider.setConfiguration("{\"attributes\": [{\"name\": \"department\", \"permissions\": {\"edit\": [\"admin\"]}},"
+ "{\"name\": \"phone\", \"permissions\": {\"edit\": [\"admin\"]}},"
+ "{\"name\": \"address\", \"permissions\": {\"edit\": [\"admin\"]}}]}");
UserProfile profile = provider.create(UserProfileContext.ACCOUNT, attributes);
UserModel user = profile.create();
assertThat(profile.getAttributes().nameSet(),
containsInAnyOrder(UserModel.USERNAME, UserModel.EMAIL, "address", "department", "phone"));
profile = provider.create(UserProfileContext.USER_API, attributes, user);
Set<String> attributesUpdated = new HashSet<>();
profile.update((attributeName, userModel) -> assertTrue(attributesUpdated.add(attributeName)));
assertThat(attributesUpdated, containsInAnyOrder("department", "address", "phone"));
provider.setConfiguration("{\"attributes\": [{\"name\": \"department\", \"permissions\": {\"edit\": [\"admin\"]}},"
+ "{\"name\": \"phone\", \"permissions\": {\"edit\": [\"admin\"]}}]}");
attributesUpdated.clear();
attributes.remove("address");
attributes.put("department", "foo");
attributes.put("phone", "foo");
profile = provider.create(UserProfileContext.USER_API, attributes, user);
profile.update((attributeName, userModel) -> assertTrue(attributesUpdated.add(attributeName)));
assertThat(attributesUpdated, containsInAnyOrder("department", "phone"));
assertTrue(user.getAttributes().containsKey("address"));
provider.setConfiguration("{\"attributes\": [{\"name\": \"department\", \"permissions\": {\"edit\": [\"admin\"]}},"
+ "{\"name\": \"phone\", \"permissions\": {\"edit\": [\"admin\"]}},"
+ "{\"name\": \"address\", \"permissions\": {\"edit\": [\"admin\"]}}]}");
attributes.put("department", "foo");
attributes.put("phone", "foo");
attributes.put("address", "bar");
attributesUpdated.clear();
profile = provider.create(UserProfileContext.USER_API, attributes, user);
profile.update((attributeName, userModel) -> assertTrue(attributesUpdated.add(attributeName)));
assertThat(attributesUpdated, containsInAnyOrder("address"));
assertEquals("bar", user.getFirstAttribute("address"));
assertEquals("foo", user.getFirstAttribute("phone"));
assertEquals("foo", user.getFirstAttribute("department"));
attributes.remove("address");
attributesUpdated.clear();
profile = provider.create(UserProfileContext.USER_API, attributes, user);
profile.update((attributeName, userModel) -> assertTrue(attributesUpdated.add(attributeName)));
assertThat(attributesUpdated, containsInAnyOrder("address"));
assertFalse(user.getAttributes().containsKey("address"));
assertTrue(user.getAttributes().containsKey("phone"));
assertTrue(user.getAttributes().containsKey("department"));
String prefixedAttributeName = Constants.USER_ATTRIBUTES_PREFIX.concat("prefixed");
attributes.put(prefixedAttributeName, "foo");
attributesUpdated.clear();
profile = provider.create(UserProfileContext.USER_API, attributes, user);
profile.update((attributeName, userModel) -> assertTrue(attributesUpdated.add(attributeName)));
assertTrue(attributesUpdated.isEmpty());
assertFalse(user.getAttributes().containsKey("prefixedAttributeName"));
}
@Test
public void testInvalidConfiguration() {