From: Mathieu Baudier Date: Mon, 21 Sep 2015 12:22:59 +0000 (+0000) Subject: Fix issue with propagating user removal. X-Git-Tag: argeo-commons-2.1.30~132 X-Git-Url: http://git.argeo.org/?a=commitdiff_plain;h=268e023a9de5db2549431a4415e584ac68a4f98b;p=lgpl%2Fargeo-commons.git Fix issue with propagating user removal. git-svn-id: https://svn.argeo.org/commons/trunk@8425 4cfe0d0a-d680-48aa-b62c-e0a02a3f76cc --- diff --git a/org.argeo.cms/src/org/argeo/cms/internal/kernel/NodeUserAdmin.java b/org.argeo.cms/src/org/argeo/cms/internal/kernel/NodeUserAdmin.java index 416ecef6b..bd48bc385 100644 --- a/org.argeo.cms/src/org/argeo/cms/internal/kernel/NodeUserAdmin.java +++ b/org.argeo.cms/src/org/argeo/cms/internal/kernel/NodeUserAdmin.java @@ -122,8 +122,8 @@ public class NodeUserAdmin implements UserAdmin { Dictionary nodeRolesProperties = UserAdminConf .uriAsProperties(nodeRolesUri); - if (!nodeRolesProperties.get(UserAdminConf.baseDn.property()) - .equals(baseNodeRoleDn)) { + if (!nodeRolesProperties.get(UserAdminConf.baseDn.property()).equals( + baseNodeRoleDn)) { throw new CmsException("Invalid base dn for node roles"); // TODO deal with "mounted" roles with a different baseDN } @@ -135,7 +135,7 @@ public class NodeUserAdmin implements UserAdmin { } nodeRoles.setExternalRoles(this); nodeRoles.init(); - addUserAdmin(baseNodeRoleDn, (UserAdmin)nodeRoles); + addUserAdmin(baseNodeRoleDn, (UserAdmin) nodeRoles); if (log.isTraceEnabled()) log.trace("Node roles enabled."); } @@ -175,7 +175,9 @@ public class NodeUserAdmin implements UserAdmin { @Override public boolean removeRole(String name) { - return findUserAdmin(name).removeRole(name); + boolean actuallyDeleted = findUserAdmin(name).removeRole(name); + nodeRoles.removeRole(name); + return actuallyDeleted; } @Override diff --git a/org.argeo.security.core/src/org/argeo/osgi/useradmin/AbstractUserDirectory.java b/org.argeo.security.core/src/org/argeo/osgi/useradmin/AbstractUserDirectory.java index 40b58be74..884e4ce09 100644 --- a/org.argeo.security.core/src/org/argeo/osgi/useradmin/AbstractUserDirectory.java +++ b/org.argeo.security.core/src/org/argeo/osgi/useradmin/AbstractUserDirectory.java @@ -36,11 +36,11 @@ import org.osgi.framework.Filter; import org.osgi.framework.FrameworkUtil; import org.osgi.framework.InvalidSyntaxException; import org.osgi.service.useradmin.Authorization; -import org.osgi.service.useradmin.Group; import org.osgi.service.useradmin.Role; import org.osgi.service.useradmin.User; import org.osgi.service.useradmin.UserAdmin; +/** Base class for a {@link UserDirectory}. */ abstract class AbstractUserDirectory implements UserAdmin, UserDirectory { private final static Log log = LogFactory .getLog(AbstractUserDirectory.class); @@ -61,9 +61,6 @@ abstract class AbstractUserDirectory implements UserAdmin, UserDirectory { private List credentialAttributeIds = Arrays .asList(new String[] { LdifName.userpassword.name() }); - // private TransactionSynchronizationRegistry syncRegistry; - // private Object editingTransactionKey = null; - private TransactionManager transactionManager; private ThreadLocal workingCopy = new ThreadLocal(); private Xid editingTransactionXid = null; @@ -95,13 +92,8 @@ abstract class AbstractUserDirectory implements UserAdmin, UserDirectory { .getValue(properties); } - // public AbstractUserDirectory(URI uri, boolean isReadOnly) { - // this.uri = uri; - // this.isReadOnly = isReadOnly; - // } - - /** Returns the {@link Group}s this user is a direct member of. */ - protected abstract List getDirectGroups(User user); + /** Returns the groups this user is a direct member of. */ + protected abstract List getDirectGroups(LdapName dn); protected abstract Boolean daoHasRole(LdapName dn); @@ -121,10 +113,6 @@ abstract class AbstractUserDirectory implements UserAdmin, UserDirectory { if (editingTransactionXid == null) return false; return workingCopy.get() != null; - // Object currentTrKey = syncRegistry.getTransactionKey(); - // if (currentTrKey == null) - // return false; - // return editingTransactionKey.equals(currentTrKey); } protected WorkingCopy getWorkingCopy() { @@ -168,7 +156,7 @@ abstract class AbstractUserDirectory implements UserAdmin, UserDirectory { } } - List getAllRoles(User user) { + List getAllRoles(DirectoryUser user) { List allRoles = new ArrayList(); if (user != null) { collectRoles(user, allRoles); @@ -178,9 +166,10 @@ abstract class AbstractUserDirectory implements UserAdmin, UserDirectory { return allRoles; } - private void collectRoles(User user, List allRoles) { - for (Group group : getDirectGroups(user)) { + private void collectRoles(DirectoryUser user, List allRoles) { + for (LdapName groupDn : getDirectGroups(user.getDn())) { // TODO check for loops + DirectoryUser group = doGetRole(groupDn); allRoles.add(group); collectRoles(group, allRoles); } @@ -193,13 +182,16 @@ abstract class AbstractUserDirectory implements UserAdmin, UserDirectory { // USER ADMIN @Override public Role getRole(String name) { - LdapName key = toDn(name); + return doGetRole(toDn(name)); + } + + protected DirectoryUser doGetRole(LdapName dn) { WorkingCopy wc = getWorkingCopy(); - DirectoryUser user = daoGetRole(key); + DirectoryUser user = daoGetRole(dn); if (wc != null) { - if (user == null && wc.getNewUsers().containsKey(key)) - user = wc.getNewUsers().get(key); - else if (wc.getDeletedUsers().containsKey(key)) + if (user == null && wc.getNewUsers().containsKey(dn)) + user = wc.getNewUsers().get(dn); + else if (wc.getDeletedUsers().containsKey(dn)) user = null; } return user; @@ -327,16 +319,20 @@ abstract class AbstractUserDirectory implements UserAdmin, UserDirectory { checkEdit(); WorkingCopy wc = getWorkingCopy(); LdapName dn = toDn(name); - if (!daoHasRole(dn) && !wc.getNewUsers().containsKey(dn)) - return false; - DirectoryUser user = (DirectoryUser) getRole(name); - wc.getDeletedUsers().put(dn, user); - // FIXME clarify directgroups - for (DirectoryGroup group : getDirectGroups(user)) { + boolean actuallyDeleted; + if (daoHasRole(dn) || wc.getNewUsers().containsKey(dn)) { + DirectoryUser user = (DirectoryUser) getRole(name); + wc.getDeletedUsers().put(dn, user); + actuallyDeleted = true; + } else {// just removing from groups (e.g. system roles) + actuallyDeleted = false; + } + for (LdapName groupDn : getDirectGroups(dn)) { + DirectoryUser group = doGetRole(groupDn); group.getAttributes().get(getMemberAttributeId()) .remove(dn.toString()); } - return true; + return actuallyDeleted; } // TRANSACTION diff --git a/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdapUserAdmin.java b/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdapUserAdmin.java index 0c6d29376..ef212fa27 100644 --- a/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdapUserAdmin.java +++ b/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdapUserAdmin.java @@ -24,7 +24,6 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.argeo.ArgeoException; import org.osgi.framework.Filter; -import org.osgi.service.useradmin.User; /** * A user admin based on a LDAP server. Requires a {@link TransactionManager} @@ -157,12 +156,12 @@ public class LdapUserAdmin extends AbstractUserDirectory { } @Override - protected List getDirectGroups(User user) { - List directGroups = new ArrayList(); + protected List getDirectGroups(LdapName dn) { + List directGroups = new ArrayList(); try { String searchFilter = "(&(" + objectClass + "=" + getGroupObjectClass() + ")(" + getMemberAttributeId() - + "=" + user.getName() + "))"; + + "=" + dn + "))"; SearchControls searchControls = new SearchControls(); searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE); @@ -174,14 +173,12 @@ public class LdapUserAdmin extends AbstractUserDirectory { while (results.hasMoreElements()) { SearchResult searchResult = (SearchResult) results .nextElement(); - LdifGroup group = new LdifGroup(this, toDn(searchBase, - searchResult), searchResult.getAttributes()); - directGroups.add(group); + directGroups.add(toDn(searchBase, searchResult)); } return directGroups; } catch (Exception e) { - throw new ArgeoException("Cannot populate direct members of " - + user, e); + throw new ArgeoException("Cannot populate direct members of " + dn, + e); } } diff --git a/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifUserAdmin.java b/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifUserAdmin.java index 0f0c3b657..a03a25f09 100644 --- a/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifUserAdmin.java +++ b/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifUserAdmin.java @@ -12,7 +12,6 @@ import java.util.List; import java.util.SortedMap; import java.util.TreeMap; -import javax.naming.InvalidNameException; import javax.naming.NamingEnumeration; import javax.naming.directory.Attributes; import javax.naming.ldap.LdapName; @@ -21,7 +20,6 @@ import javax.transaction.TransactionManager; import org.apache.commons.io.IOUtils; import org.osgi.framework.Filter; import org.osgi.service.useradmin.Role; -import org.osgi.service.useradmin.User; /** * A user admin based on a LDIF files. Requires a {@link TransactionManager} and @@ -155,23 +153,12 @@ public class LdifUserAdmin extends AbstractUserDirectory { } @Override - protected List getDirectGroups(User user) { - LdapName dn; - if (user instanceof LdifUser) - dn = ((LdifUser) user).getDn(); - else - try { - dn = new LdapName(user.getName()); - } catch (InvalidNameException e) { - throw new UserDirectoryException("Badly formatted user name " - + user.getName(), e); - } - - List directGroups = new ArrayList(); + protected List getDirectGroups(LdapName dn) { + List directGroups = new ArrayList(); for (LdapName name : groups.keySet()) { DirectoryGroup group = groups.get(name); if (group.getMemberNames().contains(dn)) - directGroups.add(group); + directGroups.add(group.getDn()); } return directGroups; }