Make CMS sesison management more robust.

diff --git a/org.argeo.cms/src/org/argeo/cms/auth/CmsAuthUtils.java b/org.argeo.cms/src/org/argeo/cms/auth/CmsAuthUtils.java
index e9462c3add31cb7dbd0ef16f48afb00e2225a611..f5503d5c5d74185b4dfbb18b2c2c102efdd8a08e 100644 (file)
--- a/org.argeo.cms/src/org/argeo/cms/auth/CmsAuthUtils.java
+++ b/org.argeo.cms/src/org/argeo/cms/auth/CmsAuthUtils.java
@@ -123,6 +123,7 @@ class CmsAuthUtils {
                // subject.getPrincipals().removeAll(subject.getPrincipals(AnonymousPrincipal.class));
        }
 
+       @SuppressWarnings("unused")
        synchronized static void registerSessionAuthorization(HttpServletRequest request, Subject subject,
                        Authorization authorization, Locale locale) {
                // synchronized in order to avoid multiple registrations
@@ -131,46 +132,54 @@ class CmsAuthUtils {
                        HttpSession httpSession = request.getSession(false);
                        assert httpSession != null;
                        String httpSessId = httpSession.getId();
-                       String remoteUser = authorization.getName() != null ? authorization.getName()
-                                       : NodeConstants.ROLE_ANONYMOUS;
+                       boolean anonymous = authorization.getName() == null;
+                       String remoteUser = !anonymous ? authorization.getName() : NodeConstants.ROLE_ANONYMOUS;
                        request.setAttribute(HttpContext.REMOTE_USER, remoteUser);
                        request.setAttribute(HttpContext.AUTHORIZATION, authorization);
 
-                       CmsSessionImpl cmsSession = CmsSessionImpl.getByLocalId(httpSessId);
-                       if (cmsSession != null) {
-                               if (authorization.getName() != null) {
-                                       if (cmsSession.getAuthorization().getName() == null) {
-                                               cmsSession.close();
-                                               cmsSession = null;
-                                       } else if (!authorization.getName().equals(cmsSession.getAuthorization().getName())) {
+                       CmsSessionImpl cmsSession;
+                       CmsSessionImpl currentLocalSession = CmsSessionImpl.getByLocalId(httpSessId);
+                       if (currentLocalSession != null) {
+                               boolean currentLocalSessionAnonymous = currentLocalSession.getAuthorization().getName() == null;
+                               if (!anonymous) {
+                                       if (currentLocalSessionAnonymous) {
+                                               currentLocalSession.close();
+                                               // new CMS session
+                                               cmsSession = new WebCmsSessionImpl(subject, authorization, locale, request);
+                                       } else if (!authorization.getName().equals(currentLocalSession.getAuthorization().getName())) {
                                                throw new IllegalStateException("Inconsistent user " + authorization.getName()
-                                                               + " for existing CMS session " + cmsSession);
-                                       }
-                                       // keyring
-                                       if (cmsSession != null)
+                                                               + " for existing CMS session " + currentLocalSession);
+                                       } else {
+                                               // keep current session
+                                               cmsSession = currentLocalSession;
+                                               // keyring
                                                subject.getPrivateCredentials().addAll(cmsSession.getSecretKeys());
+                                       }
                                } else {// anonymous
-                                       if (cmsSession.getAuthorization().getName() != null) {
-                                               cmsSession.close();
-                                               // TODO rather throw an exception ? log a warning ?
-                                               cmsSession = null;
+                                       if (!currentLocalSessionAnonymous) {
+                                               currentLocalSession.close();
+                                               throw new IllegalStateException(
+                                                               "Existing CMS session " + currentLocalSession + " was not logged out properly.");
                                        }
+                                       // keep current session
+                                       cmsSession = currentLocalSession;
                                }
-                       } else if (cmsSession == null) {
+                       } else {
+                               // new CMS session
                                cmsSession = new WebCmsSessionImpl(subject, authorization, locale, request);
                        }
-                       // request.setAttribute(CmsSession.class.getName(), cmsSession);
-                       if (cmsSession != null) {
-                               CmsSessionId nodeSessionId = new CmsSessionId(cmsSession.getUuid());
-                               if (subject.getPrivateCredentials(CmsSessionId.class).size() == 0)
-                                       subject.getPrivateCredentials().add(nodeSessionId);
-                               else {
-                                       UUID storedSessionId = subject.getPrivateCredentials(CmsSessionId.class).iterator().next()
-                                                       .getUuid();
-                                       // if (storedSessionId.equals(httpSessionId.getValue()))
-                                       throw new IllegalStateException(
-                                                       "Subject already logged with session " + storedSessionId + " (not " + nodeSessionId + ")");
-                               }
+
+                       if (cmsSession == null)// should be dead code (cf. SuppressWarning of the method)
+                               throw new IllegalStateException("CMS session cannot be null");
+
+                       CmsSessionId nodeSessionId = new CmsSessionId(cmsSession.getUuid());
+                       if (subject.getPrivateCredentials(CmsSessionId.class).size() == 0) {
+                               subject.getPrivateCredentials().add(nodeSessionId);
+                       } else {
+                               UUID storedSessionId = subject.getPrivateCredentials(CmsSessionId.class).iterator().next().getUuid();
+                               // if (storedSessionId.equals(httpSessionId.getValue()))
+                               throw new IllegalStateException(
+                                               "Subject already logged with session " + storedSessionId + " (not " + nodeSessionId + ")");
                        }
                } else {
                        // TODO desktop, CLI

diff --git a/org.argeo.cms/src/org/argeo/cms/internal/auth/CmsSessionImpl.java b/org.argeo.cms/src/org/argeo/cms/internal/auth/CmsSessionImpl.java
index b6966765d9534ea1469188dc5a81c06b1cf80fa3..c18348385587e4ac956160779a3235f36b0626e9 100644 (file)
--- a/org.argeo.cms/src/org/argeo/cms/internal/auth/CmsSessionImpl.java
+++ b/org.argeo.cms/src/org/argeo/cms/internal/auth/CmsSessionImpl.java
@@ -10,7 +10,6 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Hashtable;
-import java.util.LinkedHashSet;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
@@ -60,7 +59,7 @@ public class CmsSessionImpl implements CmsSession {
 
        private Map<String, Session> dataSessions = new HashMap<>();
        private Set<String> dataSessionsInUse = new HashSet<>();
-       private LinkedHashSet<Session> additionalDataSessions = new LinkedHashSet<>();
+       private Set<Session> additionalDataSessions = new HashSet<>();
 
        private Map<String, Object> views = new HashMap<>();
 
@@ -129,14 +128,17 @@ public class CmsSessionImpl implements CmsSession {
        }
 
        public Set<SecretKey> getSecretKeys() {
+               checkValid();
                return getSubject().getPrivateCredentials(SecretKey.class);
        }
 
        public Session newDataSession(String cn, String workspace, Repository repository) {
+               checkValid();
                return login(repository, workspace);
        }
 
        public synchronized Session getDataSession(String cn, String workspace, Repository repository) {
+               checkValid();
                // FIXME make it more robust
                if (workspace == null)
                        workspace = NodeConstants.SYS_WORKSPACE;
@@ -207,12 +209,18 @@ public class CmsSessionImpl implements CmsSession {
                return !isClosed();
        }
 
-       protected boolean isClosed() {
+       private void checkValid() {
+               if (!isValid())
+                       throw new IllegalStateException("CMS session " + uuid + " is not valid since " + end);
+       }
+
+       final protected boolean isClosed() {
                return getEnd() != null;
        }
 
        @Override
        public Authorization getAuthorization() {
+               checkValid();
                return authorization;
        }
 
@@ -258,6 +266,7 @@ public class CmsSessionImpl implements CmsSession {
 
        @Override
        public void registerView(String uid, Object view) {
+               checkValid();
                if (views.containsKey(uid))
                        throw new IllegalArgumentException("View " + uid + " is already registered.");
                views.put(uid, view);