[KEYCLOAK-18922] - Ignore empty values for internal attributes not set to user
This commit is contained in:
parent
0cdce1340d
commit
afb0b16e43
3 changed files with 149 additions and 9 deletions
|
@ -16,6 +16,8 @@
|
|||
*/
|
||||
package org.keycloak.userprofile.validator;
|
||||
|
||||
import static org.keycloak.common.util.ObjectUtil.isBlank;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.regex.Pattern;
|
||||
|
@ -80,20 +82,13 @@ public class ReadOnlyAttributeUnchangedValidator implements SimpleValidator {
|
|||
existingValue = existingAttrValues.get(0);
|
||||
}
|
||||
|
||||
if (values.isEmpty() && existingValue != null) {
|
||||
context.addError(new ValidationError(ID, key, UPDATE_READ_ONLY_ATTRIBUTES_REJECTED_MSG));
|
||||
return context;
|
||||
}
|
||||
|
||||
String value = null;
|
||||
|
||||
if (!values.isEmpty()) {
|
||||
value = values.get(0);
|
||||
}
|
||||
|
||||
boolean unchanged = ObjectUtil.isEqualOrBothNull(value, existingValue);
|
||||
|
||||
if (!unchanged) {
|
||||
if (!isUnchanged(existingValue, value)) {
|
||||
logger.warnf("Attempt to edit denied attribute '%s' of user '%s'", pattern, user == null ? "new user" : user.getFirstAttribute(UserModel.USERNAME));
|
||||
context.addError(new ValidationError(ID, key, UPDATE_READ_ONLY_ATTRIBUTES_REJECTED_MSG));
|
||||
}
|
||||
|
@ -101,4 +96,13 @@ public class ReadOnlyAttributeUnchangedValidator implements SimpleValidator {
|
|||
return context;
|
||||
}
|
||||
|
||||
private boolean isUnchanged(String existingValue, String value) {
|
||||
if (existingValue == null && isBlank(value)) {
|
||||
// if attribute not set to the user and value is blank/null, then pass validation
|
||||
return true;
|
||||
}
|
||||
|
||||
return ObjectUtil.isEqualOrBothNull(existingValue, value);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -0,0 +1,126 @@
|
|||
/*
|
||||
* Copyright 2020 Red Hat, Inc. and/or its affiliates
|
||||
* and other contributors as indicated by the @author tags.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*
|
||||
*/
|
||||
|
||||
package org.keycloak.testsuite.federation.ldap;
|
||||
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.junit.Assert.fail;
|
||||
import static org.keycloak.testsuite.forms.VerifyProfileTest.disableDynamicUserProfile;
|
||||
import static org.keycloak.testsuite.forms.VerifyProfileTest.setUserProfileConfiguration;
|
||||
import static org.keycloak.util.JsonSerialization.readValue;
|
||||
import static org.keycloak.util.JsonSerialization.writeValueAsString;
|
||||
|
||||
import javax.ws.rs.BadRequestException;
|
||||
import javax.ws.rs.core.Response;
|
||||
import java.io.IOException;
|
||||
import java.util.Collections;
|
||||
|
||||
import org.junit.FixMethodOrder;
|
||||
import org.junit.Test;
|
||||
import org.junit.runners.MethodSorters;
|
||||
import org.keycloak.admin.client.resource.UserResource;
|
||||
import org.keycloak.common.Profile;
|
||||
import org.keycloak.models.LDAPConstants;
|
||||
import org.keycloak.representations.idm.ComponentRepresentation;
|
||||
import org.keycloak.representations.idm.OAuth2ErrorRepresentation;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
import org.keycloak.representations.idm.UserRepresentation;
|
||||
import org.keycloak.storage.UserStorageProvider;
|
||||
import org.keycloak.testsuite.admin.ApiUtil;
|
||||
import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
|
||||
import org.keycloak.testsuite.forms.VerifyProfileTest;
|
||||
import org.keycloak.testsuite.util.UserBuilder;
|
||||
import org.keycloak.userprofile.config.UPAttribute;
|
||||
import org.keycloak.userprofile.config.UPAttributePermissions;
|
||||
import org.keycloak.userprofile.config.UPConfig;
|
||||
|
||||
@EnableFeature(value = Profile.Feature.DECLARATIVE_USER_PROFILE)
|
||||
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
|
||||
public class LDAPAdminRestApiWithUserProfileTest extends LDAPAdminRestApiTest {
|
||||
|
||||
@Test
|
||||
public void testUpdateReadOnlyAttributeWhenNotSetToUser() throws Exception {
|
||||
RealmRepresentation realmRep = testRealm().toRepresentation();
|
||||
enableSyncRegistration(realmRep, Boolean.FALSE);
|
||||
|
||||
UserRepresentation newUser = UserBuilder.create()
|
||||
.username("admintestuser1")
|
||||
.password("userpass")
|
||||
.addAttribute("foo", "foo-value")
|
||||
.enabled(true)
|
||||
.build();
|
||||
|
||||
try (Response response = testRealm().users().create(newUser)) {
|
||||
enableDynamicUserProfile(realmRep);
|
||||
String newUserId = ApiUtil.getCreatedId(response);
|
||||
|
||||
getCleanup().addUserId(newUserId);
|
||||
|
||||
UserResource user = testRealm().users().get(newUserId);
|
||||
UserRepresentation userRep = user.toRepresentation();
|
||||
|
||||
assertTrue(userRep.getAttributes().containsKey(LDAPConstants.LDAP_ID));
|
||||
assertTrue(userRep.getAttributes().get(LDAPConstants.LDAP_ID).isEmpty());
|
||||
|
||||
userRep.singleAttribute(LDAPConstants.LDAP_ID, "");
|
||||
user.update(userRep);
|
||||
userRep.singleAttribute(LDAPConstants.LDAP_ID, null);
|
||||
user.update(userRep);
|
||||
|
||||
try {
|
||||
userRep.singleAttribute(LDAPConstants.LDAP_ID, "should-fail");
|
||||
user.update(userRep);
|
||||
fail("Should fail, attribute is read-only");
|
||||
} catch (BadRequestException ignore) {
|
||||
}
|
||||
} finally {
|
||||
disableDynamicUserProfile(testRealm());
|
||||
enableSyncRegistration(realmRep, Boolean.TRUE);
|
||||
}
|
||||
}
|
||||
|
||||
private void enableDynamicUserProfile(RealmRepresentation realmRep) throws IOException {
|
||||
VerifyProfileTest.enableDynamicUserProfile(realmRep);
|
||||
|
||||
testRealm().update(realmRep);
|
||||
|
||||
UPConfig upConfig = readValue(testRealm().users().userProfile().getConfiguration(), UPConfig.class);
|
||||
UPAttribute attribute = new UPAttribute();
|
||||
|
||||
attribute.setName(LDAPConstants.LDAP_ID);
|
||||
|
||||
UPAttributePermissions permissions = new UPAttributePermissions();
|
||||
|
||||
permissions.setView(Collections.singleton("admin"));
|
||||
|
||||
attribute.setPermissions(permissions);
|
||||
|
||||
upConfig.addAttribute(attribute);
|
||||
|
||||
setUserProfileConfiguration(testRealm(), writeValueAsString(upConfig));
|
||||
}
|
||||
|
||||
private void enableSyncRegistration(RealmRepresentation realmRep, Boolean aFalse) {
|
||||
ComponentRepresentation ldapStorage = testRealm().components()
|
||||
.query(realmRep.getRealm(), UserStorageProvider.class.getName()).get(0);
|
||||
ldapStorage.getConfig().put(LDAPConstants.SYNC_REGISTRATIONS, Collections.singletonList(aFalse.toString()));
|
||||
testRealm().components().component(ldapStorage.getId()).update(ldapStorage);
|
||||
}
|
||||
}
|
|
@ -932,6 +932,16 @@ public class VerifyProfileTest extends AbstractTestRealmKeycloakTest {
|
|||
testRealm.getAttributes().put(REALM_USER_PROFILE_ENABLED, Boolean.TRUE.toString());
|
||||
}
|
||||
|
||||
public static void disableDynamicUserProfile(RealmResource realm) {
|
||||
RealmRepresentation realmRep = realm.toRepresentation();
|
||||
if (realmRep.getAttributes() == null) {
|
||||
realmRep.setAttributes(new HashMap<>());
|
||||
}
|
||||
realmRep.getAttributes().put(REALM_USER_PROFILE_ENABLED, Boolean.FALSE.toString());
|
||||
realm.update(realmRep);
|
||||
}
|
||||
|
||||
|
||||
public static void setUserProfileConfiguration(RealmResource testRealm, String configuration) {
|
||||
Response r = testRealm.users().userProfile().update(configuration);
|
||||
if (r.getStatus() != 200) {
|
||||
|
|
Loading…
Reference in a new issue