From e499eba6852f5cc487ade0db65e6f9cfc4c5d45a Mon Sep 17 00:00:00 2001 From: Mathieu Baudier Date: Thu, 1 Nov 2012 15:40:15 +0000 Subject: [PATCH] Better protect access to Jackrabbit user manager git-svn-id: https://svn.argeo.org/commons/trunk@5686 4cfe0d0a-d680-48aa-b62c-e0a02a3f76cc --- .../jcr/OsJcrAuthenticationProvider.java | 2 +- .../security/jcr/OsJcrUserAdminService.java | 28 +++++++++---------- .../security/jcr/SimpleJcrSecurityModel.java | 5 ++-- .../jackrabbit/ArgeoSecurityManager.java | 18 ++++++++++++ .../jackrabbit/JackrabbitSecurityModel.java | 3 +- .../jackrabbit/JackrabbitAuthorizations.java | 24 ++++++++-------- .../src/main/java/org/argeo/jcr/JcrUtils.java | 8 +++--- .../main/java/org/argeo/jcr/UserJcrUtils.java | 3 +- 8 files changed, 56 insertions(+), 35 deletions(-) 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 6c26d4627..0e8213730 100644 --- 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 44521f1bd..37dca4b1e 100644 --- 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); } /** Unsupported */ @@ -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 e564e9fd0..aa2ad4915 100644 --- 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 roles) { + public synchronized Node sync(Session session, String username, + List 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 313fdca93..dd37e4816 100644 --- 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 userRolesCache = Collections .synchronizedMap(new HashMap()); + @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 25de2bea5..16dd2981d 100644 --- 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 roles) { + public synchronized Node sync(Session session, String username, + List 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 a3cf4e149..59cbe2af8 100644 --- 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 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 fdb82cb85..9f3d761ca 100644 --- 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 privileges = new ArrayList(); 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 b357227a5..b7b138ead 100644 --- 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); } } -- 2.30.2