From: Mathieu Baudier <mbaudier@argeo.org>
Date: Tue, 25 Oct 2016 11:15:29 +0000 (+0000)
Subject: Properly deal with Jackrabbit system sessions
X-Git-Tag: argeo-commons-2.1.50~11
X-Git-Url: http://git.argeo.org/?a=commitdiff_plain;h=9d0201c5b057929fb029f484f082d0e011e15673;p=lgpl%2Fargeo-commons.git

Properly deal with Jackrabbit system sessions

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

diff --git a/org.argeo.cms/src/org/argeo/cms/auth/DataAdminLoginModule.java b/org.argeo.cms/src/org/argeo/cms/auth/DataAdminLoginModule.java
index 215ed96b4..50a878834 100644
--- a/org.argeo.cms/src/org/argeo/cms/auth/DataAdminLoginModule.java
+++ b/org.argeo.cms/src/org/argeo/cms/auth/DataAdminLoginModule.java
@@ -9,12 +9,13 @@ import javax.security.auth.spi.LoginModule;
 
 import org.argeo.node.security.DataAdminPrincipal;
 
+/** Logs a system process as data admin */
 public class DataAdminLoginModule implements LoginModule {
 	private Subject subject;
 
 	@Override
-	public void initialize(Subject subject, CallbackHandler callbackHandler,
-			Map<String, ?> sharedState, Map<String, ?> options) {
+	public void initialize(Subject subject, CallbackHandler callbackHandler, Map<String, ?> sharedState,
+			Map<String, ?> options) {
 		this.subject = subject;
 	}
 
@@ -37,9 +38,7 @@ public class DataAdminLoginModule implements LoginModule {
 
 	@Override
 	public boolean logout() throws LoginException {
-		// remove ALL credentials (e.g. additional Jackrabbit credentials)
-		subject.getPrincipals().clear();
+		subject.getPrincipals().removeAll(subject.getPrincipals(DataAdminPrincipal.class));
 		return true;
 	}
-
 }
diff --git a/org.argeo.cms/src/org/argeo/cms/internal/kernel/HomeRepository.java b/org.argeo.cms/src/org/argeo/cms/internal/kernel/HomeRepository.java
index d73b554e8..97407866f 100644
--- a/org.argeo.cms/src/org/argeo/cms/internal/kernel/HomeRepository.java
+++ b/org.argeo.cms/src/org/argeo/cms/internal/kernel/HomeRepository.java
@@ -47,7 +47,8 @@ class HomeRepository extends JcrRepositoryWrapper implements KernelConstants {
 			@Override
 			public Void run() {
 				try {
-					initJcr(getRepository().login());
+					Session adminSession = getRepository().login();
+					initJcr(adminSession);
 				} catch (RepositoryException e) {
 					throw new CmsException("Cannot init JCR home", e);
 				}
diff --git a/org.argeo.ext.jackrabbit/src/org/argeo/security/jackrabbit/ArgeoSecurityManager.java b/org.argeo.ext.jackrabbit/src/org/argeo/security/jackrabbit/ArgeoSecurityManager.java
index a1fe94cd8..fde490d2e 100644
--- a/org.argeo.ext.jackrabbit/src/org/argeo/security/jackrabbit/ArgeoSecurityManager.java
+++ b/org.argeo.ext.jackrabbit/src/org/argeo/security/jackrabbit/ArgeoSecurityManager.java
@@ -28,12 +28,14 @@ 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.SecurityConstants;
+import org.apache.jackrabbit.core.security.SystemPrincipal;
 import org.apache.jackrabbit.core.security.authorization.WorkspaceAccessManager;
+import org.apache.jackrabbit.core.security.principal.AdminPrincipal;
 import org.argeo.node.NodeConstants;
 import org.argeo.node.security.AnonymousPrincipal;
 import org.argeo.node.security.DataAdminPrincipal;
 
-/** Integrates Spring Security and Jackrabbit Security users and roles. */
+/** Customises Jackrabbit security. */
 public class ArgeoSecurityManager extends DefaultSecurityManager {
 	@Override
 	public AccessManager getAccessManager(Session session, AMContext amContext) throws RepositoryException {
@@ -49,37 +51,46 @@ public class ArgeoSecurityManager extends DefaultSecurityManager {
 		}
 	}
 
-	/**
-	 * Since this is called once when the session is created, we take the
-	 * opportunity to make sure that Jackrabbit users and groups reflect Spring
-	 * Security name and authorities.
-	 */
+	/** Called once when the session is created */
 	@Override
 	public String getUserID(Subject subject, String workspaceName) throws RepositoryException {
-		Set<AnonymousPrincipal> anonymousPrincipal = subject.getPrincipals(AnonymousPrincipal.class);
-		if (!anonymousPrincipal.isEmpty())
-			return NodeConstants.ROLE_ANONYMOUS;
+		boolean isAnonymous = !subject.getPrincipals(AnonymousPrincipal.class).isEmpty();
+		boolean isDataAdmin = !subject.getPrincipals(DataAdminPrincipal.class).isEmpty();
+		boolean isJackrabbitSystem = !subject.getPrincipals(SystemPrincipal.class).isEmpty();
 		Set<X500Principal> userPrincipal = subject.getPrincipals(X500Principal.class);
-		if (userPrincipal.isEmpty()) {
-			Set<DataAdminPrincipal> dataAdminPrincipal = subject.getPrincipals(DataAdminPrincipal.class);
-			if (!dataAdminPrincipal.isEmpty())
+		boolean isRegularUser = !userPrincipal.isEmpty();
+		if (isAnonymous) {
+			if (isDataAdmin || isJackrabbitSystem || isRegularUser)
+				throw new IllegalStateException("Inconsistent " + subject);
+			else
+				return NodeConstants.ROLE_ANONYMOUS;
+		} else if (isDataAdmin) {
+			if (isAnonymous || isJackrabbitSystem || isRegularUser)
+				throw new IllegalStateException("Inconsistent " + subject);
+			else {
+				assert !subject.getPrincipals(AdminPrincipal.class).isEmpty();
 				return NodeConstants.ROLE_DATA_ADMIN;
-			throw new IllegalStateException("Subject is neither anonymous nor logged-in");
+			}
+		} else if (isJackrabbitSystem) {
+			if (isAnonymous || isDataAdmin || isRegularUser)
+				throw new IllegalStateException("Inconsistent " + subject);
+			else
+				return super.getUserID(subject, workspaceName);
+		} else if (isRegularUser) {
+			if (isAnonymous || isDataAdmin || isJackrabbitSystem)
+				throw new IllegalStateException("Inconsistent " + subject);
+			else {
+				if (userPrincipal.size() > 1) {
+					StringBuilder buf = new StringBuilder();
+					for (X500Principal principal : userPrincipal)
+						buf.append(' ').append('\"').append(principal).append('\"');
+					throw new RuntimeException("Multiple user principals:" + buf);
+				}
+				return userPrincipal.iterator().next().getName();
+			}
+		} else {
+			throw new IllegalStateException("Unrecognized subject type: " + subject);
 		}
-		// return super.getUserID(subject, workspaceName);
-		if (userPrincipal.size() > 1) {
-			StringBuilder buf = new StringBuilder();
-			for (X500Principal principal : userPrincipal)
-				buf.append(' ').append('\"').append(principal).append('\"');
-			throw new RuntimeException("Multiple user principals:" + buf);
-		}
-		return userPrincipal.iterator().next().getName();
-		// Authentication authentication = SecurityContextHolder.getContext()
-		// .getAuthentication();
-		// if (authentication != null)
-		// return authentication.getName();
-		// else
-		// return super.getUserID(subject, workspaceName);
 	}
 
 	@Override