KEYCLOAK-13047 LDAP no-import fixes. Avoid lost updates - dont allow update attributes, which are not mapped to LDAP

This commit is contained in:
mposolda 2020-02-27 08:51:47 +01:00 committed by Marek Posolda
parent 68024396f1
commit 12d965abf3
24 changed files with 697 additions and 243 deletions

View file

@ -50,6 +50,7 @@ import org.keycloak.storage.StorageId;
import org.keycloak.storage.UserStorageProvider;
import org.keycloak.storage.UserStorageProviderModel;
import org.keycloak.storage.adapter.InMemoryUserAdapter;
import org.keycloak.storage.adapter.UpdateOnlyChangeUserModelDelegate;
import org.keycloak.storage.ldap.idm.model.LDAPObject;
import org.keycloak.storage.ldap.idm.query.Condition;
import org.keycloak.storage.ldap.idm.query.EscapeStrategy;
@ -149,10 +150,10 @@ public class LDAPStorageProvider implements UserStorageProvider,
return null;
}
return proxy(realm, local, ldapObject);
return proxy(realm, local, ldapObject, false);
}
protected UserModel proxy(RealmModel realm, UserModel local, LDAPObject ldapObject) {
protected UserModel proxy(RealmModel realm, UserModel local, LDAPObject ldapObject, boolean newUser) {
UserModel existing = userManager.getManagedProxiedUser(local.getId());
if (existing != null) {
return existing;
@ -174,17 +175,17 @@ public class LDAPStorageProvider implements UserStorageProvider,
switch (editMode) {
case READ_ONLY:
if (model.isImportEnabled()) {
proxied = new ReadonlyLDAPUserModelDelegate(local, this);
} else {
proxied = new ReadOnlyUserModelDelegate(local);
}
break;
case WRITABLE:
proxied = new WritableLDAPUserModelDelegate(local, this, ldapObject);
break;
case UNSYNCED:
proxied = new UnsyncedLDAPUserModelDelegate(local, this);
// Any attempt to write data, which are not supported by the LDAP schema, should fail
// This check is skipped when register new user as there are many "generic" attributes always written (EG. enabled, emailVerified) and those are usually unsupported by LDAP schema
if (!model.isImportEnabled() && !newUser) {
UserModel readOnlyDelegate = new ReadOnlyUserModelDelegate(local, ModelException::new);
proxied = new LDAPWritesOnlyUserModelDelegate(readOnlyDelegate, this);
}
break;
}
List<ComponentModel> mappers = realm.getComponents(model.getId(), LDAPStorageMapper.class.getName());
@ -194,6 +195,10 @@ public class LDAPStorageProvider implements UserStorageProvider,
proxied = ldapMapper.proxy(ldapObject, proxied, realm);
}
if (!model.isImportEnabled()) {
proxied = new UpdateOnlyChangeUserModelDelegate(proxied);
}
userManager.setManagedProxiedUser(proxied, ldapObject);
return proxied;
@ -272,7 +277,7 @@ public class LDAPStorageProvider implements UserStorageProvider,
user.setSingleAttribute(LDAPConstants.LDAP_ENTRY_DN, ldapUser.getDn().toString());
// Add the user to the default groups and add default required actions
UserModel proxy = proxy(realm, user, ldapUser);
UserModel proxy = proxy(realm, user, ldapUser, true);
DefaultRoles.addDefaultRoles(realm, proxy);
for (GroupModel g : realm.getDefaultGroups()) {
@ -534,7 +539,7 @@ public class LDAPStorageProvider implements UserStorageProvider,
}
logger.debugf("Imported new user from LDAP to Keycloak DB. Username: [%s], Email: [%s], LDAP_ID: [%s], LDAP Entry DN: [%s]", imported.getUsername(), imported.getEmail(),
ldapUser.getUuid(), userDN);
UserModel proxy = proxy(realm, imported, ldapUser);
UserModel proxy = proxy(realm, imported, ldapUser, false);
return proxy;
}
@ -566,7 +571,7 @@ public class LDAPStorageProvider implements UserStorageProvider,
LDAPUtils.checkUuid(ldapUser, ldapIdentityStore.getConfig());
// If email attribute mapper is set to "Always Read Value From LDAP" the user may be in Keycloak DB with an old email address
if (ldapUser.getUuid().equals(user.getFirstAttribute(LDAPConstants.LDAP_ID))) {
return proxy(realm, user, ldapUser);
return proxy(realm, user, ldapUser, false);
}
throw new ModelDuplicateException("User with username '" + ldapUsername + "' already exists in Keycloak. It conflicts with LDAP user with email '" + email + "'");
}
@ -757,7 +762,7 @@ public class LDAPStorageProvider implements UserStorageProvider,
} else {
LDAPObject ldapObject = loadAndValidateUser(realm, user);
if (ldapObject != null) {
return proxy(realm, user, ldapObject);
return proxy(realm, user, ldapObject, false);
} else {
logger.warnf("User with username [%s] aready exists and is linked to provider [%s] but is not valid. Stale LDAP_ID on local user is: %s",
username, model.getName(), user.getFirstAttribute(LDAPConstants.LDAP_ID));

View file

@ -253,6 +253,7 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory<LD
@Override
public void validateConfiguration(KeycloakSession session, RealmModel realm, ComponentModel config) throws ComponentValidationException {
LDAPConfig cfg = new LDAPConfig(config.getConfig());
UserStorageProviderModel userStorageModel = new UserStorageProviderModel(config);
String customFilter = cfg.getCustomUserSearchFilter();
LDAPUtils.validateCustomLdapFilter(customFilter);
@ -277,6 +278,10 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory<LD
if(cfg.isStartTls() && cfg.getConnectionPooling() != null) {
throw new ComponentValidationException("ldapErrorCantEnableStartTlsAndConnectionPooling");
}
if (!userStorageModel.isImportEnabled() && cfg.getEditMode() == UserStorageProvider.EditMode.UNSYNCED) {
throw new ComponentValidationException("ldapErrorCantEnableUnsyncedAndImportOff");
}
}
@Override

View file

@ -0,0 +1,155 @@
/*
* Copyright 2019 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.storage.ldap;
import java.util.List;
import java.util.function.Function;
import org.keycloak.models.LDAPConstants;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.ReadOnlyUserModelDelegate;
import org.keycloak.models.utils.UserModelDelegate;
import org.keycloak.storage.ldap.mappers.LDAPTransaction;
/**
* User model delegate, which tracks what attributes were written to LDAP in this transaction. For those attributes, it will skip
* calling delegate for doing any additional updates.
*
* It may be typically used together with Read-Only delegate. The result is that read-only exception will be thrown when attempt
* to update any user attribute, which is NOT mapped to LDAP.
*
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class LDAPWritesOnlyUserModelDelegate extends UserModelDelegate {
private final LDAPStorageProvider provider;
public LDAPWritesOnlyUserModelDelegate(UserModel delegate, LDAPStorageProvider provider) {
super(delegate);
this.provider = provider;
}
@Override
public void setUsername(String username) {
if (!isAttributeUpdatedInLDAP(UserModel.USERNAME)) {
super.setUsername(username);
}
}
@Override
public void setEnabled(boolean enabled) {
if (!isAttributeUpdatedInLDAP(LDAPConstants.ENABLED)) {
super.setEnabled(enabled);
}
}
@Override
public void setSingleAttribute(String name, String value) {
if (!isAttributeUpdatedInLDAP(name)) {
super.setSingleAttribute(name, value);
}
}
@Override
public void setAttribute(String name, List<String> values) {
if (!isAttributeUpdatedInLDAP(name)) {
super.setAttribute(name, values);
}
}
@Override
public void removeAttribute(String name) {
if (!isAttributeUpdatedInLDAP(name)) {
super.removeAttribute(name);
}
}
@Override
public void addRequiredAction(String action) {
if (!isRequiredActionUpdatedInLDAP(action)) {
super.addRequiredAction(action);
}
}
@Override
public void removeRequiredAction(String action) {
if (!isRequiredActionUpdatedInLDAP(action)) {
super.removeRequiredAction(action);
}
}
@Override
public void addRequiredAction(RequiredAction action) {
if (!isRequiredActionUpdatedInLDAP(action.toString())) {
super.addRequiredAction(action);
}
}
@Override
public void removeRequiredAction(RequiredAction action) {
if (!isRequiredActionUpdatedInLDAP(action.toString())) {
super.removeRequiredAction(action);
}
}
@Override
public void setFirstName(String firstName) {
if (!isAttributeUpdatedInLDAP(UserModel.FIRST_NAME)) {
super.setFirstName(firstName);
}
}
@Override
public void setLastName(String lastName) {
if (!isAttributeUpdatedInLDAP(UserModel.LAST_NAME)) {
super.setLastName(lastName);
}
}
@Override
public void setEmail(String email) {
if (!isAttributeUpdatedInLDAP(UserModel.EMAIL)) {
super.setEmail(email);
}
}
@Override
public void setEmailVerified(boolean verified) {
if (!isAttributeUpdatedInLDAP("emailVerified")) {
super.setEmailVerified(verified);
}
}
// Checks if attribute was updated in LDAP in this transaction
protected boolean isAttributeUpdatedInLDAP(String attributeName) {
LDAPTransaction transaction = provider.getUserManager().getTransaction(getId());
if (transaction == null) return false;
return transaction.isAttributeUpdated(attributeName);
}
// Checks if requiredAction was updated in LDAP in this transaction
protected boolean isRequiredActionUpdatedInLDAP(String requiredActionName) {
LDAPTransaction transaction = provider.getUserManager().getTransaction(getId());
if (transaction == null) return false;
return transaction.isRequiredActionUpdated(requiredActionName);
}
}

View file

@ -1,57 +0,0 @@
/*
* Copyright 2016 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.storage.ldap;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.UserModelDelegate;
import org.keycloak.storage.ReadOnlyException;
/**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $
*/
public class ReadonlyLDAPUserModelDelegate extends UserModelDelegate implements UserModel {
protected LDAPStorageProvider provider;
public ReadonlyLDAPUserModelDelegate(UserModel delegate, LDAPStorageProvider provider) {
super(delegate);
this.provider = provider;
}
@Override
public void setUsername(String username) {
throw new ReadOnlyException("Federated storage is not writable");
}
@Override
public void setLastName(String lastName) {
throw new ReadOnlyException("Federated storage is not writable");
}
@Override
public void setFirstName(String first) {
throw new ReadOnlyException("Federated storage is not writable");
}
@Override
public void setEmail(String email) {
throw new ReadOnlyException("Federated storage is not writable");
}
}

View file

@ -1,37 +0,0 @@
/*
* Copyright 2016 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.storage.ldap;
import org.jboss.logging.Logger;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.UserModelDelegate;
/**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $
*/
public class UnsyncedLDAPUserModelDelegate extends UserModelDelegate implements UserModel {
private static final Logger logger = Logger.getLogger(UnsyncedLDAPUserModelDelegate.class);
protected LDAPStorageProvider provider;
public UnsyncedLDAPUserModelDelegate(UserModel delegate, LDAPStorageProvider provider) {
super(delegate);
this.provider = provider;
}
}

View file

@ -1,41 +0,0 @@
/*
* Copyright 2016 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.storage.ldap;
import org.jboss.logging.Logger;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.UserModelDelegate;
import org.keycloak.storage.ldap.idm.model.LDAPObject;
/**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $
*/
public class WritableLDAPUserModelDelegate extends UserModelDelegate implements UserModel {
private static final Logger logger = Logger.getLogger(WritableLDAPUserModelDelegate.class);
protected LDAPStorageProvider provider;
protected LDAPObject ldapObject;
public WritableLDAPUserModelDelegate(UserModel delegate, LDAPStorageProvider provider, LDAPObject ldapObject) {
super(delegate);
this.provider = provider;
this.ldapObject = ldapObject;
}
}

View file

@ -218,10 +218,7 @@ public class LDAPIdentityStore implements IdentityStore {
String newDn = newLdapDn.toString();
// TODO:mposolda
if (logger.isInfoEnabled()) {
logger.infof("Renaming LDAP Object. Old DN: [%s], New DN: [%s]", oldDn, newDn);
}
logger.debugf("Renaming LDAP Object. Old DN: [%s], New DN: [%s]", oldDn, newDn);
// In case, that there is conflict (For example already existing "CN=John Anthony"), the different DN is returned
newDn = this.operationManager.renameEntry(oldDn, newDn, true);

View file

@ -22,10 +22,8 @@ import org.keycloak.models.GroupModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.storage.ldap.LDAPConfig;
import org.keycloak.storage.ldap.LDAPStorageProvider;
import org.keycloak.storage.ldap.idm.model.LDAPObject;
import org.keycloak.storage.ldap.idm.query.internal.LDAPQuery;
import org.keycloak.storage.user.SynchronizationResult;
import javax.naming.AuthenticationException;

View file

@ -90,16 +90,32 @@ public class FullNameLDAPStorageMapper extends AbstractLDAPStorageMapper {
TxAwareLDAPUserModelDelegate txDelegate = new TxAwareLDAPUserModelDelegate(delegate, ldapProvider, ldapUser) {
// Per-transaction state. Useful due the fact that "setFirstName" and "setLastName" called within same transaction
private String firstName;
private String lastName;
@Override
public String getFirstName() {
return firstName != null? firstName : super.getFirstName();
}
@Override
public String getLastName() {
return lastName != null ? lastName : super.getLastName();
}
@Override
public void setFirstName(String firstName) {
super.setFirstName(firstName);
this.firstName = firstName;
setFullNameToLDAPObject();
super.setFirstName(firstName);
}
@Override
public void setLastName(String lastName) {
super.setLastName(lastName);
this.lastName = lastName;
setFullNameToLDAPObject();
super.setLastName(lastName);
}
private void setFullNameToLDAPObject() {
@ -108,7 +124,8 @@ public class FullNameLDAPStorageMapper extends AbstractLDAPStorageMapper {
logger.tracef("Pushing full name attribute to LDAP. Full name: %s", fullName);
}
ensureTransactionStarted();
markUpdatedAttributeInTransaction(UserModel.FIRST_NAME);
markUpdatedAttributeInTransaction(UserModel.LAST_NAME);
String ldapFullNameAttrName = getLdapFullNameAttrName();
ldapUser.setSingleAttribute(ldapFullNameAttrName, fullName);

View file

@ -17,9 +17,11 @@
package org.keycloak.storage.ldap.mappers;
import java.util.HashSet;
import java.util.Set;
import org.jboss.logging.Logger;
import org.keycloak.models.AbstractKeycloakTransaction;
import org.keycloak.models.KeycloakTransaction;
import org.keycloak.storage.ldap.LDAPStorageProvider;
import org.keycloak.storage.ldap.idm.model.LDAPObject;
@ -33,6 +35,9 @@ public class LDAPTransaction extends AbstractKeycloakTransaction {
private final LDAPStorageProvider ldapProvider;
private final LDAPObject ldapUser;
// Tracks the attributes updated in this transaction
private final Set<String> updatedAttributes = new HashSet<>();
public LDAPTransaction(LDAPStorageProvider ldapProvider, LDAPObject ldapUser) {
this.ldapProvider = ldapProvider;
this.ldapUser = ldapUser;
@ -54,5 +59,40 @@ public class LDAPTransaction extends AbstractKeycloakTransaction {
logger.warn("Transaction rollback! Ignoring LDAP updates for object " + ldapUser.getDn().toString());
}
/**
* Add attribute, which will be updated in LDAP in this transaction
*
* @param attributeName model attribute name (For example "firstName", "lastName", "street")
*/
public void addUpdatedAttribute(String attributeName) {
updatedAttributes.add(attributeName);
}
/**
* @param attributeName model attribute name (For example "firstName", "lastName", "street")
* @return true if attribute was updated in this transaction
*/
public boolean isAttributeUpdated(String attributeName) {
return updatedAttributes.contains(attributeName);
}
/**
* Add required action, which will be updated in LDAP in this transaction
*
* @param requiredActionName
*/
public void addUpdatedRequiredAction(String requiredActionName) {
updatedAttributes.add("requiredAction(" + requiredActionName + ")");
}
/**
*
* @param requiredActionName
* @return true if requiredAction was updated in this transaction
*/
public boolean isRequiredActionUpdated(String requiredActionName) {
return updatedAttributes.contains("requiredAction(" + requiredActionName + ")");
}
}

View file

@ -50,4 +50,16 @@ public abstract class TxAwareLDAPUserModelDelegate extends UserModelDelegate {
}
}
protected void markUpdatedAttributeInTransaction(String modelAttributeName) {
ensureTransactionStarted();
LDAPTransaction transaction = provider.getUserManager().getTransaction(getId());
transaction.addUpdatedAttribute(modelAttributeName);
}
protected void markUpdatedRequiredActionInTransaction(String requiredActionName) {
ensureTransactionStarted();
LDAPTransaction transaction = provider.getUserManager().getTransaction(getId());
transaction.addUpdatedRequiredAction(requiredActionName);
}
}

View file

@ -27,17 +27,13 @@ import org.keycloak.models.UserModel;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.models.utils.UserModelDelegate;
import org.keycloak.models.utils.reflection.Property;
import org.keycloak.models.utils.reflection.PropertyCriteria;
import org.keycloak.models.utils.reflection.PropertyQueries;
import org.keycloak.storage.UserStorageProvider;
import org.keycloak.storage.ldap.LDAPStorageProvider;
import org.keycloak.storage.ldap.LDAPUtils;
import org.keycloak.storage.ldap.idm.model.LDAPObject;
import org.keycloak.storage.ldap.idm.query.Condition;
import org.keycloak.storage.ldap.idm.query.internal.LDAPQuery;
import org.keycloak.storage.ldap.idm.store.ldap.LDAPUtil;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@ -46,6 +42,7 @@ import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.keycloak.models.ModelException;
/**
@ -208,7 +205,7 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
@Override
public void removeAttribute(String name) {
if ( setLDAPAttribute(name, null)) {
if (setLDAPAttribute(name, null)) {
super.removeAttribute(name);
}
}
@ -246,7 +243,7 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
UserAttributeLDAPStorageMapper.logger.tracef("Pushing user attribute to LDAP. username: %s, Model attribute name: %s, LDAP attribute name: %s, Attribute value: %s", getUsername(), modelAttrName, ldapAttrName, value);
}
ensureTransactionStarted();
markUpdatedAttributeInTransaction(modelAttrName);
if (value == null) {
if (isMandatoryInLdap) {

View file

@ -241,7 +241,7 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
control.add(UserAccountControl.ACCOUNTDISABLE);
}
ensureTransactionStarted();
markUpdatedAttributeInTransaction(LDAPConstants.ENABLED);
updateUserAccountControl(false, ldapUser, control);
}
@ -266,7 +266,7 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
ldapUser.setSingleAttribute(LDAPConstants.PWD_LAST_SET, "0");
ensureTransactionStarted();
markUpdatedRequiredActionInTransaction(action);
}
}
@ -293,7 +293,7 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
ldapUser.setSingleAttribute(LDAPConstants.PWD_LAST_SET, "-1");
ensureTransactionStarted();
markUpdatedRequiredActionInTransaction(action);
}
}
}

View file

@ -119,7 +119,9 @@ public class LDAPConstants {
public static final String EMPTY_ATTRIBUTE_VALUE = " ";
public static final String EMPTY_MEMBER_ATTRIBUTE_VALUE = "cn=empty-membership-placeholder";
public static final String CUSTOM_ATTRIBUTE_ENABLED = "enabled";
public static final String ENABLED = "enabled";
public static final String CUSTOM_ATTRIBUTE_CREATE_DATE = "createDate";
public static final String CUSTOM_ATTRIBUTE_EXPIRY_DATE = "expiryDate";
public static final String ENTRY_UUID = "entryUUID";

View file

@ -22,113 +22,127 @@ import org.keycloak.models.UserModel;
import org.keycloak.storage.ReadOnlyException;
import java.util.List;
import java.util.function.Function;
/**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $
*/
public class ReadOnlyUserModelDelegate extends UserModelDelegate {
private final Function<String, RuntimeException> exceptionCreator;
public ReadOnlyUserModelDelegate(UserModel delegate) {
this(delegate, ReadOnlyException::new);
}
public ReadOnlyUserModelDelegate(UserModel delegate, Function<String, RuntimeException> exceptionCreator) {
super(delegate);
this.exceptionCreator = exceptionCreator;
}
@Override
public void setUsername(String username) {
throw new ReadOnlyException();
throw readOnlyException("username");
}
@Override
public void setEnabled(boolean enabled) {
throw new ReadOnlyException();
throw readOnlyException("enabled");
}
@Override
public void setSingleAttribute(String name, String value) {
throw new ReadOnlyException();
throw readOnlyException("attribute(" + name + ")");
}
@Override
public void setAttribute(String name, List<String> values) {
throw new ReadOnlyException();
throw readOnlyException("attribute(" + name + ")");
}
@Override
public void removeAttribute(String name) {
throw new ReadOnlyException();
throw readOnlyException("attribute(" + name + ")");
}
@Override
public void addRequiredAction(String action) {
throw new ReadOnlyException();
throw readOnlyException("required action " + action);
}
@Override
public void removeRequiredAction(String action) {
throw new ReadOnlyException();
throw readOnlyException("required action " + action);
}
@Override
public void addRequiredAction(RequiredAction action) {
throw new ReadOnlyException();
throw readOnlyException("required action " + action);
}
@Override
public void removeRequiredAction(RequiredAction action) {
throw new ReadOnlyException();
throw readOnlyException("required action " + action);
}
@Override
public void setFirstName(String firstName) {
throw new ReadOnlyException();
throw readOnlyException("firstName");
}
@Override
public void setLastName(String lastName) {
throw new ReadOnlyException();
throw readOnlyException("lastName");
}
@Override
public void setEmail(String email) {
throw new ReadOnlyException();
throw readOnlyException("email");
}
@Override
public void setEmailVerified(boolean verified) {
throw new ReadOnlyException();
throw readOnlyException("emailVerified");
}
@Override
public void deleteRoleMapping(RoleModel role) {
throw new ReadOnlyException();
throw readOnlyException("role mapping for role " + role.getName());
}
@Override
public void setFederationLink(String link) {
throw new ReadOnlyException();
throw readOnlyException("federationLink");
}
@Override
public void setServiceAccountClientLink(String clientInternalId) {
throw new ReadOnlyException();
throw readOnlyException("serviceAccountClientLink");
}
@Override
public void setCreatedTimestamp(Long timestamp) {
throw new ReadOnlyException();
throw readOnlyException("createdTimestamp");
}
@Override
public void joinGroup(GroupModel group) {
throw new ReadOnlyException();
throw readOnlyException("group mapping for group " + group.getName());
}
@Override
public void leaveGroup(GroupModel group) {
throw new ReadOnlyException();
throw readOnlyException("group mapping for group " + group.getName());
}
@Override
public void grantRole(RoleModel role) {
throw new ReadOnlyException();
throw readOnlyException("role mapping for role " + role.getName());
}
private RuntimeException readOnlyException(String detail) {
String message = String.format("Not possible to write '%s' when updating user '%s'", detail, getUsername());
return exceptionCreator.apply(message);
}
}

View file

@ -251,7 +251,7 @@ public class InMemoryUserAdapter implements UserModel {
@Override
public Set<GroupModel> getGroups() {
if (groupIds.isEmpty()) return Collections.emptySet();
if (groupIds.isEmpty()) return new HashSet<>();
Set<GroupModel> groups = new HashSet<>();
for (String id : groupIds) {
groups.add(realm.getGroupById(id));
@ -347,7 +347,7 @@ public class InMemoryUserAdapter implements UserModel {
@Override
public Set<RoleModel> getRoleMappings() {
if (roleIds.isEmpty()) return Collections.emptySet();
if (roleIds.isEmpty()) return new HashSet<>();
Set<RoleModel> roles = new HashSet<>();
for (String id : roleIds) {
roles.add(realm.getRoleById(id));

View file

@ -0,0 +1,185 @@
/*
* Copyright 2019 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.storage.adapter;
import java.util.List;
import org.keycloak.models.GroupModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.UserModelDelegate;
import static org.keycloak.common.util.ObjectUtil.isEqualOrBothNull;
/**
* This will perform update operation for particular attribute/property just if the existing value is not already same.
* In other words, just "real updates" will be passed to the delegate.
*
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class UpdateOnlyChangeUserModelDelegate extends UserModelDelegate {
public UpdateOnlyChangeUserModelDelegate(UserModel delegate) {
super(delegate);
}
@Override
public void setUsername(String username) {
if (!isEqualOrBothNull(getUsername(), username)) {
delegate.setUsername(username);
}
}
@Override
public void setEnabled(boolean enabled) {
if (!isEqualOrBothNull(isEnabled(), enabled)) {
delegate.setEnabled(enabled);
}
}
@Override
public void setSingleAttribute(String name, String value) {
if (!isEqualOrBothNull(getFirstAttribute(name), value)) {
delegate.setSingleAttribute(name, value);
}
}
@Override
public void setAttribute(String name, List<String> values) {
if (!isEqualOrBothNull(getAttribute(name), values)) {
delegate.setAttribute(name, values);
}
}
@Override
public void removeAttribute(String name) {
List<String> currentVal = getAttribute(name);
if (currentVal != null && !currentVal.isEmpty()) {
delegate.removeAttribute(name);
}
}
@Override
public void addRequiredAction(String action) {
if (!getRequiredActions().contains(action)) {
delegate.addRequiredAction(action);
}
}
@Override
public void removeRequiredAction(String action) {
if (getRequiredActions().contains(action)) {
delegate.removeRequiredAction(action);
}
}
@Override
public void addRequiredAction(RequiredAction action) {
String actionName = action.name();
addRequiredAction(actionName);
}
@Override
public void removeRequiredAction(RequiredAction action) {
String actionName = action.name();
removeRequiredAction(actionName);
}
@Override
public void setFirstName(String firstName) {
if (!isEqualOrBothNull(getFirstName(), firstName)) {
delegate.setFirstName(firstName);
}
}
@Override
public void setLastName(String lastName) {
if (!isEqualOrBothNull(getLastName(), lastName)) {
delegate.setLastName(lastName);
}
}
@Override
public void setEmail(String email) {
if (!isEqualOrBothNull(getEmail(), email)) {
delegate.setEmail(email);
}
}
@Override
public void setEmailVerified(boolean verified) {
if (!isEqualOrBothNull(isEmailVerified(), verified)) {
delegate.setEmailVerified(verified);
}
}
@Override
public void grantRole(RoleModel role) {
if (!hasRole(role)) {
delegate.grantRole(role);
}
}
@Override
public void deleteRoleMapping(RoleModel role) {
if (hasRole(role)) {
delegate.deleteRoleMapping(role);
}
}
@Override
public void setFederationLink(String link) {
if (!isEqualOrBothNull(getFederationLink(), link)) {
delegate.setFederationLink(link);
}
}
@Override
public void setServiceAccountClientLink(String clientInternalId) {
if (!isEqualOrBothNull(getServiceAccountClientLink(), clientInternalId)) {
delegate.setServiceAccountClientLink(clientInternalId);
}
}
@Override
public void setCreatedTimestamp(Long timestamp){
if (!isEqualOrBothNull(getCreatedTimestamp(), timestamp)) {
delegate.setCreatedTimestamp(timestamp);
}
}
@Override
public void joinGroup(GroupModel group) {
if (!isMemberOf(group)) {
delegate.joinGroup(group);
}
}
@Override
public void leaveGroup(GroupModel group) {
if (isMemberOf(group)) {
delegate.leaveGroup(group);
}
}
}

View file

@ -18,6 +18,8 @@ package org.keycloak.services.resources.admin;
import org.jboss.logging.Logger;
import org.jboss.resteasy.annotations.cache.NoCache;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.NotFoundException;
import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator;
import org.keycloak.events.admin.OperationType;
@ -31,6 +33,7 @@ import org.keycloak.models.RoleModel;
import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.idm.RoleRepresentation;
import org.keycloak.services.ErrorResponseException;
import org.keycloak.storage.ReadOnlyException;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
@ -168,6 +171,7 @@ public class ClientRoleMappingsResource {
public void addClientRoleMapping(List<RoleRepresentation> roles) {
managePermission.require();
try {
for (RoleRepresentation role : roles) {
RoleModel roleModel = client.getRole(role.getName());
if (roleModel == null || !roleModel.getId().equals(role.getId())) {
@ -176,6 +180,11 @@ public class ClientRoleMappingsResource {
auth.roles().requireMapRole(roleModel);
user.grantRole(roleModel);
}
} catch (ModelException | ReadOnlyException me) {
logger.warn(me.getMessage(), me);
throw new ErrorResponseException("invalid_request", "Could not add user role mappings!", Response.Status.BAD_REQUEST);
}
adminEvent.operation(OperationType.CREATE).resourcePath(uriInfo).representation(roles).success();
}
@ -214,10 +223,9 @@ public class ClientRoleMappingsResource {
auth.roles().requireMapRole(roleModel);
try {
user.deleteRoleMapping(roleModel);
} catch (ModelException me) {
Properties messages = AdminRoot.getMessages(session, realm, auth.adminAuth().getToken().getLocale());
throw new ErrorResponseException(me.getMessage(), MessageFormat.format(messages.getProperty(me.getMessage(), me.getMessage()), me.getParameters()),
Response.Status.BAD_REQUEST);
} catch (ModelException | ReadOnlyException me) {
logger.warn(me.getMessage(), me);
throw new ErrorResponseException("invalid_request", "Could not remove user role mappings!", Response.Status.BAD_REQUEST);
}
}
}

View file

@ -18,6 +18,8 @@ package org.keycloak.services.resources.admin;
import org.jboss.logging.Logger;
import org.jboss.resteasy.annotations.cache.NoCache;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.NotFoundException;
import org.keycloak.common.ClientConnection;
import org.keycloak.events.admin.OperationType;
@ -33,8 +35,10 @@ import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.idm.ClientMappingsRepresentation;
import org.keycloak.representations.idm.MappingsRepresentation;
import org.keycloak.representations.idm.RoleRepresentation;
import org.keycloak.services.ErrorResponse;
import org.keycloak.services.ErrorResponseException;
import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator;
import org.keycloak.storage.ReadOnlyException;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
@ -220,6 +224,7 @@ public class RoleMapperResource {
logger.debugv("** addRealmRoleMappings: {0}", roles);
try {
for (RoleRepresentation role : roles) {
RoleModel roleModel = realm.getRole(role.getName());
if (roleModel == null || !roleModel.getId().equals(role.getId())) {
@ -228,6 +233,10 @@ public class RoleMapperResource {
auth.roles().requireMapRole(roleModel);
roleMapper.grantRole(roleModel);
}
} catch (ModelException | ReadOnlyException me) {
logger.warn(me.getMessage(), me);
throw new ErrorResponseException("invalid_request", "Could not add user role mappings!", Response.Status.BAD_REQUEST);
}
adminEvent.operation(OperationType.CREATE).resourcePath(session.getContext().getUri()).representation(roles).success();
}
@ -263,10 +272,9 @@ public class RoleMapperResource {
auth.roles().requireMapRole(roleModel);
try {
roleMapper.deleteRoleMapping(roleModel);
} catch (ModelException me) {
Properties messages = AdminRoot.getMessages(session, realm, auth.adminAuth().getToken().getLocale());
throw new ErrorResponseException(me.getMessage(), MessageFormat.format(messages.getProperty(me.getMessage(), me.getMessage()), me.getParameters()),
Response.Status.BAD_REQUEST);
} catch (ModelException | ReadOnlyException me) {
logger.warn(me.getMessage(), me);
throw new ErrorResponseException("invalid_request", "Could not remove user role mappings!", Response.Status.BAD_REQUEST);
}
}

View file

@ -185,12 +185,12 @@ public class UserResource {
return ErrorResponse.exists("User is read only!");
} catch (ModelException me) {
logger.warn("Could not update user!", me);
return ErrorResponse.exists("Could not update user!");
return ErrorResponse.error("Could not update user!", Status.BAD_REQUEST);
} catch (ForbiddenException fe) {
throw fe;
} catch (Exception me) { // JPA
logger.warn("Could not update user!", me);// may be committed by JTA which can't
return ErrorResponse.exists("Could not update user!");
return ErrorResponse.error("Could not update user!", Status.BAD_REQUEST);
}
}

View file

@ -241,11 +241,8 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
LDAPTestUtils.updateGroupMapperConfigOptions(mapperModel, GroupMapperConfig.MODE, LDAPGroupMapperMode.READ_ONLY.toString());
appRealm.updateComponent(mapperModel);
UserModel mary = session.users().getUserByUsername("marykeycloak", appRealm);
GroupModel group1 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1");
GroupModel group11 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1/group11");
GroupModel group12 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1/group12");
// Add some group mappings directly into LDAP
GroupLDAPStorageMapper groupMapper = LDAPTestUtils.getGroupMapper(mapperModel, ctx.getLdapProvider(), appRealm);
@ -253,6 +250,17 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
LDAPObject maryLdap = ctx.getLdapProvider().loadLDAPUserByUsername(appRealm, "marykeycloak");
groupMapper.addGroupMappingInLDAP(appRealm, group1, maryLdap);
groupMapper.addGroupMappingInLDAP(appRealm, group11, maryLdap);
});
if (importEnabled) {
testingClient.server().run(session -> {
LDAPTestContext ctx = LDAPTestContext.init(session);
RealmModel appRealm = ctx.getRealm();
UserModel mary = session.users().getUserByUsername("marykeycloak", appRealm);
GroupModel group1 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1");
GroupModel group11 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1/group11");
GroupModel group12 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1/group12");
// Add some group mapping to model
mary.joinGroup(group12);
@ -279,6 +287,56 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
Set<GroupModel> maryGroupsWith12 = mary.getGroups("12", 0, 10);
Assert.assertEquals(2, maryGroupsWith12.size());
});
} else {
testingClient.server().run(session -> {
LDAPTestContext ctx = LDAPTestContext.init(session);
RealmModel appRealm = ctx.getRealm();
UserModel mary = session.users().getUserByUsername("marykeycloak", appRealm);
GroupModel group12 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1/group12");
// Add some group mapping to model. This should fail with no-import mode for LDAP provider READ_ONLY mode for the group mapper
// as it is not allowed to update group mappings in LDAP nor in the DB
try {
mary.joinGroup(group12);
Assert.fail("Not expected to successfully add group12 in no-import mode and READ_ONLY mode of the group mapper");
} catch (ModelException me) {
// Ignore
}
});
testingClient.server().run(session -> {
LDAPTestContext ctx = LDAPTestContext.init(session);
RealmModel appRealm = ctx.getRealm();
UserModel mary = session.users().getUserByUsername("marykeycloak", appRealm);
GroupModel group1 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1");
GroupModel group11 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1/group11");
GroupModel group12 = KeycloakModelUtils.findGroupByPath(appRealm, "/group1/group12");
// Assert that mary has both LDAP and DB mapped groups
Set<GroupModel> maryGroups = mary.getGroups();
Assert.assertEquals(4, maryGroups.size());
Assert.assertTrue(maryGroups.contains(group1));
Assert.assertTrue(maryGroups.contains(group11));
Assert.assertFalse(maryGroups.contains(group12));
long groupCount = mary.getGroupsCount();
Assert.assertEquals(4, groupCount);
Set<GroupModel> maryGroupsWithGr = mary.getGroups("gr", 0, 10);
Assert.assertEquals(4, maryGroupsWithGr.size());
Set<GroupModel> maryGroupsWithGr2 = mary.getGroups("gr", 1, 10);
Assert.assertEquals(3, maryGroupsWithGr2.size());
Set<GroupModel> maryGroupsWithGr3 = mary.getGroups("gr", 0, 1);
Assert.assertEquals(1, maryGroupsWithGr3.size());
Set<GroupModel> maryGroupsWith12 = mary.getGroups("12", 0, 10);
Assert.assertEquals(1, maryGroupsWith12.size());
});
}
// Assert that access through DB will have just DB mapped groups
if (importEnabled) {

View file

@ -17,6 +17,9 @@
package org.keycloak.testsuite.federation.ldap.noimport;
import java.util.Collections;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.core.Response;
import org.junit.Assert;
@ -24,6 +27,8 @@ import org.junit.FixMethodOrder;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runners.MethodSorters;
import org.keycloak.admin.client.resource.ComponentResource;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.component.ComponentModel;
import org.keycloak.models.LDAPConstants;
import org.keycloak.models.RealmModel;
@ -33,15 +38,18 @@ import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.idm.ComponentRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.storage.StorageId;
import org.keycloak.storage.UserStorageProvider;
import org.keycloak.storage.ldap.LDAPStorageProvider;
import org.keycloak.storage.ldap.mappers.FullNameLDAPStorageMapper;
import org.keycloak.storage.ldap.mappers.FullNameLDAPStorageMapperFactory;
import org.keycloak.storage.ldap.mappers.LDAPStorageMapper;
import org.keycloak.storage.ldap.mappers.UserAttributeLDAPStorageMapper;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest;
import org.keycloak.testsuite.federation.ldap.LDAPTestAsserts;
import org.keycloak.testsuite.federation.ldap.LDAPTestContext;
import org.keycloak.testsuite.util.LDAPTestUtils;
import org.keycloak.testsuite.util.WaitUtils;
/**
@ -155,8 +163,25 @@ public class LDAPProvidersIntegrationNoImportTest extends LDAPProvidersIntegrati
@Test
@Override
@Ignore // Unsynced mode doesn't have much sense in no-import
// Unsynced mode doesn't have much sense in no-import. So it is not allowed at the configuration level
public void testUnsynced() throws Exception {
ComponentResource ldapProviderResource = testRealm().components().component(ldapModelId);
ComponentRepresentation ldapProviderRep = ldapProviderResource.toRepresentation();
String currentEditMode = ldapProviderRep.getConfig().getFirst(LDAPConstants.EDIT_MODE);
Assert.assertEquals(UserStorageProvider.EditMode.WRITABLE.toString(), currentEditMode);
// Try update editMode to UNSYNCED. It should not work as UNSYNCED with no-import is not allowed
ldapProviderRep.getConfig().putSingle(LDAPConstants.EDIT_MODE, UserStorageProvider.EditMode.UNSYNCED.toString());
try {
ldapProviderResource.update(ldapProviderRep);
Assert.fail("Not expected to successfully update provider");
} catch (BadRequestException bre) {
// Expected
}
// Try to set editMode to WRITABLE should work
ldapProviderRep.getConfig().putSingle(LDAPConstants.EDIT_MODE, currentEditMode);
ldapProviderResource.update(ldapProviderRep);
}
@ -219,7 +244,6 @@ public class LDAPProvidersIntegrationNoImportTest extends LDAPProvidersIntegrati
fullnameUser.setLastName("Dee2");
});
// Assert changed user available in Keycloak, but his firstName is null (due the fullnameMapper is write-only and firstName mapper is removed)
testingClient.server().run(session -> {
LDAPTestContext ctx = LDAPTestContext.init(session);
@ -243,4 +267,67 @@ public class LDAPProvidersIntegrationNoImportTest extends LDAPProvidersIntegrati
response.close();
}
// Tests that attempt to change some user attributes, which are not mapped to LDAP, will fail
@Test
public void testImpossibleToChangeNonLDAPMappedAttributes() {
UserResource john = ApiUtil.findUserByUsernameId(testRealm(), "johnkeycloak");
UserRepresentation johnRep = john.toRepresentation();
String firstNameOrig = johnRep.getFirstName();
String lastNameOrig = johnRep.getLastName();
String postalCodeOrig = johnRep.getAttributes().get("postal_code").get(0);
try {
// Attempt to disable user should fail
try {
johnRep.setFirstName("John2");
johnRep.setLastName("Doe2");
johnRep.setEnabled(false);
john.update(johnRep);
Assert.fail("Not supposed to successfully update 'enabled' state of the user");
} catch (BadRequestException bre) {
// Expected
}
// Attempt to set requiredAction to the user should fail
try {
johnRep = john.toRepresentation();
johnRep.setRequiredActions(Collections.singletonList(UserModel.RequiredAction.CONFIGURE_TOTP.toString()));
john.update(johnRep);
Assert.fail("Not supposed to successfully add requiredAction to the user");
} catch (BadRequestException bre) {
// Expected
}
// Attempt to add some new attribute should fail
try {
johnRep = john.toRepresentation();
johnRep.singleAttribute("foo", "bar");
john.update(johnRep);
Assert.fail("Not supposed to successfully add attribute to the user");
} catch (BadRequestException bre) {
// Expected
}
// Attempt to update firstName, lastName and postal_code should be successful. All those attributes are mapped to LDAP
johnRep = john.toRepresentation();
johnRep.setFirstName("John2");
johnRep.setLastName("Doe2");
johnRep.singleAttribute("postal_code", "654321");
john.update(johnRep);
johnRep = john.toRepresentation();
Assert.assertEquals("John2", johnRep.getFirstName());
Assert.assertEquals("Doe2", johnRep.getLastName());
Assert.assertEquals("654321", johnRep.getAttributes().get("postal_code").get(0));
} finally {
// Revert
johnRep.setFirstName(firstNameOrig);
johnRep.setLastName(lastNameOrig);
johnRep.singleAttribute("postal_code", postalCodeOrig);
john.update(johnRep);
}
}
}

View file

@ -304,7 +304,7 @@ public class ServiceAccountTest extends AbstractKeycloakTest {
representation.setCredentials(Arrays.asList(password));
this.expectedException.expect(Matchers.allOf(Matchers.instanceOf(ClientErrorException.class),
Matchers.hasProperty("response", Matchers.hasProperty("status", Matchers.is(409)))));
Matchers.hasProperty("response", Matchers.hasProperty("status", Matchers.is(400)))));
this.expectedException.reportMissingExceptionWithMessage("Should fail, should not be possible to manage credentials for service accounts");
serviceAccount.update(representation);

View file

@ -17,6 +17,7 @@ ldapErrorCantPreserveGroupInheritanceWithUIDMembershipType=Not possible to prese
ldapErrorCantWriteOnlyForReadOnlyLdap=Can not set write only when LDAP provider mode is not WRITABLE
ldapErrorCantWriteOnlyAndReadOnly=Can not set write-only and read-only together
ldapErrorCantEnableStartTlsAndConnectionPooling=Can not enable both StartTLS and connection pooling.
ldapErrorCantEnableUnsyncedAndImportOff=Can not disable Importing users when LDAP provider mode is UNSYNCED
clientRedirectURIsFragmentError=Redirect URIs must not contain an URI fragment
clientRootURLFragmentError=Root URL must not contain an URL fragment