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 416ecef6ba9942413511b595b504c0f5e449c691..bd48bc38501f40aa21c99497a8482839946c99f8 100644 (file)
--- 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<String, ?> 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 40b58be7433fdbf2a9b879ae63aae5035978f625..884e4ce09ff0ccd1e550f01fe3c402dadae12608 100644 (file)
--- 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<String> credentialAttributeIds = Arrays
                        .asList(new String[] { LdifName.userpassword.name() });
 
-       // private TransactionSynchronizationRegistry syncRegistry;
-       // private Object editingTransactionKey = null;
-
        private TransactionManager transactionManager;
        private ThreadLocal<WorkingCopy> workingCopy = new ThreadLocal<AbstractUserDirectory.WorkingCopy>();
        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<? extends DirectoryGroup> getDirectGroups(User user);
+       /** Returns the groups this user is a direct member of. */
+       protected abstract List<LdapName> 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<Role> getAllRoles(User user) {
+       List<Role> getAllRoles(DirectoryUser user) {
                List<Role> allRoles = new ArrayList<Role>();
                if (user != null) {
                        collectRoles(user, allRoles);
@@ -178,9 +166,10 @@ abstract class AbstractUserDirectory implements UserAdmin, UserDirectory {
                return allRoles;
        }
 
-       private void collectRoles(User user, List<Role> allRoles) {
-               for (Group group : getDirectGroups(user)) {
+       private void collectRoles(DirectoryUser user, List<Role> 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 0c6d2937649431414be31b615d91b04b8837c4eb..ef212fa27a0160629a592af1c578630699941fbc 100644 (file)
--- 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<DirectoryGroup> getDirectGroups(User user) {
-               List<DirectoryGroup> directGroups = new ArrayList<DirectoryGroup>();
+       protected List<LdapName> getDirectGroups(LdapName dn) {
+               List<LdapName> directGroups = new ArrayList<LdapName>();
                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 0f0c3b657a10ac4c9f8e3b2a9dad76499f640c10..a03a25f09e5ce58fc21208f9817608d75df54fdb 100644 (file)
--- 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<DirectoryGroup> 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<DirectoryGroup> directGroups = new ArrayList<DirectoryGroup>();
+       protected List<LdapName> getDirectGroups(LdapName dn) {
+               List<LdapName> directGroups = new ArrayList<LdapName>();
                for (LdapName name : groups.keySet()) {
                        DirectoryGroup group = groups.get(name);
                        if (group.getMemberNames().contains(dn))
-                               directGroups.add(group);
+                               directGroups.add(group.getDn());
                }
                return directGroups;
        }