From 650835c0d767c0fb59dd6237dd9b31a6d7b9ccd0 Mon Sep 17 00:00:00 2001
From: felmer <franz-josef.elmer@id.ethz.ch>
Date: Wed, 4 Sep 2019 15:03:36 +0200
Subject: [PATCH] SSDM-8599: Instead of throwing an exception the oldest
session will be expired
---
.../authentication/DefaultSessionManager.java | 88 ++++++++++++++-----
.../cisd/authentication/ISessionManager.java | 2 +
.../DefaultSessionManagerTest.java | 30 ++++---
.../web/server/AbstractClientService.java | 74 ++++++++++++----
.../generic/server/AbstractServer.java | 17 ++++
.../eln-lims/html/js/server/ServerFacade.js | 3 +-
6 files changed, 162 insertions(+), 52 deletions(-)
diff --git a/authentication/source/java/ch/systemsx/cisd/authentication/DefaultSessionManager.java b/authentication/source/java/ch/systemsx/cisd/authentication/DefaultSessionManager.java
index 41f9aeaaea8..778350bcd3b 100644
--- a/authentication/source/java/ch/systemsx/cisd/authentication/DefaultSessionManager.java
+++ b/authentication/source/java/ch/systemsx/cisd/authentication/DefaultSessionManager.java
@@ -19,11 +19,15 @@ package ch.systemsx.cisd.authentication;
import java.io.File;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
-import java.util.Collection;
+import java.util.ArrayList;
+import java.util.Collections;
import java.util.Date;
import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
import java.util.Map;
import java.util.Properties;
+import java.util.Set;
import javax.annotation.Resource;
@@ -32,6 +36,7 @@ import org.apache.commons.lang3.time.DateUtils;
import org.apache.commons.lang3.time.DurationFormatUtils;
import org.apache.log4j.Logger;
+import ch.systemsx.cisd.common.collection.SimpleComparator;
import ch.systemsx.cisd.common.exceptions.EnvironmentFailureException;
import ch.systemsx.cisd.common.exceptions.InvalidSessionException;
import ch.systemsx.cisd.common.exceptions.UserFailureException;
@@ -55,6 +60,11 @@ import ch.systemsx.cisd.common.spring.ExposablePropertyPlaceholderConfigurer;
*/
public class DefaultSessionManager<T extends BasicSession> implements ISessionManager<T>
{
+ private enum SessionClosingReason
+ {
+ LOGOUT, SESSION_EXPIRATION, SESSIONS_LIMIT
+ }
+
public static final File NO_LOGIN_FILE = new File("./etc/nologin.html");
private static final String LOGOUT_PREFIX = "LOGOUT: ";
@@ -143,6 +153,8 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
private final boolean tryEmailAsUserName;
+ private final Set<ISessionActionListener> listeners = new LinkedHashSet<>();
+
public DefaultSessionManager(final ISessionFactory<T> sessionFactory,
final ILogMessagePrefixGenerator<T> prefixGenerator,
final IAuthenticationService authenticationService,
@@ -212,11 +224,11 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
int maxNumberOfSessions = getMaxNumberOfSessionsFor(user);
if (maxNumberOfSessions > 0)
{
- int numberOfOpenSessions = getNumberOfOpenSessionsFor(user);
- if (numberOfOpenSessions >= maxNumberOfSessions)
+ List<FullSession<T>> openSessions = getOpenSessionsFor(user);
+ while (openSessions.size() >= maxNumberOfSessions)
{
- throw new UserFailureException(numberOfOpenSessions + " open session(s) but only "
- + maxNumberOfSessions + " session(s) are allowed for user " + user + ".");
+ FullSession<T> session = openSessions.remove(0);
+ closeSession(session.getSession(), SessionClosingReason.SESSIONS_LIMIT);
}
}
@@ -232,18 +244,25 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
}
}
- private int getNumberOfOpenSessionsFor(String user)
+ private List<FullSession<T>> getOpenSessionsFor(String user)
{
- int count = 0;
- Collection<FullSession<T>> values = sessions.values();
- for (FullSession<T> session : values)
+ List<FullSession<T>> userSessions = new ArrayList<>();
+ for (FullSession<T> session : sessions.values())
{
if (session.getSession().getUserName().equals(user))
{
- count++;
+ userSessions.add(session);
}
}
- return count;
+ Collections.sort(userSessions, new SimpleComparator<FullSession<T>, Long>()
+ {
+ @Override
+ public Long evaluate(FullSession<T> session)
+ {
+ return session.lastActiveTime;
+ }
+ });
+ return userSessions;
}
protected int getMaxNumberOfSessionsFor(String user)
@@ -453,6 +472,18 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
}
}
+ private void logLimitedNumberOfSessions(final T session)
+ {
+ final String prefix = prefixGenerator.createPrefix(session);
+ authenticationLog.info(prefix + ": session closed because limit of open session has been reached.");
+ if (operationLog.isInfoEnabled())
+ {
+ final String user = session.getUserName();
+ operationLog.info(LOGOUT_PREFIX + "Session '" + session.getSessionToken()
+ + "' of user '" + user + "' has been closed because limit of open session has been reached.");
+ }
+ }
+
@Override
public boolean isAWellFormedSessionToken(String sessionTokenOrNull)
{
@@ -540,7 +571,7 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
}
if (checkAndTouch && doSessionExpiration(session))
{
- closeSession(session.getSession(), false);
+ closeSession(session.getSession(), SessionClosingReason.SESSION_EXPIRATION);
}
if (checkAndTouch && isSessionUnavailable(session))
{
@@ -615,7 +646,7 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
synchronized (sessions)
{
final T session = getSession(sessionToken, false);
- closeSession(session, true);
+ closeSession(session, SessionClosingReason.LOGOUT);
}
}
@@ -623,22 +654,32 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
public void expireSession(String sessionToken) throws InvalidSessionException
{
final T session = getSession(sessionToken, false);
- closeSession(session, false);
+ closeSession(session, SessionClosingReason.SESSION_EXPIRATION);
}
- private void closeSession(final T session, final boolean regularLogout)
+ private void closeSession(final T session, SessionClosingReason reason)
throws InvalidSessionException
{
synchronized (sessions)
{
session.cleanup();
- sessions.remove(session.getSessionToken());
- if (regularLogout)
+ String sessionToken = session.getSessionToken();
+ sessions.remove(sessionToken);
+ switch (reason)
{
- logLogout(session);
- } else
+ case LOGOUT:
+ logLogout(session);
+ break;
+ case SESSION_EXPIRATION:
+ logSessionExpired(session);
+ break;
+ case SESSIONS_LIMIT:
+ logLimitedNumberOfSessions(session);
+ break;
+ }
+ for (ISessionActionListener listener : listeners)
{
- logSessionExpired(session);
+ listener.sessionClosed(sessionToken);
}
}
}
@@ -659,4 +700,11 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
}
return p;
}
+
+ @Override
+ public void addListener(ISessionActionListener listener)
+ {
+ listeners.add(listener);
+ }
+
}
diff --git a/authentication/source/java/ch/systemsx/cisd/authentication/ISessionManager.java b/authentication/source/java/ch/systemsx/cisd/authentication/ISessionManager.java
index e08049acc03..30aa716f670 100644
--- a/authentication/source/java/ch/systemsx/cisd/authentication/ISessionManager.java
+++ b/authentication/source/java/ch/systemsx/cisd/authentication/ISessionManager.java
@@ -75,5 +75,7 @@ public interface ISessionManager<T extends BasicSession> extends IRemoteHostProv
* </p>
*/
public T tryGetSession(final String sessionToken);
+
+ public void addListener(ISessionActionListener listener);
}
\ No newline at end of file
diff --git a/authentication/sourceTest/java/ch/systemsx/cisd/authentication/DefaultSessionManagerTest.java b/authentication/sourceTest/java/ch/systemsx/cisd/authentication/DefaultSessionManagerTest.java
index 78ed2c035df..8704bf905bc 100644
--- a/authentication/sourceTest/java/ch/systemsx/cisd/authentication/DefaultSessionManagerTest.java
+++ b/authentication/sourceTest/java/ch/systemsx/cisd/authentication/DefaultSessionManagerTest.java
@@ -24,6 +24,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.regex.Pattern;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Level;
@@ -40,6 +41,7 @@ import ch.systemsx.cisd.common.exceptions.EnvironmentFailureException;
import ch.systemsx.cisd.common.exceptions.UserFailureException;
import ch.systemsx.cisd.common.logging.BufferedAppender;
import ch.systemsx.cisd.common.server.IRemoteHostProvider;
+import ch.systemsx.cisd.common.test.RecordingMatcher;
import ch.systemsx.cisd.common.test.RetryTen;
import ch.systemsx.cisd.common.test.TestReportCleaner;
@@ -78,6 +80,8 @@ public class DefaultSessionManagerTest
private Sequence sequence;
+ private ISessionActionListener sessionActionListener;
+
private void assertExceptionMessageForInvalidSessionToken(final UserFailureException ex)
{
final String message = ex.getMessage();
@@ -94,6 +98,7 @@ public class DefaultSessionManagerTest
authenticationService = context.mock(IAuthenticationService.class);
remoteHostProvider = context.mock(IRemoteHostProvider.class);
principalProvider = context.mock(IPrincipalProvider.class);
+ sessionActionListener = context.mock(ISessionActionListener.class);
sequence = context.sequence("sequence");
context.checking(new Expectations()
{
@@ -232,6 +237,7 @@ public class DefaultSessionManagerTest
public void testLimitedNumberOfSession()
{
final String user = "bla";
+ RecordingMatcher<BasicSession> recordingSessionMatcher = new RecordingMatcher<BasicSession>();
context.checking(new Expectations()
{
{
@@ -243,7 +249,7 @@ public class DefaultSessionManagerTest
exactly(3).of(authenticationService).tryGetAndAuthenticateUser(user, "blub");
will(returnValue(principal));
- for (int i = 0; i < 2; i++)
+ for (int i = 0; i < 3; i++)
{
one(sessionFactory).create(with(any(String.class)), with(equal(user)),
with(equal(principal)), with(equal(REMOTE_HOST)),
@@ -253,26 +259,24 @@ public class DefaultSessionManagerTest
will(returnValue(session));
inSequence(sequence);
- one(prefixGenerator).createPrefix(session);
+ one(prefixGenerator).createPrefix(with(recordingSessionMatcher));
will(returnValue("[USER:'" + user + "', HOST:'remote-host']"));
}
-
- one(prefixGenerator).createPrefix("bla", REMOTE_HOST);
- will(returnValue("[USER:'bla', HOST:'remote-host']"));
+ one(prefixGenerator).createPrefix(with(recordingSessionMatcher));
+ will(returnValue("[USER:'" + user + "', HOST:'remote-host']"));
+
+ one(sessionActionListener).sessionClosed("bla-1");
}
});
sessionManager = createSessionManager(SESSION_EXPIRATION_PERIOD_MINUTES, Collections.singletonMap("bla", 2));
+ sessionManager.addListener(sessionActionListener);
assertEquals("bla-1", sessionManager.tryToOpenSession("bla", "blub"));
assertEquals("bla-2", sessionManager.tryToOpenSession("bla", "blub"));
- try
- {
- assertEquals("bla-3", sessionManager.tryToOpenSession("bla", "blub"));
- fail("UserFailureException expected");
- } catch (UserFailureException e)
- {
- assertEquals("2 open session(s) but only 2 session(s) are allowed for user bla.", e.getMessage());
- }
+ assertEquals("bla-3", sessionManager.tryToOpenSession("bla", "blub"));
+ assertEquals("[bla-1, bla-2, bla-1, bla-3]",
+ recordingSessionMatcher.getRecordedObjects().stream().map(s -> s.getSessionToken())
+ .collect(Collectors.toList()).toString());
context.assertIsSatisfied();
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/AbstractClientService.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/AbstractClientService.java
index c1d3ce70d52..0976d7a0f46 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/AbstractClientService.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/AbstractClientService.java
@@ -36,6 +36,7 @@ import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import ch.rinn.restrictions.Private;
+import ch.systemsx.cisd.authentication.ISessionActionListener;
import ch.systemsx.cisd.common.exceptions.UserFailureException;
import ch.systemsx.cisd.common.logging.LogCategory;
import ch.systemsx.cisd.common.logging.LogFactory;
@@ -73,8 +74,10 @@ import ch.systemsx.cisd.openbis.generic.client.web.server.translator.ResultSetTr
import ch.systemsx.cisd.openbis.generic.client.web.server.translator.UserFailureExceptionTranslator;
import ch.systemsx.cisd.openbis.generic.client.web.server.util.TableModelUtils;
import ch.systemsx.cisd.openbis.generic.client.web.server.util.XMLPropertyTransformer;
+import ch.systemsx.cisd.openbis.generic.server.ComponentNames;
import ch.systemsx.cisd.openbis.generic.server.SessionConstants;
import ch.systemsx.cisd.openbis.generic.shared.Constants;
+import ch.systemsx.cisd.openbis.generic.shared.IOpenBisSessionManager;
import ch.systemsx.cisd.openbis.generic.shared.IServer;
import ch.systemsx.cisd.openbis.generic.shared.WebClientConfigurationProvider;
import ch.systemsx.cisd.openbis.generic.shared.basic.IEntityInformationHolder;
@@ -116,6 +119,29 @@ public abstract class AbstractClientService implements IClientService,
@Resource(name = ExposablePropertyPlaceholderConfigurer.PROPERTY_CONFIGURER_BEAN_NAME)
private ExposablePropertyPlaceholderConfigurer configurer;
+ @Resource(name = ComponentNames.SESSION_MANAGER)
+ private IOpenBisSessionManager sessionManager;
+
+ private final Map<String, HttpSession> httpSessionsBySessionToken = new HashMap<>();
+
+ private ISessionActionListener sessionActionListener = new ISessionActionListener()
+ {
+ @Override
+ public void sessionClosed(String sessionToken)
+ {
+ synchronized (httpSessionsBySessionToken)
+ {
+ HttpSession httpSession = httpSessionsBySessionToken.remove(sessionToken);
+ logout(null, true, httpSession, sessionToken);
+ if (httpSession != null)
+ {
+ operationLog.info("Session " + sessionToken + " closed. "
+ + "httpSessionsBySessionToken.size() = " + httpSessionsBySessionToken.size());
+ }
+ }
+ }
+ };
+
@Autowired
private TableDataCache<String, Object> tableDataCache;
@@ -653,6 +679,12 @@ public abstract class AbstractClientService implements IClientService,
return null;
}
final HttpSession httpSession = createHttpSession();
+ synchronized (httpSessionsBySessionToken)
+ {
+ httpSessionsBySessionToken.put(session.getSessionToken(), httpSession);
+ operationLog.info("httpSessionsBySessionToken.size() = " + httpSessionsBySessionToken.size());
+ }
+
// Expiration time of httpSession is 10 seconds less than of session
final int sessionExpirationTimeInMillis = session.getSessionExpirationTime();
final int sessionExpirationTimeInSeconds = sessionExpirationTimeInMillis / 1000;
@@ -772,26 +804,31 @@ public abstract class AbstractClientService implements IClientService,
try
{
final HttpSession httpSession = getHttpSession();
- if (httpSession != null)
+ String sessionToken = getSessionToken();
+ logout(displaySettings, simpleViewMode, httpSession, sessionToken);
+ } catch (Exception e)
+ {
+ operationLog.info("logout exception: " + e);
+ }
+ }
+
+ private void logout(DisplaySettings displaySettings, boolean simpleViewMode, final HttpSession httpSession, String sessionToken)
+ {
+ if (httpSession != null)
+ {
+ httpSession.removeAttribute(SessionConstants.OPENBIS_SESSION_TOKEN_ATTRIBUTE_KEY);
+ httpSession.removeAttribute(SessionConstants.OPENBIS_SERVER_ATTRIBUTE_KEY);
+ httpSession.removeAttribute(SessionConstants.OPENBIS_RESULT_SET_MANAGER);
+ httpSession.removeAttribute(SessionConstants.OPENBIS_EXPORT_MANAGER);
+ httpSession.invalidate();
+ IServer server = getServer();
+ if (simpleViewMode == false)
{
- String sessionToken = getSessionToken();
- httpSession.removeAttribute(SessionConstants.OPENBIS_SESSION_TOKEN_ATTRIBUTE_KEY);
- httpSession.removeAttribute(SessionConstants.OPENBIS_SERVER_ATTRIBUTE_KEY);
- httpSession.removeAttribute(SessionConstants.OPENBIS_RESULT_SET_MANAGER);
- httpSession.removeAttribute(SessionConstants.OPENBIS_EXPORT_MANAGER);
- httpSession.invalidate();
- IServer server = getServer();
- if (simpleViewMode == false)
- {
- // only save settings for "normal" view
- int maxEntityVisits = getWebClientConfiguration().getMaxEntityVisits();
- server.saveDisplaySettings(sessionToken, displaySettings, maxEntityVisits);
- }
- server.logout(sessionToken);
+ // only save settings for "normal" view
+ int maxEntityVisits = getWebClientConfiguration().getMaxEntityVisits();
+ server.saveDisplaySettings(sessionToken, displaySettings, maxEntityVisits);
}
- } catch (final UserFailureException e)
- {
- // Just ignore it because we are logging out anyway.
+ server.logout(sessionToken);
}
}
@@ -832,5 +869,6 @@ public abstract class AbstractClientService implements IClientService,
public void setApplicationContext(ApplicationContext applicationContext)
{
this.applicationContext = applicationContext;
+ sessionManager.addListener(sessionActionListener);
}
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
index 5952c837f6b..3ce90cd554e 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
@@ -29,6 +29,7 @@ import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
+import javax.annotation.PostConstruct;
import javax.annotation.Resource;
import org.apache.commons.lang3.StringUtils;
@@ -39,6 +40,7 @@ import org.springframework.transaction.annotation.Transactional;
import ch.ethz.sis.openbis.generic.asapi.v3.IApplicationServerApi;
import ch.systemsx.cisd.authentication.DefaultSessionManager;
import ch.systemsx.cisd.authentication.IPrincipalProvider;
+import ch.systemsx.cisd.authentication.ISessionActionListener;
import ch.systemsx.cisd.authentication.ISessionManager;
import ch.systemsx.cisd.authentication.Principal;
import ch.systemsx.cisd.common.action.IDelegatedActionWithResult;
@@ -178,6 +180,15 @@ public abstract class AbstractServer<T> extends AbstractServiceWithLogger<T> imp
private IApplicationServerApi v3Api;
protected String CISDHelpdeskEmail;
+
+ private ISessionActionListener sessionActionListener = new ISessionActionListener()
+ {
+ @Override
+ public void sessionClosed(String sessionToken)
+ {
+ logout(sessionToken);
+ }
+ };
protected AbstractServer()
{
@@ -204,6 +215,12 @@ public abstract class AbstractServer<T> extends AbstractServiceWithLogger<T> imp
this.sampleTypeSlaveServerPlugin = sampleTypeSlaveServerPlugin;
this.dataSetTypeSlaveServerPlugin = dataSetTypeSlaveServerPlugin;
}
+
+ @PostConstruct
+ public void registerAtSessionManager()
+ {
+ sessionManager.addListener(sessionActionListener);
+ }
// For unit tests - in production Spring will inject this object.
public void setDisplaySettingsProvider(DisplaySettingsProvider displaySettingsProvider)
diff --git a/openbis_standard_technologies/dist/core-plugins/eln-lims/1/as/webapps/eln-lims/html/js/server/ServerFacade.js b/openbis_standard_technologies/dist/core-plugins/eln-lims/1/as/webapps/eln-lims/html/js/server/ServerFacade.js
index 96e277604e4..c8bab098eba 100644
--- a/openbis_standard_technologies/dist/core-plugins/eln-lims/1/as/webapps/eln-lims/html/js/server/ServerFacade.js
+++ b/openbis_standard_technologies/dist/core-plugins/eln-lims/1/as/webapps/eln-lims/html/js/server/ServerFacade.js
@@ -51,7 +51,8 @@ function ServerFacade(openbisServer) {
var responseInterceptor = function(response, action){
var isError = false;
if(response && response.error) {
- if(response.error.message === "Session no longer available. Please login again.") {
+ if(response.error.message === "Session no longer available. Please login again."
+ || response.error.message.endsWith("is invalid: user is not logged in.")) {
isError = true;
Util.showError(response.error.message, function() {
location.reload(true);
--
GitLab