Better protect access to Jackrabbit user manager

git-svn-id: https://svn.argeo.org/commons/trunk@5686 4cfe0d0a-d680-48aa-b62c-e0a02a3f76cc

diff --git a/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/OsJcrAuthenticationProvider.java b/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/OsJcrAuthenticationProvider.java
index 6c26d4627ec3ce291d03e7b3d1a5cb629f6f93cf..0e8213730ebcdee66823274d911f9732d63b477d 100644 (file)
--- a/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/OsJcrAuthenticationProvider.java
+++ b/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/OsJcrAuthenticationProvider.java
@@ -58,7 +58,7 @@ public class OsJcrAuthenticationProvider extends OsAuthenticationProvider {
                        throws AuthenticationException {
                if (authentication instanceof UsernamePasswordAuthenticationToken) {
                        // deal with remote access to internal server
-                       // FIXME very primitive and unsecure at this stage
+                       // FIXME very primitive and unsecure at this sSession adminSession =tage
                        // consider using the keyring for username / password authentication
                        // or certificate
                        UsernamePasswordAuthenticationToken upat = (UsernamePasswordAuthenticationToken) authentication;

diff --git a/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/OsJcrUserAdminService.java b/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/OsJcrUserAdminService.java
index 44521f1bd8aa9df32a04caecbf05153d09139451..37dca4b1ebaf1f59624cd3ad53cd103e2e529c51 100644 (file)
--- a/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/OsJcrUserAdminService.java
+++ b/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/OsJcrUserAdminService.java
@@ -21,21 +21,20 @@ import org.springframework.security.userdetails.UsernameNotFoundException;
  * desktop). TODO integrate with JCR user / groups
  */
 public class OsJcrUserAdminService implements UserAdminService {
-       private String securityWorkspace = "security";
        private Repository repository;
 
-       private Session securitySession;
+       // private Session adminSession;
 
        public void init() {
-               try {
-                       securitySession = repository.login(securityWorkspace);
-               } catch (RepositoryException e) {
-                       throw new ArgeoException("Cannot initialize", e);
-               }
+               // try {
+               // adminSession = repository.login();
+               // } catch (RepositoryException e) {
+               // throw new ArgeoException("Cannot initialize", e);
+               // }
        }
 
        public void destroy() {
-               JcrUtils.logoutQuietly(securitySession);
+               // JcrUtils.logoutQuietly(adminSession);
        }
 
        /** <b>Unsupported</b> */
@@ -68,15 +67,19 @@ public class OsJcrUserAdminService implements UserAdminService {
        public UserDetails loadUserByUsername(String username)
                        throws UsernameNotFoundException, DataAccessException {
                if (getSPropertyUsername().equals(username)) {
-                       Node userProfile = UserJcrUtils.getUserProfile(securitySession,
-                                       username);
                        JcrUserDetails userDetails;
+                       Session adminSession = null;
                        try {
+                               adminSession = repository.login();
+                               Node userProfile = UserJcrUtils.getUserProfile(adminSession,
+                                               username);
                                userDetails = new JcrUserDetails(userProfile, "",
                                                OsJcrAuthenticationProvider.getBaseAuthorities());
                        } catch (RepositoryException e) {
                                throw new ArgeoException("Cannot retrieve user profile for "
                                                + username, e);
+                       } finally {
+                               JcrUtils.logoutQuietly(adminSession);
                        }
                        return userDetails;
                } else {
@@ -122,9 +125,4 @@ public class OsJcrUserAdminService implements UserAdminService {
        public void setRepository(Repository repository) {
                this.repository = repository;
        }
-
-       public void setSecurityWorkspace(String securityWorkspace) {
-               this.securityWorkspace = securityWorkspace;
-       }
-
 }

diff --git a/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/SimpleJcrSecurityModel.java b/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/SimpleJcrSecurityModel.java
index e564e9fd08380fc7179c03f4cf27eac3ca0be07a..aa2ad4915ea242a2d941a4def8c66e1a828981e9 100644 (file)
--- a/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/SimpleJcrSecurityModel.java
+++ b/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/jcr/SimpleJcrSecurityModel.java
@@ -31,7 +31,8 @@ public class SimpleJcrSecurityModel implements JcrSecurityModel {
        /** The home base path. */
        private String homeBasePath = "/home";
 
-       public Node sync(Session session, String username, List<String> roles) {
+       public synchronized Node sync(Session session, String username,
+                       List<String> roles) {
                // TODO check user name validity (e.g. should not start by ROLE_)
 
                try {
@@ -57,7 +58,7 @@ public class SimpleJcrSecurityModel implements JcrSecurityModel {
 
                        // Remote roles
                        if (roles != null) {
-                               //writeRemoteRoles(userHome, roles);
+                               // writeRemoteRoles(userHome, roles);
                        }
 
                        Node userProfile = UserJcrUtils.getUserProfile(session, username);

diff --git a/security/runtime/org.argeo.security.jackrabbit/src/main/java/org/argeo/security/jackrabbit/ArgeoSecurityManager.java b/security/runtime/org.argeo.security.jackrabbit/src/main/java/org/argeo/security/jackrabbit/ArgeoSecurityManager.java
index 313fdca93ae9768801783034b595b82fefdb39c1..dd37e4816a03602803d926bfdcd0ba54eb3367e2 100644 (file)
--- a/security/runtime/org.argeo.security.jackrabbit/src/main/java/org/argeo/security/jackrabbit/ArgeoSecurityManager.java
+++ b/security/runtime/org.argeo.security.jackrabbit/src/main/java/org/argeo/security/jackrabbit/ArgeoSecurityManager.java
@@ -34,6 +34,8 @@ import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.api.security.user.UserManager;
 import org.apache.jackrabbit.core.DefaultSecurityManager;
+import org.apache.jackrabbit.core.security.AMContext;
+import org.apache.jackrabbit.core.security.AccessManager;
 import org.apache.jackrabbit.core.security.AnonymousPrincipal;
 import org.apache.jackrabbit.core.security.SecurityConstants;
 import org.apache.jackrabbit.core.security.authorization.WorkspaceAccessManager;
@@ -55,6 +57,22 @@ public class ArgeoSecurityManager extends DefaultSecurityManager {
        private Map<String, String> userRolesCache = Collections
                        .synchronizedMap(new HashMap<String, String>());
 
+       @Override
+       public AccessManager getAccessManager(Session session, AMContext amContext)
+                       throws RepositoryException {
+               synchronized (getSystemSession()) {
+                       return super.getAccessManager(session, amContext);
+               }
+       }
+
+       @Override
+       public UserManager getUserManager(Session session)
+                       throws RepositoryException {
+               synchronized (getSystemSession()) {
+                       return super.getUserManager(session);
+               }
+       }
+
        /**
         * Since this is called once when the session is created, we take the
         * opportunity to make sure that Jackrabbit users and groups reflect Spring

diff --git a/security/runtime/org.argeo.security.jackrabbit/src/main/java/org/argeo/security/jackrabbit/JackrabbitSecurityModel.java b/security/runtime/org.argeo.security.jackrabbit/src/main/java/org/argeo/security/jackrabbit/JackrabbitSecurityModel.java
index 25de2bea568d977b0f351ce2364f78f2ccb0666b..16dd2981d1be56fd39f6404873658fe40e1c9459 100644 (file)
--- a/security/runtime/org.argeo.security.jackrabbit/src/main/java/org/argeo/security/jackrabbit/JackrabbitSecurityModel.java
+++ b/security/runtime/org.argeo.security.jackrabbit/src/main/java/org/argeo/security/jackrabbit/JackrabbitSecurityModel.java
@@ -24,7 +24,8 @@ public class JackrabbitSecurityModel extends SimpleJcrSecurityModel {
                        .getLog(JackrabbitSecurityModel.class);
 
        @Override
-       public Node sync(Session session, String username, List<String> roles) {
+       public synchronized Node sync(Session session, String username,
+                       List<String> roles) {
                if (!(session instanceof JackrabbitSession))
                        return super.sync(session, username, roles);
 

diff --git a/server/runtime/org.argeo.server.jackrabbit/src/main/java/org/argeo/jackrabbit/JackrabbitAuthorizations.java b/server/runtime/org.argeo.server.jackrabbit/src/main/java/org/argeo/jackrabbit/JackrabbitAuthorizations.java
index a3cf4e1498509f00100c10ad7e5fd94ac3bc17ae..59cbe2af86786897ff6077ecdbf838fa1e6b8a45 100644 (file)
--- a/server/runtime/org.argeo.server.jackrabbit/src/main/java/org/argeo/jackrabbit/JackrabbitAuthorizations.java
+++ b/server/runtime/org.argeo.server.jackrabbit/src/main/java/org/argeo/jackrabbit/JackrabbitAuthorizations.java
@@ -42,20 +42,22 @@ public class JackrabbitAuthorizations extends JcrAuthorizations {
        protected Principal getOrCreatePrincipal(Session session,
                        String principalName) throws RepositoryException {
                UserManager um = ((JackrabbitSession) session).getUserManager();
-               Authorizable authorizable = um.getAuthorizable(principalName);
-               if (authorizable == null) {
-                       groupPrefixes: for (String groupPrefix : groupPrefixes) {
-                               if (principalName.startsWith(groupPrefix)) {
-                                       authorizable = um.createGroup(principalName);
-                                       log.info("Created group " + principalName);
-                                       break groupPrefixes;
+               synchronized (um) {
+                       Authorizable authorizable = um.getAuthorizable(principalName);
+                       if (authorizable == null) {
+                               groupPrefixes: for (String groupPrefix : groupPrefixes) {
+                                       if (principalName.startsWith(groupPrefix)) {
+                                               authorizable = um.createGroup(principalName);
+                                               log.info("Created group " + principalName);
+                                               break groupPrefixes;
+                                       }
                                }
+                               if (authorizable == null)
+                                       throw new ArgeoException("Authorizable " + principalName
+                                                       + " not found");
                        }
-                       if (authorizable == null)
-                               throw new ArgeoException("Authorizable " + principalName
-                                               + " not found");
+                       return authorizable.getPrincipal();
                }
-               return authorizable.getPrincipal();
        }
 
        public void setGroupPrefixes(List<String> groupsToCreate) {

diff --git a/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/JcrUtils.java b/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/JcrUtils.java
index fdb82cb85f2bb054e44a39feb95472b2d4b542e9..9f3d761cafd79e8bdb51a7ae481fc9cfb32c847e 100644 (file)
--- a/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/JcrUtils.java
+++ b/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/JcrUtils.java
@@ -1229,7 +1229,7 @@ public class JcrUtils implements ArgeoJcrConstants {
         * Convenience method for adding a single privilege to a principal (user or
         * role), typically jcr:all
         */
-       public static void addPrivilege(Session session, String path,
+       public synchronized static void addPrivilege(Session session, String path,
                        String principal, String privilege) throws RepositoryException {
                List<Privilege> privileges = new ArrayList<Privilege>();
                privileges.add(session.getAccessControlManager().privilegeFromName(
@@ -1264,7 +1264,7 @@ public class JcrUtils implements ArgeoJcrConstants {
        }
 
        /** Gets access control list for this path, throws exception if not found */
-       public static AccessControlList getAccessControlList(
+       public synchronized static AccessControlList getAccessControlList(
                        AccessControlManager acm, String path) throws RepositoryException {
                // search for an access control list
                AccessControlList acl = null;
@@ -1291,8 +1291,8 @@ public class JcrUtils implements ArgeoJcrConstants {
        }
 
        /** Clear authorizations for a user at this path */
-       public static void clearAccessControList(Session session, String path,
-                       String username) throws RepositoryException {
+       public synchronized static void clearAccessControList(Session session,
+                       String path, String username) throws RepositoryException {
                AccessControlManager acm = session.getAccessControlManager();
                AccessControlList acl = getAccessControlList(acm, path);
                for (AccessControlEntry ace : acl.getAccessControlEntries()) {

diff --git a/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/UserJcrUtils.java b/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/UserJcrUtils.java
index b357227a570a62d42ad36fd71d5b633519b45ba4..b7b138eadfb5dfd9e43a7d917c31263a55383fa1 100644 (file)
--- a/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/UserJcrUtils.java
+++ b/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/UserJcrUtils.java
@@ -65,7 +65,8 @@ public class UserJcrUtils {
                        Query query = qomf.createQuery(userHomeSel, constraint, null, null);
                        return JcrUtils.querySingleNode(query);
                } catch (RepositoryException e) {
-                       throw new ArgeoException("Cannot find home for user " + username, e);
+                       throw new ArgeoException(
+                                       "Cannot find profile for user " + username, e);
                }
        }