diff --git a/stroom-app/src/main/java/stroom/app/App.java b/stroom-app/src/main/java/stroom/app/App.java index a4445cae0ba..c658964179a 100644 --- a/stroom-app/src/main/java/stroom/app/App.java +++ b/stroom-app/src/main/java/stroom/app/App.java @@ -25,6 +25,8 @@ import stroom.config.app.AppConfig; import stroom.config.app.Config; import stroom.config.app.SecurityConfig; +import stroom.config.app.SessionConfig; +import stroom.config.app.SessionCookieConfig; import stroom.config.app.StroomYamlUtil; import stroom.config.global.impl.ConfigMapper; import stroom.dropwizard.common.AdminServlets; @@ -54,7 +56,9 @@ import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; import stroom.util.shared.AbstractConfig; +import stroom.util.shared.ModelStringUtil; import stroom.util.shared.ResourcePaths; +import stroom.util.time.StroomDuration; import stroom.util.validation.ValidationModule; import stroom.util.yaml.YamlUtil; @@ -70,7 +74,6 @@ import jakarta.inject.Inject; import jakarta.servlet.DispatcherType; import jakarta.servlet.FilterRegistration; -import jakarta.servlet.SessionCookieConfig; import jakarta.validation.ValidatorFactory; import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.servlets.CrossOriginFilter; @@ -78,6 +81,7 @@ import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; +import java.time.Duration; import java.util.EnumSet; import java.util.Objects; @@ -220,15 +224,14 @@ public void run(final Config configuration, final Environment environment) { // We want Stroom to use the root path so we need to move Dropwizard's path. environment.jersey().setUrlPattern(ResourcePaths.API_ROOT_PATH + "/*"); - // Set up a session handler for Jetty - configureSessionHandling(environment); - - // Ensure the session cookie that provides JSESSIONID is secure. - // Need to get it from ConfigMapper not AppConfig as ConfigMapper is now the source of - // truth for config. + // Need to get these config classed from ConfigMapper as the main appInjector is not created yet + // and configuration only holds the YAML view of the config, not the DB view. final ConfigMapper configMapper = bootStrapInjector.getInstance(ConfigMapper.class); - final stroom.config.app.SessionCookieConfig sessionCookieConfig = configMapper.getConfigObject( - stroom.config.app.SessionCookieConfig.class); + final SessionCookieConfig sessionCookieConfig = configMapper.getConfigObject(SessionCookieConfig.class); + final SessionConfig sessionConfig = configMapper.getConfigObject(SessionConfig.class); + + // Set up a session handler for Jetty + configureSessionHandling(environment, sessionConfig); configureSessionCookie(environment, sessionCookieConfig); // Configure Cross-Origin Resource Sharing. @@ -273,11 +276,11 @@ public void run(final Config configuration, final Environment environment) { private void showNodeInfo(final Config configuration) { LOGGER.info("" - + "\n********************************************************************************" - + "\n Stroom home: " + homeDirProvider.get().toAbsolutePath().normalize() - + "\n Stroom temp: " + tempDirProvider.get().toAbsolutePath().normalize() - + "\n Node name: " + getNodeName(configuration.getYamlAppConfig()) - + "\n********************************************************************************"); + + "\n********************************************************************************" + + "\n Stroom home: " + homeDirProvider.get().toAbsolutePath().normalize() + + "\n Stroom temp: " + tempDirProvider.get().toAbsolutePath().normalize() + + "\n Node name: " + getNodeName(configuration.getYamlAppConfig()) + + "\n********************************************************************************"); } private void warnAboutDefaultOpenIdCreds(final Config configuration, final Injector injector) { @@ -299,17 +302,17 @@ private void warnAboutDefaultOpenIdCreds(final Config configuration, final Injec .getFullPathStr(AbstractOpenIdConfig.PROP_NAME_IDP_TYPE); LOGGER.warn("\n" + - "\n -----------------------------------------------------------------------------" + - "\n " + - "\n WARNING!" + - "\n " + - "\n Using default and publicly available Open ID authentication credentials. " + - "\n This is insecure! These should only be used in test/demo environments. " + - "\n Set " + propPath + " to INTERNAL_IDP/EXTERNAL_IDP for production environments." + - "\n" + - "\n " + defaultOpenIdCredentials.getApiKey() + - "\n -----------------------------------------------------------------------------" + - "\n"); + "\n -----------------------------------------------------------------------------" + + "\n " + + "\n WARNING!" + + "\n " + + "\n Using default and publicly available Open ID authentication credentials. " + + "\n This is insecure! These should only be used in test/demo environments. " + + "\n Set " + propPath + " to INTERNAL_IDP/EXTERNAL_IDP for production environments." + + "\n" + + "\n " + defaultOpenIdCredentials.getApiKey() + + "\n -----------------------------------------------------------------------------" + + "\n"); } } @@ -344,35 +347,56 @@ private void validateAppConfig(final Config config, final Path configFile) { if (result.hasErrors() && appConfig.isHaltBootOnConfigValidationFailure()) { LOGGER.error("Application configuration is invalid. Stopping Stroom. To run Stroom with invalid " + - "configuration, set {} to false, however this is not advised!", + "configuration, set {} to false, however this is not advised!", appConfig.getFullPathStr(AppConfig.PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE)); System.exit(1); } } - private static void configureSessionHandling(final Environment environment) { - SessionHandler sessionHandler = new SessionHandler(); + private void configureSessionHandling(final Environment environment, + final SessionConfig sessionConfig) { + + final SessionHandler sessionHandler = new SessionHandler(); // We need to give our session cookie a name other than JSESSIONID, otherwise it might // clash with other services running on the same domain. sessionHandler.setSessionCookie(SESSION_COOKIE_NAME); + long maxInactiveIntervalSecs = NullSafe.getOrElse( + sessionConfig.getMaxInactiveInterval(), + StroomDuration::getDuration, + Duration::toSeconds, + -1L); + if (maxInactiveIntervalSecs > Integer.MAX_VALUE) { + maxInactiveIntervalSecs = -1; + } + LOGGER.info("Setting session maxInactiveInterval to {} secs ({})", + ModelStringUtil.formatCsv(maxInactiveIntervalSecs), + (maxInactiveIntervalSecs > 0 + ? Duration.ofSeconds(maxInactiveIntervalSecs).toString() + : String.valueOf(maxInactiveIntervalSecs))); + + // If we don't let sessions expire then the Map of HttpSession in SessionListListener + // will grow and grow + sessionHandler.setMaxInactiveInterval((int) maxInactiveIntervalSecs); + environment.servlets().setSessionHandler(sessionHandler); environment.jersey().register(SessionFactoryProvider.class); } - private static void configureSessionCookie(final Environment environment, - final stroom.config.app.SessionCookieConfig config) { + private void configureSessionCookie(final Environment environment, + final SessionCookieConfig sessionCookieConfig) { // Ensure the session cookie that provides JSESSIONID is secure. - final SessionCookieConfig sessionCookieConfig = environment + final jakarta.servlet.SessionCookieConfig servletSessionCookieConfig = environment .getApplicationContext() .getServletContext() .getSessionCookieConfig(); - sessionCookieConfig.setSecure(config.isSecure()); - sessionCookieConfig.setHttpOnly(config.isHttpOnly()); + servletSessionCookieConfig.setSecure(sessionCookieConfig.isSecure()); + servletSessionCookieConfig.setHttpOnly(sessionCookieConfig.isHttpOnly()); // TODO : Add `SameSite=Strict` when supported by JEE } private static void configureCors(io.dropwizard.core.setup.Environment environment) { - FilterRegistration.Dynamic cors = environment.servlets().addFilter("CORS", CrossOriginFilter.class); + final FilterRegistration.Dynamic cors = environment.servlets() + .addFilter("CORS", CrossOriginFilter.class); cors.addMappingForUrlPatterns(EnumSet.allOf(DispatcherType.class), true, "/*"); cors.setInitParameter(CrossOriginFilter.ALLOWED_METHODS_PARAM, "GET,PUT,POST,DELETE,OPTIONS,PATCH"); cors.setInitParameter(CrossOriginFilter.ALLOWED_ORIGINS_PARAM, "*"); diff --git a/stroom-config/stroom-config-app/src/main/java/stroom/config/app/AppConfig.java b/stroom-config/stroom-config-app/src/main/java/stroom/config/app/AppConfig.java index 1c77ad076b6..d9581906ca3 100644 --- a/stroom-config/stroom-config-app/src/main/java/stroom/config/app/AppConfig.java +++ b/stroom-config/stroom-config-app/src/main/java/stroom/config/app/AppConfig.java @@ -116,6 +116,7 @@ public class AppConfig extends AbstractConfig implements IsStroomConfig { public static final String PROP_NAME_SECURITY = "security"; public static final String PROP_NAME_SERVICE_DISCOVERY = "serviceDiscovery"; public static final String PROP_NAME_SESSION_COOKIE = "sessionCookie"; + public static final String PROP_NAME_SESSION = "session"; public static final String PROP_NAME_SOLR = "solr"; public static final String PROP_NAME_STATE = "state"; public static final String PROP_NAME_STATISTICS = "statistics"; @@ -161,6 +162,7 @@ public class AppConfig extends AbstractConfig implements IsStroomConfig { private final SecurityConfig securityConfig; private final ServiceDiscoveryConfig serviceDiscoveryConfig; private final SessionCookieConfig sessionCookieConfig; + private final SessionConfig sessionConfig; private final SolrConfig solrConfig; private final StateConfig stateConfig; private final StatisticsConfig statisticsConfig; @@ -211,6 +213,7 @@ public AppConfig() { new SecurityConfig(), new ServiceDiscoveryConfig(), new SessionCookieConfig(), + new SessionConfig(), new SolrConfig(), new StateConfig(), new StatisticsConfig(), @@ -260,6 +263,7 @@ public AppConfig(@JsonProperty(PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE) @JsonProperty(PROP_NAME_SECURITY) final SecurityConfig securityConfig, @JsonProperty(PROP_NAME_SERVICE_DISCOVERY) final ServiceDiscoveryConfig serviceDiscoveryConfig, @JsonProperty(PROP_NAME_SESSION_COOKIE) final SessionCookieConfig sessionCookieConfig, + @JsonProperty(PROP_NAME_SESSION) final SessionConfig sessionConfig, @JsonProperty(PROP_NAME_SOLR) final SolrConfig solrConfig, @JsonProperty(PROP_NAME_STATE) final StateConfig stateConfig, @JsonProperty(PROP_NAME_STATISTICS) final StatisticsConfig statisticsConfig, @@ -305,6 +309,7 @@ public AppConfig(@JsonProperty(PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE) this.securityConfig = securityConfig; this.serviceDiscoveryConfig = serviceDiscoveryConfig; this.sessionCookieConfig = sessionCookieConfig; + this.sessionConfig = sessionConfig; this.solrConfig = solrConfig; this.stateConfig = stateConfig; this.statisticsConfig = statisticsConfig; @@ -317,11 +322,12 @@ public AppConfig(@JsonProperty(PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE) @AssertTrue( message = "stroom." + PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE + " is set to false. If there is " + - "invalid configuration the system may behave in unexpected ways. This setting is not advised.", + "invalid configuration the system may behave in unexpected ways. This setting is not advised.", payload = ValidationSeverity.Warning.class) @JsonProperty(PROP_NAME_HALT_BOOT_ON_CONFIG_VALIDATION_FAILURE) @JsonPropertyDescription("If true, Stroom will halt on start up if any errors are found in the YAML " + - "configuration file. If false, the errors will simply be logged. Setting this to false is not advised.") + "configuration file. If false, the errors will simply be logged." + + "Setting this to false is not advised.") public boolean isHaltBootOnConfigValidationFailure() { return haltBootOnConfigValidationFailure; } @@ -363,8 +369,9 @@ public ClusterLockConfig getClusterLockConfig() { @JsonProperty(PROP_NAME_COMMON_DB_DETAILS) @JsonPropertyDescription("Defines a set of common database connection details to use if no connection details " + - "are defined for a service area in stroom, e.g. core or config. This means you can have all service " + - "areas running in a single database, have each in their own database or a mixture.") + "are defined for a service area in stroom, e.g. core or config. This means you can " + + "have all service areas running in a single database, have each in their own " + + "database or a mixture.") public CommonDbConfig getCommonDbConfig() { return commonDbConfig; } @@ -448,10 +455,10 @@ public NodeConfig getNodeConfig() { } @JsonPropertyDescription("This is the base endpoint of the node for all inter-node communications, " + - "i.e. all cluster management and node info calls. " + - "This endpoint will typically be hidden behind a firewall and not be publicly available. " + - "The address must be resolvable from all other nodes in the cluster. " + - "This does not need to be set for a single node cluster.") + "i.e. all cluster management and node info calls. " + + "This endpoint will typically be hidden behind a firewall and not be " + + "publicly available. The address must be resolvable from all other nodes " + + "in the cluster. This does not need to be set for a single node cluster.") @JsonProperty(PROP_NAME_NODE_URI) public NodeUriConfig getNodeUri() { return nodeUri; @@ -479,7 +486,7 @@ public PropertyServiceConfig getPropertyServiceConfig() { } @JsonPropertyDescription("This is public facing URI of stroom which may be different from the local host if " + - "behind a proxy") + "behind a proxy") @JsonProperty(PROP_NAME_PUBLIC_URI) public PublicUriConfig getPublicUri() { return publicUri; @@ -537,6 +544,11 @@ public SessionCookieConfig getSessionCookieConfig() { return sessionCookieConfig; } + @JsonProperty(PROP_NAME_SESSION) + public SessionConfig getSessionConfig() { + return sessionConfig; + } + @JsonProperty(PROP_NAME_STATE) @JsonPropertyDescription("Configuration for the stroom state service") public StateConfig getStateConfig() { @@ -555,7 +567,7 @@ public UiConfig getUiConfig() { } @JsonPropertyDescription("This is the URI where the UI is hosted if different to the public facing URI of the " + - "server, e.g. during development or some other deployments") + "server, e.g. during development or some other deployments") @JsonProperty(PROP_NAME_UI_URI) public UiUriConfig getUiUri() { return uiUri; diff --git a/stroom-config/stroom-config-app/src/main/java/stroom/config/app/SessionConfig.java b/stroom-config/stroom-config-app/src/main/java/stroom/config/app/SessionConfig.java new file mode 100644 index 00000000000..295b6b2be20 --- /dev/null +++ b/stroom-config/stroom-config-app/src/main/java/stroom/config/app/SessionConfig.java @@ -0,0 +1,51 @@ +package stroom.config.app; + +import stroom.util.config.annotations.RequiresRestart; +import stroom.util.config.annotations.RequiresRestart.RestartScope; +import stroom.util.shared.AbstractConfig; +import stroom.util.shared.IsStroomConfig; +import stroom.util.time.StroomDuration; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonPropertyDescription; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; + + +@JsonPropertyOrder(alphabetic = true) +public class SessionConfig extends AbstractConfig implements IsStroomConfig { + + public static final String PROP_NAME_MAX_INACTIVE_INTERVAL = "maxInactiveInterval"; + + public static final StroomDuration DEFAULT_MAX_INACTIVE_INTERVAL = StroomDuration.ofDays(1); + + private final StroomDuration maxInactiveInterval; + + public SessionConfig() { + this.maxInactiveInterval = DEFAULT_MAX_INACTIVE_INTERVAL; + } + + @SuppressWarnings("unused") + @JsonCreator + public SessionConfig( + @JsonProperty(PROP_NAME_MAX_INACTIVE_INTERVAL) final StroomDuration maxInactiveInterval) { + this.maxInactiveInterval = maxInactiveInterval; + } + + @RequiresRestart(RestartScope.UI) + @JsonProperty(PROP_NAME_MAX_INACTIVE_INTERVAL) + @JsonPropertyDescription("The maximum time interval between the last access of a HTTP session and " + + "it being considered expired. Set to null for sessions that never expire, " + + "however this will causes sessions to be held and build up in memory indefinitely, " + + "so is best avoided.") + public StroomDuration getMaxInactiveInterval() { + return maxInactiveInterval; + } + + @Override + public String toString() { + return "SessionConfig{" + + "maxInactiveInterval=" + maxInactiveInterval + + '}'; + } +} diff --git a/stroom-config/stroom-config-app/src/test/resources/stroom/config/app/expected.yaml b/stroom-config/stroom-config-app/src/test/resources/stroom/config/app/expected.yaml index 36d8776be43..4eb26bef35e 100644 --- a/stroom-config/stroom-config-app/src/test/resources/stroom/config/app/expected.yaml +++ b/stroom-config/stroom-config-app/src/test/resources/stroom/config/app/expected.yaml @@ -926,6 +926,8 @@ appConfig: servicesPort: 8080 zookeeperBasePath: "/stroom-services" zookeeperUrl: "localhost:2181" + session: + maxInactiveInterval: "P1D" sessionCookie: httpOnly: true secure: true diff --git a/stroom-config/stroom-config-global-impl/src/main/java/stroom/config/global/impl/ConfigProvidersModule.java b/stroom-config/stroom-config-global-impl/src/main/java/stroom/config/global/impl/ConfigProvidersModule.java index a30e11c3d0f..5d78d0ab122 100644 --- a/stroom-config/stroom-config-global-impl/src/main/java/stroom/config/global/impl/ConfigProvidersModule.java +++ b/stroom-config/stroom-config-global-impl/src/main/java/stroom/config/global/impl/ConfigProvidersModule.java @@ -132,6 +132,15 @@ stroom.config.app.SecurityConfig getSecurityConfig( stroom.config.app.SecurityConfig.class); } + @Generated("stroom.config.global.impl.GenerateConfigProvidersModule") + @Provides + @SuppressWarnings("unused") + stroom.config.app.SessionConfig getSessionConfig( + final ConfigMapper configMapper) { + return configMapper.getConfigObject( + stroom.config.app.SessionConfig.class); + } + @Generated("stroom.config.global.impl.GenerateConfigProvidersModule") @Provides @SuppressWarnings("unused") @@ -935,20 +944,20 @@ stroom.config.common.UriConfig getUriConfigButThrow( @Generated("stroom.config.global.impl.GenerateConfigProvidersModule") @Provides @SuppressWarnings("unused") - stroom.query.common.v2.ResultStoreLmdbConfig getResultStoreLmdbConfigButThrow( + stroom.query.common.v2.AbstractResultStoreConfig getAbstractResultStoreConfigButThrow( final ConfigMapper configMapper) { throw new UnsupportedOperationException( - "stroom.query.common.v2.ResultStoreLmdbConfig cannot be injected directly. " + "stroom.query.common.v2.AbstractResultStoreConfig cannot be injected directly. " + "Inject a config class that uses it or one of its sub-class instead."); } @Generated("stroom.config.global.impl.GenerateConfigProvidersModule") @Provides @SuppressWarnings("unused") - stroom.query.common.v2.AbstractResultStoreConfig getAbstractResultStoreConfigButThrow( + stroom.query.common.v2.ResultStoreLmdbConfig getResultStoreLmdbConfigButThrow( final ConfigMapper configMapper) { throw new UnsupportedOperationException( - "stroom.query.common.v2.AbstractResultStoreConfig cannot be injected directly. " + "stroom.query.common.v2.ResultStoreLmdbConfig cannot be injected directly. " + "Inject a config class that uses it or one of its sub-class instead."); } diff --git a/stroom-config/stroom-config-global-impl/src/test/java/stroom/config/global/impl/TestConfigMapper.java b/stroom-config/stroom-config-global-impl/src/test/java/stroom/config/global/impl/TestConfigMapper.java index b8f196f010a..c9a160a872e 100644 --- a/stroom-config/stroom-config-global-impl/src/test/java/stroom/config/global/impl/TestConfigMapper.java +++ b/stroom-config/stroom-config-global-impl/src/test/java/stroom/config/global/impl/TestConfigMapper.java @@ -28,6 +28,7 @@ import stroom.config.app.DataConfig; import stroom.config.app.PropertyServiceConfig; import stroom.config.app.SecurityConfig; +import stroom.config.app.SessionConfig; import stroom.config.app.SessionCookieConfig; import stroom.config.app.StatisticsConfig; import stroom.config.common.CommonDbConfig; @@ -359,11 +360,11 @@ void testFindPropsWithNoDefault() { LOGGER.info("Properties with no default value"); configProperties.forEach(configProperty -> { if (configProperty.getDefaultValue().isEmpty() - && !configProperty.getName().toString().contains("Pool") - && !configProperty.getName().toString().contains("expireAfterWrite") - && !configProperty.getName().toString().contains("expireAfterAccess") - && !configProperty.getName().toString().contains("db.connection") - && !configProperty.getName().toString().contains("Uri.")) { + && !configProperty.getName().toString().contains("Pool") + && !configProperty.getName().toString().contains("expireAfterWrite") + && !configProperty.getName().toString().contains("expireAfterAccess") + && !configProperty.getName().toString().contains("db.connection") + && !configProperty.getName().toString().contains("Uri.")) { LOGGER.info("{}", configProperty.getName()); } }); @@ -968,6 +969,7 @@ public TestConfig( @JsonProperty(PROP_NAME_SECURITY) final SecurityConfig securityConfig, @JsonProperty(PROP_NAME_SERVICE_DISCOVERY) final ServiceDiscoveryConfig serviceDiscoveryConfig, @JsonProperty(PROP_NAME_SESSION_COOKIE) final SessionCookieConfig sessionCookieConfig, + @JsonProperty(PROP_NAME_SESSION) final SessionConfig sessionConfig, @JsonProperty(PROP_NAME_SOLR) final SolrConfig solrConfig, @JsonProperty(PROP_NAME_STATE) final StateConfig stateConfig, @JsonProperty(PROP_NAME_STATISTICS) final StatisticsConfig statisticsConfig, @@ -1028,6 +1030,7 @@ public TestConfig( securityConfig, serviceDiscoveryConfig, sessionCookieConfig, + sessionConfig, solrConfig, stateConfig, statisticsConfig, diff --git a/stroom-core-shared/src/main/java/stroom/security/shared/SessionListResponse.java b/stroom-core-shared/src/main/java/stroom/security/shared/SessionListResponse.java index 17f9304c75d..3db09837dc2 100644 --- a/stroom-core-shared/src/main/java/stroom/security/shared/SessionListResponse.java +++ b/stroom-core-shared/src/main/java/stroom/security/shared/SessionListResponse.java @@ -23,7 +23,7 @@ public SessionListResponse(final List values) { super(values, SessionListResponse.createPageResponse(values)); } - public static final SessionListResponse empty() { + public static SessionListResponse empty() { return new SessionListResponse( Collections.emptyList(), SessionListResponse.createPageResponse(Collections.emptyList())); diff --git a/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/SessionListeners.java b/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/SessionListeners.java index 2cc4223cdbe..f4b078eba56 100644 --- a/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/SessionListeners.java +++ b/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/SessionListeners.java @@ -2,6 +2,7 @@ import io.dropwizard.core.setup.Environment; import jakarta.inject.Inject; +import jakarta.servlet.http.HttpSessionIdListener; import jakarta.servlet.http.HttpSessionListener; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -15,11 +16,15 @@ public class SessionListeners { private final Environment environment; private final Set sessionListeners; + private final Set sessionIdListeners; @Inject - SessionListeners(final Environment environment, final Set sessionListeners) { + SessionListeners(final Environment environment, + final Set sessionListeners, + final Set sessionIdListeners) { this.environment = environment; this.sessionListeners = sessionListeners; + this.sessionIdListeners = sessionIdListeners; } public void register() { @@ -31,5 +36,14 @@ public void register() { LOGGER.info("\t{}", name); environment.servlets().addServletListeners(sessionListener); }); + + LOGGER.info("Adding session Id listeners:"); + sessionIdListeners.stream() + .sorted(Comparator.comparing(sessionListener -> sessionListener.getClass().getName())) + .forEach(sessionIdListener -> { + final String name = sessionIdListener.getClass().getName(); + LOGGER.info("\t{}", name); + environment.servlets().addServletListeners(sessionIdListener); + }); } } diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SecurityFilter.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SecurityFilter.java index d3a1985b12b..278b7d9f7ee 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SecurityFilter.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SecurityFilter.java @@ -47,6 +47,7 @@ import jakarta.ws.rs.core.Response; import java.io.IOException; +import java.io.PrintWriter; import java.util.Optional; import java.util.Set; @@ -193,18 +194,28 @@ private void filter(final HttpServletRequest request, } else if (shouldBypassAuthentication(request, fullPath, servletPath, servletName)) { LOGGER.debug("Running as proc user for unauthenticated servletName: {}, " + - "fullPath: {}, servletPath: {}", servletName, fullPath, servletPath); + "fullPath: {}, servletPath: {}", servletName, fullPath, servletPath); // Some paths don't need authentication. If that is the case then proceed as proc user. securityContext.asProcessingUser(() -> process(request, response, chain)); - } else if (isApiRequest(servletName)) { + } else if (isApiRequest(servletPath)) { // If we couldn't log in with a token or couldn't get a token then error as this is an API call // or no login flow is possible/expected. LOGGER.debug("No user identity so responding with UNAUTHORIZED for servletName: {}, " + - "fullPath: {}, servletPath: {}", servletName, fullPath, servletPath); + "fullPath: {}, servletPath: {}", servletName, fullPath, servletPath); response.setStatus(Response.Status.UNAUTHORIZED.getStatusCode()); - + try { + final PrintWriter writer = response.getWriter(); + if (writer != null) { + writer.println(""" + Unauthorised + Your session may have expired in which case you \ + need to refresh the page in browser."""); + } + } catch (Exception e) { + // Ignore. The status will be sufficient. + } } else { // No identity found and not an unauthenticated servlet/api so assume it is // a UI request. Thus instigate an OpenID authentication flow @@ -215,7 +226,7 @@ private void filter(final HttpServletRequest request, final String stateId = UrlUtils.getLastParam(request, OpenId.STATE); final String redirectUri = openIdManager.redirect(request, code, stateId, postAuthRedirectUri); LOGGER.debug("Code flow UI request so redirecting to IDP, " + - "redirectUri: {}, postAuthRedirectUri: {}, path: {}", + "redirectUri: {}, postAuthRedirectUri: {}, path: {}", redirectUri, postAuthRedirectUri, fullPath); // response.setStatus(HttpServletResponse.SC_TEMPORARY_REDIRECT); // response.sendRedirect(redirectUri); diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionListListener.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionListListener.java index db51e55ec53..b355253ca27 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionListListener.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionListListener.java @@ -28,6 +28,8 @@ import stroom.task.api.TaskContextFactory; import stroom.util.jersey.UriBuilderUtil; import stroom.util.jersey.WebTargetFactory; +import stroom.util.logging.LambdaLogger; +import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; import stroom.util.servlet.UserAgentSessionUtil; import stroom.util.shared.ResourcePaths; @@ -37,23 +39,24 @@ import jakarta.inject.Singleton; import jakarta.servlet.http.HttpSession; import jakarta.servlet.http.HttpSessionEvent; +import jakarta.servlet.http.HttpSessionIdListener; import jakarta.servlet.http.HttpSessionListener; import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.client.WebTarget; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import java.time.Duration; +import java.time.Instant; import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; @Singleton -class SessionListListener implements HttpSessionListener, SessionListService { +class SessionListListener implements HttpSessionListener, HttpSessionIdListener, SessionListService { - private static final Logger LOGGER = LoggerFactory.getLogger(SessionListListener.class); + private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(SessionListListener.class); private final ConcurrentHashMap sessionMap = new ConcurrentHashMap<>(); @@ -84,8 +87,32 @@ public void sessionCreated(final HttpSessionEvent event) { @Override public void sessionDestroyed(final HttpSessionEvent event) { final HttpSession httpSession = event.getSession(); - LOGGER.info("sessionDestroyed() - {}", httpSession.getId()); - sessionMap.remove(httpSession.getId()); + try { + // Uncomment during merge up from 7.5 +// final UserRef userRef = getUserRefFromSession(httpSession); + // Get rid during merge up from 7.5 + final UserName userRef = getUserNameFromSession(httpSession); + final String userAgent = UserAgentSessionUtil.get(httpSession); + final Instant createTime = Instant.ofEpochMilli(httpSession.getCreationTime()); + final Instant lastAccessedTime = Instant.ofEpochMilli(httpSession.getLastAccessedTime()); + LOGGER.info("sessionDestroyed() - {}, createTime: {} ({}), lastAccessTime: {} ({}), " + + "userRef: {}, userAgent: {}", + httpSession.getId(), + createTime, + Duration.between(createTime, Instant.now()), + lastAccessedTime, + Duration.between(lastAccessedTime, Instant.now()), + userRef, + userAgent); + } finally { + sessionMap.remove(httpSession.getId()); + } + } + + @Override + public void sessionIdChanged(final HttpSessionEvent event, final String oldSessionId) { + final HttpSession httpSession = event.getSession(); + LOGGER.info("sessionIdChanged() - old: {}, new: {}", oldSessionId, httpSession.getId()); } // public ResultPage find(final BaseCriteria criteria) { @@ -119,11 +146,10 @@ public SessionListResponse listSessions(final String nodeName) { } else { // This is a different node so make a rest call to it to get the result - final String url = NodeCallUtil.getBaseEndpointUrl(nodeInfo, nodeService, nodeName) + - ResourcePaths.buildAuthenticatedApiPath( - SessionResource.BASE_PATH, - SessionResource.LIST_PATH_PART); + ResourcePaths.buildAuthenticatedApiPath( + SessionResource.BASE_PATH, + SessionResource.LIST_PATH_PART); try { LOGGER.debug("Sending request to {} for node {}", url, nodeName); @@ -143,26 +169,42 @@ public SessionListResponse listSessions(final String nodeName) { throw NodeCallUtil.handleExceptionsOnNodeCall(nodeName, url, e); } } + LOGGER.debug(() -> LogUtil.message("Returning {} session items", sessionList.size())); return sessionList; } private SessionListResponse listSessionsOnThisNode() { - return sessionMap.values().stream() - .map(httpSession -> { - // Don't really care about userUuids for this - final UserName userName = UserIdentitySessionUtil.get(httpSession) - .map(UserIdentity::asUserName) - .orElse(null); - return new SessionDetails( - userName, - httpSession.getCreationTime(), - httpSession.getLastAccessedTime(), - UserAgentSessionUtil.get(httpSession), - nodeInfo.getThisNodeName()); - }) - .collect(SessionListResponse.collector(SessionListResponse::new)); + final String thisNodeName = nodeInfo.getThisNodeName(); + return LOGGER.logDurationIfDebugEnabled(() -> + sessionMap.values().stream() + .map(httpSession -> { + final UserName userName = getUserNameFromSession(httpSession); + return new SessionDetails( + userName, + httpSession.getCreationTime(), + httpSession.getLastAccessedTime(), + UserAgentSessionUtil.get(httpSession), + thisNodeName); + }) + .collect(SessionListResponse.collector(SessionListResponse::new)), + () -> LogUtil.message("Obtain session list for this node ({})", thisNodeName)); + } + + // Get rid during merge up from 7.5 + private static UserName getUserNameFromSession(final HttpSession httpSession) { + return UserIdentitySessionUtil.get(httpSession) + .map(UserIdentity::asUserName) + .orElse(null); } + // Uncomment during merge up from 7.5 +// private static UserRef getUserRefFromSession(final HttpSession httpSession) { +// return UserIdentitySessionUtil.get(httpSession) +// .filter(uid -> uid instanceof HasUserRef) +// .map(uid -> ((HasUserRef) uid).getUserRef()) +// .orElse(null); +// } + public SessionListResponse listSessions() { return taskContextFactory.contextResult("Get session list on all active nodes", parentTaskContext -> nodeService.findNodeNames(FindNodeCriteria.allEnabled()) @@ -179,7 +221,7 @@ public SessionListResponse listSessions() { .supplyAsync(listSessionsOnNodeTask) .exceptionally(throwable -> { LOGGER.error("Error getting session list for node [{}]: {}. " + - "Enable DEBUG for stacktrace", + "Enable DEBUG for stacktrace", nodeName, throwable.getMessage()); LOGGER.debug("Error getting session list for node [{}]", diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionSecurityModule.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionSecurityModule.java index 6d44e841f85..de8647fcd0e 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionSecurityModule.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionSecurityModule.java @@ -4,6 +4,7 @@ import stroom.util.guice.ServletBinder; import com.google.inject.AbstractModule; +import jakarta.servlet.http.HttpSessionIdListener; import jakarta.servlet.http.HttpSessionListener; public class SessionSecurityModule extends AbstractModule { @@ -15,6 +16,9 @@ protected void configure() { GuiceUtil.buildMultiBinder(binder(), HttpSessionListener.class) .addBinding(SessionListListener.class); + GuiceUtil.buildMultiBinder(binder(), HttpSessionIdListener.class) + .addBinding(SessionListListener.class); + ServletBinder.create(binder()) .bind(SessionListServlet.class); } diff --git a/stroom-util/src/main/java/stroom/util/logging/LambdaLogger.java b/stroom-util/src/main/java/stroom/util/logging/LambdaLogger.java index 468da2ac2a8..300ae51dc90 100644 --- a/stroom-util/src/main/java/stroom/util/logging/LambdaLogger.java +++ b/stroom-util/src/main/java/stroom/util/logging/LambdaLogger.java @@ -73,7 +73,8 @@ public interface LambdaLogger extends Logger { * * @param timedWork Work to perform and to time if required * @param The type of the result of the work - * @param workDescription The name of the work to be added to the log message + * @param workDescription The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. * @return The result of the work */ default T logDurationIfTraceEnabled(final Supplier timedWork, final String workDescription) { @@ -86,7 +87,9 @@ default T logDurationIfTraceEnabled(final Supplier timedWork, final Strin * * @param timedWork Work to perform and to time if required * @param The type of the result of the work - * @param workDescriptionSupplier A supplier of the description of the work to be added to the log message + * @param workDescriptionSupplier A supplier of the description of the work to be added to the log message. + * The log message looks like + * '{@code Completed [] in }'. * @return The result of the work */ T logDurationIfTraceEnabled(final Supplier timedWork, final Supplier workDescriptionSupplier); @@ -97,7 +100,9 @@ default T logDurationIfTraceEnabled(final Supplier timedWork, final Strin * * @param timedWork Work to perform and to time if required * @param The type of the result of the work - * @param workDescriptionFunction A supplier of the description of the work to be added to the log message + * @param workDescriptionFunction A supplier of the description of the work to be added to the log message. + * The log message looks like + * '{@code Completed [] in }'. * @return The result of the work */ T logDurationIfTraceEnabled(final Supplier timedWork, final Function workDescriptionFunction); @@ -107,7 +112,8 @@ default T logDurationIfTraceEnabled(final Supplier timedWork, final Strin * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescription The name of the work to be added to the log message + * @param workDescription The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. * @param The type of the result of the work * @return The result of the work */ @@ -120,7 +126,8 @@ default T logDurationIfDebugEnabled(final Supplier timedWork, final Strin * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescriptionSupplier The name of the work to be added to the log message + * @param workDescriptionSupplier The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. * @param The type of the result of the work * @return The result of the work */ @@ -131,7 +138,8 @@ default T logDurationIfDebugEnabled(final Supplier timedWork, final Strin * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescriptionFunction The name of the work to be added to the log message + * @param workDescriptionFunction The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. * @param The type of the result of the work * @return The result of the work */ @@ -142,7 +150,8 @@ default T logDurationIfDebugEnabled(final Supplier timedWork, final Strin * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescription The name of the work to be added to the log message + * @param workDescription The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. * @param The type of the result of the work * @return The result of the work */ @@ -155,7 +164,8 @@ default T logDurationIfInfoEnabled(final Supplier timedWork, final String * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescriptionSupplier The name of the work to be added to the log message + * @param workDescriptionSupplier The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. * @param The type of the result of the work * @return The result of the work */ @@ -166,7 +176,8 @@ default T logDurationIfInfoEnabled(final Supplier timedWork, final String * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescriptionFunction The name of the work to be added to the log message + * @param workDescriptionFunction The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. * @param The type of the result of the work * @return The result of the work */ @@ -177,7 +188,8 @@ default T logDurationIfInfoEnabled(final Supplier timedWork, final String * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescription The name of the work to be added to the log message + * @param workDescription The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. */ default void logDurationIfTraceEnabled(final Runnable timedWork, final String workDescription) { logDurationIfTraceEnabled(timedWork, () -> workDescription); @@ -188,7 +200,8 @@ default void logDurationIfTraceEnabled(final Runnable timedWork, final String wo * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescriptionSupplier The name of the work to be added to the log message + * @param workDescriptionSupplier The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. */ void logDurationIfTraceEnabled(final Runnable timedWork, final Supplier workDescriptionSupplier); @@ -197,7 +210,8 @@ default void logDurationIfTraceEnabled(final Runnable timedWork, final String wo * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescription The name of the work to be added to the log message + * @param workDescription The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. */ default void logDurationIfDebugEnabled(final Runnable timedWork, final String workDescription) { logDurationIfDebugEnabled(timedWork, () -> workDescription); @@ -208,7 +222,8 @@ default void logDurationIfDebugEnabled(final Runnable timedWork, final String wo * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescriptionSupplier The name of the work to be added to the log message + * @param workDescriptionSupplier The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. */ void logDurationIfDebugEnabled(final Runnable timedWork, final Supplier workDescriptionSupplier); @@ -217,7 +232,8 @@ default void logDurationIfDebugEnabled(final Runnable timedWork, final String wo * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescription The name of the work to be added to the log message + * @param workDescription The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. */ default void logDurationIfInfoEnabled(final Runnable timedWork, final String workDescription) { logDurationIfInfoEnabled(timedWork, () -> workDescription); @@ -228,7 +244,8 @@ default void logDurationIfInfoEnabled(final Runnable timedWork, final String wor * where the work throws a checked exception. * * @param timedWork Work to perform and to time if required - * @param workDescriptionSupplier The name of the work to be added to the log message + * @param workDescriptionSupplier The name of the work to be added to the log message. The log message looks + * like '{@code Completed [] in }'. */ void logDurationIfInfoEnabled(final Runnable timedWork, final Supplier workDescriptionSupplier); diff --git a/stroom-util/src/main/java/stroom/util/servlet/UserAgentSessionUtil.java b/stroom-util/src/main/java/stroom/util/servlet/UserAgentSessionUtil.java index db71da116eb..5475b54d6a6 100644 --- a/stroom-util/src/main/java/stroom/util/servlet/UserAgentSessionUtil.java +++ b/stroom-util/src/main/java/stroom/util/servlet/UserAgentSessionUtil.java @@ -1,10 +1,17 @@ package stroom.util.servlet; +import stroom.util.NullSafe; +import stroom.util.logging.LambdaLogger; +import stroom.util.logging.LambdaLoggerFactory; +import stroom.util.logging.LogUtil; + import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpSession; public final class UserAgentSessionUtil { + private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(UserAgentSessionUtil.class); + private static final String USER_AGENT = "user-agent"; private UserAgentSessionUtil() { @@ -16,6 +23,10 @@ public static void set(final HttpServletRequest request) { if (session != null) { final String userAgent = request.getHeader(USER_AGENT); if (userAgent != null) { + LOGGER.debug(() -> LogUtil.message("Setting {} '{}' in session {}", + USER_AGENT, + userAgent, + NullSafe.get(session, HttpSession::getId))); session.setAttribute(USER_AGENT, userAgent); } } diff --git a/unreleased_changes/20250115_150813_268__4693.md b/unreleased_changes/20250115_150813_268__4693.md new file mode 100644 index 00000000000..4d515ccc63b --- /dev/null +++ b/unreleased_changes/20250115_150813_268__4693.md @@ -0,0 +1,24 @@ +* Issue **#4693** : Add the property `stroom.session.maxInactiveInterval` to control the HTTP session expiry. Defaults to `1d`. + + +```sh +# ******************************************************************************** +# Issue title: sessionList is very slow v7.5 on a cluster +# Issue link: https://github.com/gchq/stroom/issues/4693 +# ******************************************************************************** + +# ONLY the top line will be included as a change entry in the CHANGELOG. +# The entry should be in GitHub flavour markdown and should be written on a SINGLE +# line with no hard breaks. You can have multiple change files for a single GitHub issue. +# The entry should be written in the imperative mood, i.e. 'Fix nasty bug' rather than +# 'Fixed nasty bug'. +# +# Examples of acceptable entries are: +# +# +# * Issue **123** : Fix bug with an associated GitHub issue in this repository +# +# * Issue **namespace/other-repo#456** : Fix bug with an associated GitHub issue in another repository +# +# * Fix bug with no associated GitHub issue. +```