From 63663553085448b6ec4111094bf4521967f6f24d Mon Sep 17 00:00:00 2001 From: stroomdev66 Date: Fri, 5 Sep 2025 12:29:34 +0100 Subject: [PATCH 01/10] Improve HTTP session creation and usage --- .../security/shared/SessionResource.java | 7 --- .../common/impl/UserIdentitySessionUtil.java | 34 +---------- .../stroom/security/impl/OpenIdManager.java | 22 +------ .../stroom/security/impl/SecurityFilter.java | 48 +++++++++++---- .../security/impl/SessionResourceImpl.java | 60 ------------------- 5 files changed, 39 insertions(+), 132 deletions(-) diff --git a/stroom-core-shared/src/main/java/stroom/security/shared/SessionResource.java b/stroom-core-shared/src/main/java/stroom/security/shared/SessionResource.java index 417eae75a74..fc6ffc49d6e 100644 --- a/stroom-core-shared/src/main/java/stroom/security/shared/SessionResource.java +++ b/stroom-core-shared/src/main/java/stroom/security/shared/SessionResource.java @@ -29,13 +29,6 @@ public interface SessionResource extends RestResource, DirectRestService { String LIST_PATH_PART = "/list"; String NODE_NAME_PARAM = "nodeName"; - @GET - @Path("/noauth/validateSession") - @Operation( - summary = "Validate the current session, return a redirect Uri if invalid.", - operationId = "validateStroomSession") - ValidateSessionResponse validateSession(@QueryParam("redirect_uri") @NotNull String redirectUri); - @GET @Path("logout") @Operation( diff --git a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java index 48de6e65e14..09c22aa03f0 100644 --- a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java +++ b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java @@ -4,7 +4,6 @@ import stroom.util.logging.LambdaLogger; import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; -import stroom.util.servlet.UserAgentSessionUtil; import stroom.util.shared.NullSafe; import jakarta.servlet.http.HttpServletRequest; @@ -12,14 +11,13 @@ import java.util.Arrays; import java.util.Optional; +import javax.swing.text.html.Option; public final class UserIdentitySessionUtil { private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(UserIdentitySessionUtil.class); private static final String SESSION_USER_IDENTITY = "SESSION_USER_IDENTITY"; - private static final String STROOM_SESSION_ID = "STROOM_SESSION_ID"; - private static final String JSESSIONID = "JSESSIONID"; private UserIdentitySessionUtil() { } @@ -32,41 +30,11 @@ public static void set(final HttpSession session, final UserIdentity userIdentit userIdentity, NullSafe.get(userIdentity, UserIdentity::getClass, Class::getSimpleName), NullSafe.get(session, HttpSession::getId))); - session.setAttribute(SESSION_USER_IDENTITY, userIdentity); } - /** - * Set the userIdentity on the session, creating the session as required - */ - public static void set(final HttpServletRequest request, final UserIdentity userIdentity) { - // Set the user ref in the session. - final HttpSession session = request.getSession(true); - set(session, userIdentity); - // Now we have the session make note of the user-agent for logging and sessionListServlet duties - UserAgentSessionUtil.setUserAgentInSession(request, session); - } - public static Optional get(final HttpSession session) { return Optional.ofNullable(session) .map(session2 -> (UserIdentity) session2.getAttribute(SESSION_USER_IDENTITY)); } - - public static Optional get(final HttpServletRequest request) { - return Optional.ofNullable(request) - .flatMap(req -> get(req.getSession(false))); - } - - public static boolean requestHasSessionCookie(final HttpServletRequest request) { - if (request == null || request.getCookies() == null) { - return false; - } - - // Find out if we have a session cookie - return Arrays - .stream(request.getCookies()) - .anyMatch(cookie -> - cookie.getName().equalsIgnoreCase(STROOM_SESSION_ID) || - cookie.getName().equalsIgnoreCase(JSESSIONID)); - } } diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java index 4df0827700c..453914e48bd 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java @@ -113,7 +113,7 @@ private String backChannelOIDC(final HttpServletRequest request, if (optionalUserIdentity.isPresent()) { // Set the token in the session. - UserIdentitySessionUtil.set(request, optionalUserIdentity.get()); + UserIdentitySessionUtil.set(request.getSession(true), optionalUserIdentity.get()); // Successful login, so redirect to the original URL held in the state. LOGGER.info(() -> "Redirecting to initiating URI: " + state.getInitiatingUri()); @@ -142,26 +142,6 @@ public Optional loginWithRequestToken(final HttpServletRequest req } } - public Optional getOrSetSessionUser(final HttpServletRequest request, - final Optional userIdentity) { - Optional result = userIdentity; - - if (userIdentity.isEmpty()) { - // Provide identity from the session if we are allowing this to happen. - result = UserIdentitySessionUtil.get(request.getSession(false)); - - if (LOGGER.isDebugEnabled()) { - LOGGER.debug("User identity from session: [{}]", result.orElse(null)); - } - - } else if (UserIdentitySessionUtil.requestHasSessionCookie(request)) { - // Set the user ref in the session. - UserIdentitySessionUtil.set(request.getSession(true), userIdentity.get()); - } - - return result; - } - public String logout(final String postAuthRedirectUri) { final String endpoint = openIdConfiguration.getLogoutEndpoint(); final String clientId = openIdConfiguration.getClientId(); 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 5068fcf7a7a..a4eab24160a 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 @@ -26,6 +26,7 @@ import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; import stroom.util.net.UrlUtils; +import stroom.util.servlet.UserAgentSessionUtil; import stroom.util.shared.AuthenticationBypassChecker; import stroom.util.shared.NullSafe; import stroom.util.shared.ResourcePaths; @@ -62,6 +63,8 @@ class SecurityFilter implements Filter { ".jpg", ".gif", ".ico", ".svg", ".ttf", ".woff", ".woff2"); private static final String SIGN_IN_URL_PATH = ResourcePaths.buildServletPath(ResourcePaths.SIGN_IN_PATH); + private static final String STROOM_SESSION_ID = "STROOM_SESSION_ID"; + private static final String JSESSIONID = "JSESSIONID"; private final UriFactory uriFactory; private final SecurityContext securityContext; @@ -152,27 +155,35 @@ private void filter(final HttpServletRequest request, // Need to do this first, so we get a fresh token from AWS ALB rather than using a stale // one from session. optUserIdentity = openIdManager.loginWithRequestToken(request); + optUserIdentity.ifPresent(userIdentity -> + ensureSessionIfCookiePresent(request).ifPresent(session -> { + LOGGER.debug(() -> LogUtil.message("Setting user in session, user: {} {}, path: {}", + userIdentity.getClass().getSimpleName(), + userIdentity, + fullPath)); + UserIdentitySessionUtil.set(session, userIdentity); + })); + + // Log current user. if (LOGGER.isDebugEnabled()) { logUserIdentityToDebug( optUserIdentity, fullPath, "after trying to login with request token"); } // If no user from header token, see if we have one in session already. - optUserIdentity = openIdManager.getOrSetSessionUser(request, optUserIdentity); - if (LOGGER.isDebugEnabled()) { - logUserIdentityToDebug(optUserIdentity, fullPath, "from session"); + if (optUserIdentity.isEmpty()) { + optUserIdentity = UserIdentitySessionUtil.get(request.getSession(false)); + if (LOGGER.isDebugEnabled()) { + logUserIdentityToDebug(optUserIdentity, fullPath, "from session"); + } } if (optUserIdentity.isPresent()) { final UserIdentity userIdentity = optUserIdentity.get(); - LOGGER.debug(() -> LogUtil.message("Setting user in session, user: {} {}, path: {}", - userIdentity.getClass().getSimpleName(), - userIdentity, - fullPath)); - // Set the identity in session if we have a session and cookie - if (UserIdentitySessionUtil.requestHasSessionCookie(request)) { - UserIdentitySessionUtil.set(request, userIdentity); - } + + // Now we have the session make note of the user-agent for logging and sessionListServlet duties + ensureSessionIfCookiePresent(request).ifPresent(session -> + UserAgentSessionUtil.setUserAgentInSession(request, session)); // Now handle the request as this user securityContext.asUser(userIdentity, () -> @@ -319,4 +330,19 @@ private void process(final HttpServletRequest request, @Override public void destroy() { } + + private Optional ensureSessionIfCookiePresent(final HttpServletRequest request) { + if (requestHasSessionCookie(request)) { + return Optional.of(request.getSession(true)); + } + return Optional.empty(); + } + + private boolean requestHasSessionCookie(final HttpServletRequest request) { + // Find out if we have a session cookie + return NullSafe.stream(NullSafe.get(request, HttpServletRequest::getCookies)) + .anyMatch(cookie -> + cookie.getName().equalsIgnoreCase(STROOM_SESSION_ID) || + cookie.getName().equalsIgnoreCase(JSESSIONID)); + } } diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionResourceImpl.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionResourceImpl.java index 3772d00ea2f..404d6beae6b 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionResourceImpl.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionResourceImpl.java @@ -2,13 +2,10 @@ import stroom.event.logging.rs.api.AutoLogged; import stroom.event.logging.rs.api.AutoLogged.OperationType; -import stroom.security.api.UserIdentity; import stroom.security.common.impl.UserIdentitySessionUtil; -import stroom.security.openid.api.OpenId; import stroom.security.shared.SessionListResponse; import stroom.security.shared.SessionResource; import stroom.security.shared.UrlResponse; -import stroom.security.shared.ValidateSessionResponse; import jakarta.inject.Inject; import jakarta.inject.Provider; @@ -17,10 +14,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; -import java.util.Optional; - @AutoLogged(OperationType.MANUALLY_LOGGED) class SessionResourceImpl implements SessionResource { @@ -45,59 +38,6 @@ class SessionResourceImpl implements SessionResource { this.stroomUserIdentityFactoryProvider = stroomUserIdentityFactoryProvider; } - @Override - @AutoLogged(OperationType.UNLOGGED) - public ValidateSessionResponse validateSession(final String postAuthRedirectUri) { - final OpenIdManager openIdManager = openIdManagerProvider.get(); - final HttpServletRequest request = httpServletRequestProvider.get(); - Optional userIdentity = openIdManager.loginWithRequestToken(request); - userIdentity = openIdManager.getOrSetSessionUser(request, userIdentity); - if (userIdentity.isPresent()) { - return new ValidateSessionResponse(true, userIdentity.get().getSubjectId(), null); - } - - // If the session doesn't have a user ref then attempt login. - try { - LOGGER.debug("Using postAuthRedirectUri: {}", postAuthRedirectUri); - - // We might have completed the back channel authentication now so see if we have a user session. - userIdentity = UserIdentitySessionUtil.get(request.getSession(false)); - return userIdentity - .map(identity -> - createValidResponse(identity.getSubjectId())) - .orElseGet(() -> createRedirectResponse(request, postAuthRedirectUri)); - - } catch (final RuntimeException e) { - LOGGER.error(e.getMessage(), e); - throw e; - } - } - - private ValidateSessionResponse createValidResponse(final String userId) { - return new ValidateSessionResponse(true, userId, null); - } - - private ValidateSessionResponse createRedirectResponse(final HttpServletRequest request, final String url) { - final OpenIdManager openIdManager = openIdManagerProvider.get(); - final String code = getParam(url, OpenId.CODE); - final String stateId = getParam(url, OpenId.STATE); - final String redirectUri = openIdManager.redirect(request, code, stateId, url); - return new ValidateSessionResponse(false, null, redirectUri); - } - - private String getParam(final String url, final String param) { - int start = url.indexOf(param + "="); - if (start != -1) { - start += param.length() + 1; - final int end = url.indexOf("&", start); - if (end != -1) { - return URLDecoder.decode(url.substring(start, end), StandardCharsets.UTF_8); - } - return URLDecoder.decode(url.substring(start), StandardCharsets.UTF_8); - } - return null; - } - @Override @AutoLogged(OperationType.MANUALLY_LOGGED) public UrlResponse logout(final String redirectUri) { From 7c2647ef87ec147ec180973a534b4bb099183903 Mon Sep 17 00:00:00 2001 From: at055612 <22818309+at055612@users.noreply.github.com> Date: Mon, 8 Sep 2025 12:48:51 +0100 Subject: [PATCH 02/10] Fix OIDC code flow to stop session being lost --- stroom-app/src/main/java/stroom/app/App.java | 6 +- .../resources/ui/noauth/swagger/stroom.json | 70 ++++++--------- .../resources/ui/noauth/swagger/stroom.yaml | 48 +++++----- .../shared/ValidateSessionResponse.java | 38 -------- .../common/impl/UserIdentitySessionUtil.java | 26 ++++-- .../stroom/security/impl/OpenIdManager.java | 85 +++++++++++++----- .../stroom/security/impl/SecurityFilter.java | 90 ++++++++++++------- .../impl/StroomUserIdentityFactory.java | 18 +++- .../security/impl/UserIdentityImpl.java | 8 +- .../java/stroom/util/servlet/SessionUtil.java | 33 +++++++ unreleased_changes/20250908_114717_485__0.md | 55 ++++++++++++ 11 files changed, 300 insertions(+), 177 deletions(-) delete mode 100644 stroom-core-shared/src/main/java/stroom/security/shared/ValidateSessionResponse.java create mode 100644 stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java create mode 100644 unreleased_changes/20250908_114717_485__0.md diff --git a/stroom-app/src/main/java/stroom/app/App.java b/stroom-app/src/main/java/stroom/app/App.java index b21454284f1..0f66f8ab681 100644 --- a/stroom-app/src/main/java/stroom/app/App.java +++ b/stroom-app/src/main/java/stroom/app/App.java @@ -54,6 +54,7 @@ import stroom.util.logging.LambdaLogger; import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; +import stroom.util.servlet.SessionUtil; import stroom.util.shared.AbstractConfig; import stroom.util.shared.ModelStringUtil; import stroom.util.shared.NullSafe; @@ -88,7 +89,6 @@ public class App extends Application { private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(App.class); private static final String APP_NAME = "Stroom"; - public static final String SESSION_COOKIE_NAME = "STROOM_SESSION_ID"; @Inject private HealthChecks healthChecks; @@ -358,7 +358,9 @@ private void configureSessionHandling(final Environment environment, 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); + sessionHandler.setSessionCookie(SessionUtil.STROOM_SESSION_COOKIE_NAME); + // In case we use URL encoding of the session ID, which we currently don't + sessionHandler.setSessionIdPathParameterName(SessionUtil.STROOM_SESSION_COOKIE_NAME); long maxInactiveIntervalSecs = NullSafe.getOrElse( sessionConfig.getMaxInactiveInterval(), StroomDuration::getDuration, diff --git a/stroom-app/src/main/resources/ui/noauth/swagger/stroom.json b/stroom-app/src/main/resources/ui/noauth/swagger/stroom.json index 128925577cf..0bba87556db 100644 --- a/stroom-app/src/main/resources/ui/noauth/swagger/stroom.json +++ b/stroom-app/src/main/resources/ui/noauth/swagger/stroom.json @@ -2172,6 +2172,32 @@ "tags" : [ "Global Config" ] } }, + "/content/v1/abortImport" : { + "post" : { + "operationId" : "abortImport", + "requestBody" : { + "content" : { + "application/json" : { + "schema" : { + "$ref" : "#/components/schemas/ResourceKey" + } + } + }, + "description" : "request", + "required" : true + }, + "responses" : { + "default" : { + "content" : { + "application/json" : { } + }, + "description" : "default response" + } + }, + "summary" : "Abort Import", + "tags" : [ "Content" ] + } + }, "/content/v1/export" : { "post" : { "operationId" : "exportContent", @@ -9734,33 +9760,6 @@ "tags" : [ "Sessions" ] } }, - "/session/v1/noauth/validateSession" : { - "get" : { - "operationId" : "validateStroomSession", - "parameters" : [ { - "in" : "query", - "name" : "redirect_uri", - "required" : true, - "schema" : { - "type" : "string" - } - } ], - "responses" : { - "default" : { - "content" : { - "application/json" : { - "schema" : { - "$ref" : "#/components/schemas/ValidateSessionResponse" - } - } - }, - "description" : "default response" - } - }, - "summary" : "Validate the current session, return a redirect Uri if invalid.", - "tags" : [ "Sessions" ] - } - }, "/sessionInfo/v1" : { "get" : { "operationId" : "getSessionInfo", @@ -17257,6 +17256,9 @@ "$ref" : "#/components/schemas/Message" } }, + "ownerDocRef" : { + "$ref" : "#/components/schemas/DocRef" + }, "sourcePath" : { "type" : "string" }, @@ -23996,20 +23998,6 @@ } } }, - "ValidateSessionResponse" : { - "type" : "object", - "properties" : { - "redirectUri" : { - "type" : "string" - }, - "userId" : { - "type" : "string" - }, - "valid" : { - "type" : "boolean" - } - } - }, "ValidationResult" : { "type" : "object", "properties" : { diff --git a/stroom-app/src/main/resources/ui/noauth/swagger/stroom.yaml b/stroom-app/src/main/resources/ui/noauth/swagger/stroom.yaml index 49d02357443..e222575e116 100644 --- a/stroom-app/src/main/resources/ui/noauth/swagger/stroom.yaml +++ b/stroom-app/src/main/resources/ui/noauth/swagger/stroom.yaml @@ -1492,6 +1492,24 @@ paths: summary: "Fetch a property by its name, e.g. 'stroom.path.home'" tags: - Global Config + /content/v1/abortImport: + post: + operationId: abortImport + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/ResourceKey" + description: request + required: true + responses: + default: + content: + application/json: {} + description: default response + summary: Abort Import + tags: + - Content /content/v1/export: post: operationId: exportContent @@ -6679,25 +6697,6 @@ paths: summary: Logout of Stroom session tags: - Sessions - /session/v1/noauth/validateSession: - get: - operationId: validateStroomSession - parameters: - - in: query - name: redirect_uri - required: true - schema: - type: string - responses: - default: - content: - application/json: - schema: - $ref: "#/components/schemas/ValidateSessionResponse" - description: default response - summary: "Validate the current session, return a redirect Uri if invalid." - tags: - - Sessions /sessionInfo/v1: get: operationId: getSessionInfo @@ -12492,6 +12491,8 @@ components: type: array items: $ref: "#/components/schemas/Message" + ownerDocRef: + $ref: "#/components/schemas/DocRef" sourcePath: type: string state: @@ -17871,15 +17872,6 @@ components: type: boolean string: type: string - ValidateSessionResponse: - type: object - properties: - redirectUri: - type: string - userId: - type: string - valid: - type: boolean ValidationResult: type: object properties: diff --git a/stroom-core-shared/src/main/java/stroom/security/shared/ValidateSessionResponse.java b/stroom-core-shared/src/main/java/stroom/security/shared/ValidateSessionResponse.java deleted file mode 100644 index b17d56a713a..00000000000 --- a/stroom-core-shared/src/main/java/stroom/security/shared/ValidateSessionResponse.java +++ /dev/null @@ -1,38 +0,0 @@ -package stroom.security.shared; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.annotation.JsonInclude.Include; -import com.fasterxml.jackson.annotation.JsonProperty; - -@JsonInclude(Include.NON_NULL) -public class ValidateSessionResponse { - - @JsonProperty - private final boolean valid; - @JsonProperty - private final String userId; - @JsonProperty - private final String redirectUri; - - @JsonCreator - public ValidateSessionResponse(@JsonProperty("valid") final boolean valid, - @JsonProperty("userId") final String userId, - @JsonProperty("redirectUri") final String redirectUri) { - this.valid = valid; - this.userId = userId; - this.redirectUri = redirectUri; - } - - public boolean isValid() { - return valid; - } - - public String getUserId() { - return userId; - } - - public String getRedirectUri() { - return redirectUri; - } -} diff --git a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java index 09c22aa03f0..44e4c1ddbe1 100644 --- a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java +++ b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java @@ -6,12 +6,9 @@ import stroom.util.logging.LogUtil; import stroom.util.shared.NullSafe; -import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpSession; -import java.util.Arrays; import java.util.Optional; -import javax.swing.text.html.Option; public final class UserIdentitySessionUtil { @@ -26,15 +23,28 @@ private UserIdentitySessionUtil() { * Set the {@link UserIdentity} on the session */ public static void set(final HttpSession session, final UserIdentity userIdentity) { - LOGGER.debug(() -> LogUtil.message("Setting userIdentity {} of type {} in session {}", - userIdentity, + LOGGER.debug(() -> LogUtil.message("Setting userIdentity of type {} in session {}, userIdentity: {}", NullSafe.get(userIdentity, UserIdentity::getClass, Class::getSimpleName), - NullSafe.get(session, HttpSession::getId))); + NullSafe.get(session, HttpSession::getId), + userIdentity)); session.setAttribute(SESSION_USER_IDENTITY, userIdentity); } public static Optional get(final HttpSession session) { - return Optional.ofNullable(session) - .map(session2 -> (UserIdentity) session2.getAttribute(SESSION_USER_IDENTITY)); + final Optional optUserIdentity = Optional.ofNullable(session) + .map(session2 -> + (UserIdentity) session2.getAttribute(SESSION_USER_IDENTITY)); + if (LOGGER.isTraceEnabled()) { + optUserIdentity.ifPresentOrElse(userIdentity -> { + LOGGER.trace(() -> LogUtil.message("Got userIdentity of type {} in session {}, userIdentity: {}", + NullSafe.get(userIdentity, UserIdentity::getClass, Class::getSimpleName), + NullSafe.get(session, HttpSession::getId), + userIdentity)); + }, () -> { + LOGGER.trace(() -> LogUtil.message("No userIdentity in session {}", + NullSafe.get(session, HttpSession::getId))); + }); + } + return optUserIdentity; } } diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java index 453914e48bd..ad1fd017274 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java @@ -3,7 +3,6 @@ import stroom.config.common.UriFactory; import stroom.security.api.UserIdentity; import stroom.security.common.impl.AuthenticationState; -import stroom.security.common.impl.UserIdentitySessionUtil; import stroom.security.openid.api.OpenId; import stroom.security.openid.api.OpenIdConfiguration; import stroom.util.jersey.UriBuilderUtil; @@ -15,6 +14,7 @@ import jakarta.inject.Inject; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpSession; import jakarta.ws.rs.core.UriBuilder; import java.net.URI; @@ -43,11 +43,16 @@ public OpenIdManager(final OpenIdConfiguration openIdConfiguration, this.uriFactory = uriFactory; } - public String redirect(final HttpServletRequest request, - final String code, - final String stateId, - final String postAuthRedirectUri) { - String redirectUri = null; + public RedirectUrl redirect(final HttpServletRequest request, + final String code, + final String stateId, + final String postAuthRedirectUri) { + RedirectUrl redirectUri = null; + LOGGER.debug("redirect() - requestURI: {}, code: {}, stateId: {}, postAuthRedirectUri: {}", + request.getRequestURI(), + code, + stateId, + postAuthRedirectUri); // Retrieve state if we have a state id param. final Optional optionalState = getState(stateId); @@ -60,11 +65,13 @@ public String redirect(final HttpServletRequest request, // If we aren't doing back channel check yet or the back channel check failed then proceed with front channel. if (redirectUri == null) { // Restore the initiating URI as needed for logout. - redirectUri = optionalState + final String url = optionalState .map(state -> frontChannelOIDC(state.getInitiatingUri(), state.isPrompt())) .orElse(frontChannelOIDC(postAuthRedirectUri, false)); + redirectUri = RedirectUrl.create(url); } + LOGGER.debug("redirect() - redirectUri: {}", redirectUri); return redirectUri; } @@ -97,34 +104,40 @@ private String frontChannelOIDC(final String postAuthRedirectUri, return createAuthUri(endpoint, clientId, state); } - private String backChannelOIDC(final HttpServletRequest request, - final String code, - final AuthenticationState state) { + private RedirectUrl backChannelOIDC(final HttpServletRequest request, + final String code, + final AuthenticationState state) { Objects.requireNonNull(code, "Null code"); - - String redirectUri; + RedirectUrl redirectUri; // If we have a state id then this should be a return from the auth service. - LOGGER.debug(() -> LogUtil.message("We have the following backChannelOIDC state: {}", state)); + LOGGER.debug(() -> LogUtil.message("backChannelOIDC() - state: {}, sessionId: {}, cookies: {}", + state, + NullSafe.get(request.getSession(false), HttpSession::getId), + request.getCookies())); try { final Optional optionalUserIdentity = userIdentityFactory.getAuthFlowUserIdentity(request, code, state); + LOGGER.debug("backChannelOIDC() - optionalUserIdentity after back channel auth: {}", optionalUserIdentity); + if (optionalUserIdentity.isPresent()) { - // Set the token in the session. - UserIdentitySessionUtil.set(request.getSession(true), optionalUserIdentity.get()); + // Set the token in the session so that when we re-direct to the initiating page (i.e. '/') + // we will have the identity in session so won't go back round the code flow loop +// UserIdentitySessionUtil.set(request.getSession(true), optionalUserIdentity.get()); // Successful login, so redirect to the original URL held in the state. - LOGGER.info(() -> "Redirecting to initiating URI: " + state.getInitiatingUri()); - redirectUri = state.getInitiatingUri(); + final String uri = state.getInitiatingUri(); + LOGGER.info(() -> LogUtil.message("backChannelOIDC() - Redirecting to initiating URI: {}", uri)); + redirectUri = RedirectUrl.createWithRefresh(uri); } else { - LOGGER.debug("No userIdentity so redirect to error page"); - redirectUri = createErrorUri("Authentication failed"); + redirectUri = RedirectUrl.create(createErrorUri("Authentication failed")); + LOGGER.debug("backChannelOIDC() - No userIdentity so redirect to error page: {}", redirectUri); } } catch (final Exception e) { - LOGGER.debug("backChannelOIDC() - {}", e.getMessage(), e); - redirectUri = createErrorUri(e.getMessage()); + LOGGER.debug("backChannelOIDC() - Error: {}", LogUtil.exceptionMessage(e), e); + redirectUri = RedirectUrl.create(createErrorUri(e.getMessage())); } return redirectUri; @@ -215,4 +228,34 @@ private String createErrorUri(final String message) { LOGGER.debug("Sending user to error screen with uri: {}", uriStr); return uriStr; } + + + // -------------------------------------------------------------------------------- + + + public record RedirectUrl(String redirectUrl, RedirectMode redirectMode) { + + public RedirectUrl(final String redirectUrl, final RedirectMode redirectMode) { + this.redirectUrl = redirectUrl; + this.redirectMode = Objects.requireNonNullElse(redirectMode, RedirectMode.REDIRECT); + } + + static RedirectUrl create(final String url) { + return new RedirectUrl(url, RedirectMode.REDIRECT); + } + + static RedirectUrl createWithRefresh(final String url) { + return new RedirectUrl(url, RedirectMode.REFRESH); + } + } + + + // -------------------------------------------------------------------------------- + + + public enum RedirectMode { + REDIRECT, + REFRESH, + ; + } } 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 a4eab24160a..7c91dc240f0 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 @@ -21,11 +21,13 @@ import stroom.security.api.UserIdentity; import stroom.security.api.exception.AuthenticationException; import stroom.security.common.impl.UserIdentitySessionUtil; +import stroom.security.impl.OpenIdManager.RedirectUrl; import stroom.security.openid.api.OpenId; import stroom.util.logging.LambdaLogger; import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; import stroom.util.net.UrlUtils; +import stroom.util.servlet.SessionUtil; import stroom.util.servlet.UserAgentSessionUtil; import stroom.util.shared.AuthenticationBypassChecker; import stroom.util.shared.NullSafe; @@ -45,8 +47,10 @@ import jakarta.servlet.http.HttpSession; import jakarta.ws.rs.HttpMethod; import jakarta.ws.rs.core.Response; +import org.apache.hc.core5.http.ContentType; import java.io.IOException; +import java.io.PrintWriter; import java.util.Optional; import java.util.Set; @@ -63,8 +67,6 @@ class SecurityFilter implements Filter { ".jpg", ".gif", ".ico", ".svg", ".ttf", ".woff", ".woff2"); private static final String SIGN_IN_URL_PATH = ResourcePaths.buildServletPath(ResourcePaths.SIGN_IN_PATH); - private static final String STROOM_SESSION_ID = "STROOM_SESSION_ID"; - private static final String JSESSIONID = "JSESSIONID"; private final UriFactory uriFactory; private final SecurityContext securityContext; @@ -204,33 +206,61 @@ private void filter(final HttpServletRequest request, response.setStatus(Response.Status.UNAUTHORIZED.getStatusCode()); } else { - // No identity found and not an unauthenticated servlet/api so assume it is - // a UI request. Thus instigate an OpenID authentication flow - try { - final String postAuthRedirectUri = getPostAuthRedirectUri(request); - - final String code = UrlUtils.getLastParam(request, OpenId.CODE); - 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, fullPath); - - // HTTP 1.1. - response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); - // HTTP 1.0. - response.setHeader("Pragma", "no-cache"); - // Proxies. - response.setHeader("Expires", "0"); + doOpenIdFlow(request, response, fullPath); + } + } + } - response.setStatus(HttpServletResponse.SC_TEMPORARY_REDIRECT); - response.setHeader("Location", redirectUri); + private void doOpenIdFlow(final HttpServletRequest request, + final HttpServletResponse response, + final String fullPath) throws IOException { + // No identity found and not an unauthenticated servlet/api so assume it is + // a UI request. Thus instigate an OpenID authentication flow + try { + final String postAuthRedirectUri = getPostAuthRedirectUri(request); + final String code = UrlUtils.getLastParam(request, OpenId.CODE); + final String stateId = UrlUtils.getLastParam(request, OpenId.STATE); + final RedirectUrl redirectUri = openIdManager.redirect( + request, code, stateId, postAuthRedirectUri); + // HTTP 1.1. + response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); + // HTTP 1.0. + response.setHeader("Pragma", "no-cache"); + // Proxies. + response.setHeader("Expires", "0"); + + switch (redirectUri.redirectMode()) { + case REFRESH -> { + // If the session has only just been created, and we do a normal redirect, + // then the browser will never get the session cookie as the response is terminated + // for the redirect. Thus, the session ID will not be known at the redirect URI. + // A refresh will ensure the cookie is set. + LOGGER.debug("Responding with a http-equiv refresh to {}", redirectUri); + response.setContentType(ContentType.TEXT_HTML.getMimeType()); + try (final PrintWriter responseWriter = response.getWriter()) { + responseWriter.print(""); + responseWriter.print(""); + responseWriter.print(""); + responseWriter.print(""); + responseWriter.print(""); + } + } + case REDIRECT -> { + // Do a standard http redirect, e.g. to the IDP + final String url = redirectUri.redirectUrl(); + LOGGER.debug("Code flow UI request so redirecting to:, " + + "redirectUri: {}, url: {}, postAuthRedirectUri: {}, path: {}", + redirectUri, url, postAuthRedirectUri, fullPath); - } catch (final RuntimeException e) { - LOGGER.error(e.getMessage(), e); - throw e; + response.setStatus(HttpServletResponse.SC_TEMPORARY_REDIRECT); + response.setHeader("Location", url); } } + } catch (final RuntimeException e) { + LOGGER.error(e.getMessage(), e); + throw e; } } @@ -332,17 +362,9 @@ public void destroy() { } private Optional ensureSessionIfCookiePresent(final HttpServletRequest request) { - if (requestHasSessionCookie(request)) { + if (SessionUtil.requestHasSessionCookie(request)) { return Optional.of(request.getSession(true)); } return Optional.empty(); } - - private boolean requestHasSessionCookie(final HttpServletRequest request) { - // Find out if we have a session cookie - return NullSafe.stream(NullSafe.get(request, HttpServletRequest::getCookies)) - .anyMatch(cookie -> - cookie.getName().equalsIgnoreCase(STROOM_SESSION_ID) || - cookie.getName().equalsIgnoreCase(JSESSIONID)); - } } diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java index 2f2983cf68f..6ad7cdd87a8 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java @@ -15,6 +15,7 @@ import stroom.security.common.impl.JwtUtil; import stroom.security.common.impl.RefreshManager; import stroom.security.common.impl.UpdatableToken; +import stroom.security.common.impl.UserIdentitySessionUtil; import stroom.security.impl.apikey.ApiKeyService; import stroom.security.impl.event.PermissionChangeEvent; import stroom.security.openid.api.IdpType; @@ -318,8 +319,12 @@ private UserIdentity createAuthFlowUserIdentity(final JwtClaims jwtClaims, final TokenResponse tokenResponse, final User user) { Objects.requireNonNull(user); - final HttpSession session = request.getSession(false); + // At this point we have authenticated the User so need to ensure + // we have a session as that gets attached to the userIdentity object to + // allow us to refresh it's token. + final HttpSession session = request.getSession(true); + // Make a token object that we can update as/when we do a token refresh final UpdatableToken updatableToken = new UpdatableToken( tokenResponse, jwtClaims, @@ -334,10 +339,19 @@ private UserIdentity createAuthFlowUserIdentity(final JwtClaims jwtClaims, session, updatableToken); + // We have authenticated so set the userIdentity in the session for future requests + // to retrieve it from + UserIdentitySessionUtil.set(session, userIdentity); + updatableToken.setUserIdentity(userIdentity); addTokenToRefreshManager(updatableToken); - LOGGER.info(() -> "Authenticated user " + userIdentity + LOGGER.debug(() -> LogUtil.message( + "createAuthFlowUserIdentity() - Authenticated user - session: {}, userIdentity: {}", + NullSafe.get(session, HttpSession::getId), + userIdentity)); + + LOGGER.info(() -> "createAuthFlowUserIdentity() - Authenticated user " + userIdentity + " for sessionId " + NullSafe.get(session, HttpSession::getId)); return userIdentity; } diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/UserIdentityImpl.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/UserIdentityImpl.java index 0abb34938b0..81270a6d68f 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/UserIdentityImpl.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/UserIdentityImpl.java @@ -74,11 +74,11 @@ public UserRef getUserRef() { @Override public String getSessionId() { - return httpSession.getId(); + return NullSafe.get(httpSession, HttpSession::getId); } public void invalidateSession() { - httpSession.invalidate(); + NullSafe.consume(httpSession, HttpSession::invalidate); } /** @@ -86,7 +86,9 @@ public void invalidateSession() { * to re-authenticate with the IDP. */ public void removeUserFromSession() { - UserIdentitySessionUtil.set(httpSession, null); + if (httpSession != null) { + UserIdentitySessionUtil.set(httpSession, null); + } } // /** diff --git a/stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java b/stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java new file mode 100644 index 00000000000..33d728db670 --- /dev/null +++ b/stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java @@ -0,0 +1,33 @@ +package stroom.util.servlet; + +import stroom.util.logging.LambdaLogger; +import stroom.util.logging.LambdaLoggerFactory; +import stroom.util.shared.NullSafe; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpSession; + +public class SessionUtil { + + private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(SessionUtil.class); + + public static final String STROOM_SESSION_COOKIE_NAME = "STROOM_SESSION_ID"; + public static final String JSESSIONID = "JSESSIONID"; + + private SessionUtil() { + } + + public static boolean requestHasSessionCookie(final HttpServletRequest request) { + // Find out if we have a session cookie + final boolean hasCookie = NullSafe.stream(NullSafe.get(request, HttpServletRequest::getCookies)) + .anyMatch(cookie -> + cookie.getName().equalsIgnoreCase(STROOM_SESSION_COOKIE_NAME) || + cookie.getName().equalsIgnoreCase(JSESSIONID)); + + LOGGER.debug("requestHasSessionCookie() - sessionId: {}, hasCookie: {}", + NullSafe.get(request, HttpServletRequest::getSession, HttpSession::getId), + hasCookie); + + return hasCookie; + } +} diff --git a/unreleased_changes/20250908_114717_485__0.md b/unreleased_changes/20250908_114717_485__0.md new file mode 100644 index 00000000000..7ff16ac248b --- /dev/null +++ b/unreleased_changes/20250908_114717_485__0.md @@ -0,0 +1,55 @@ +* Fix the OpenID code flow to stop the session being lost after redirection back to the initiating URL. + + +```sh +# 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. + + +# -------------------------------------------------------------------------------- +# The following is random text to make this file unique for git's change detection +# t1t0pkLs2C9H0kI0I5FfttWO2x0IuutrR7sFcKH7IOWs3k02udf63vJA1WnHUTpNSlTPrRc5Ow6Hoeg5 +# wip9aiQEv7TYhHC72mOlfwAmg1CdyOi2EmJ4e4uOSrOxSUhltcYlzFn6UAjyFxMdHI0Qk60qK5drE9rp +# z5umFfONXT5mg9TjuueVjgYOHGqvizYE2ZpLTclizcH3QiLrQ4tZvVuGc2e5v9HWFJ6MyZvytbtqssfg +# b2dL7nIdrWuHhFfLHUvnICdLnEJWU1t1dHRm41SpOONU2H4Enjm7sop4A0NQ6618RvKyibRysdUGicKs +# NDWeiYdinbyGCKR4OzsIA3AYlgwmKKYXypk6Y4g9IgpXYOiCjY3w8Qmca8bBoNtIjeDM8Qw1lHHgvT4G +# UGOFLWTFFngaWz2EBchW6fUFaIpUmSxQiKRIAKtLAx6FgH23WHasZ5Cm5yoOJYBpkYpy21Zi8JlbvGIH +# ALgV0VovJ1e5ol2ksGZAqGjYOQh6A0ksknXF1UOhTLYCRMs9ru1nCRxmyCx6B9CqLUfPpwVsNoi27jOl +# 6D4uwoxYxemUAdSQOSOeMMT7MCiaCpzYuhHPXpVM9UlOOy5PE3GB0Kah0VROEbkobTjkmee4c2HDBQWs +# THWRdvHWkc95aNQcmX6OGPCFvibHbeACreamoawHkiRyilijqKkAWUJfEVyxbL9PjiC6xogYe8fe4jxl +# CfRmYmZoU6IrL9NqA2zG9FNKhQzqnrmtsh4udYaDgOCFfpG4gaBrqJGzRGAWUrTy0pfZcb5JWSjPFbhQ +# fiRfJzvUnliWnM9uNFeKt79rLlY9O3pWxZKoJlgOUXYwmgdTewjWDme9blMO8wrwtM2zvlEdMe7m1M8n +# c1dphWZkkWpyZamWZdARgq7EMx7T6ceZyRbnKP2ion8rz2P4IRhjuRDb5zx88qL9WOBwcoSoXFtUX2jL +# pkrs6aV7atYYbDLOwTNRSASclxnSZj481EObtZGiYS4BMXdTpSGvhxpdoH63vnPxdb2o7o1MHOLjYylY +# o9HeJgRpQma2vVQEWR3tMQJzzUaXGjfqZoNZujrTqZCJYisgdIjGxk81OHq8wv2sKwznnI0jLVI3W9dS +# 11gKOmrQjjfVm5omqqfcCz61kbAelWhW60ZH5jxjHgf2el8ZGXO3rkYe3bzMc6dPbbh6j0r08NV5jlt0 +# ERovNFJlWheNxWeFwk8wxl0osGcUuJanCallJ2wvPsriQbjKYVC07KKaI8D5Mud8VRhifLqeMd8WojXY +# JaDRx8ojo7D2idpsS9DPXwEokoLhJZU7lJhGP9lZDJzOIkwzJ9Y4kaiOObfkomxqt6y9wb8EInIjztEJ +# sISmOMSg0heoCXfoCctbcTfSkBnnk3xvHD45eNOHE9vIAilgd6PzK4CaRy6Y7ZiO3F9ua6heWGH0BKIs +# rMxr2dz9glmVV2cY1fIO5lmtbwdjEIy6PRV9jHJ5uKA2zvbQyDLPBHb1SL0AboTx3AYEcr77ZBbjFSs5 +# Rc3eU2dHWfM7p14cHEd5ihuxOuCX5utWcQHXlYEsmmJui56jalJvHTFNmQfVyfnzGfnlQ5aMKogTzln9 +# NvQCiBdaaht96HR0lK2gJSv9035KrV8hwybdrOKvzGkwDKnLsgmyW1cf6BuJarulNpFNe4Ihz4HTMhBX +# KajE4gwpNcwhyCSSzeeCjseZZq449QXujW1G1r8xhYEt4vZp0Zfmbg0PHybwW778vYGOtJFoUQYpqBaj +# O7BhFaleuGQYQdh3hFk8ZLolGDcEYVjGmi0VGpcuSnYBHU70AfcSDTL1lDnE1sULw3vIzKp62oZ829Ek +# gkSg0MWQg7stozmaq6P9uu0ynvzjU5DmZBo5yWClKHIuvc4xyoUFpGNqakdAK8dJPFlNr7yHZlOJA8Xp +# yJrzzns66ykym1TseMCDZrVCJajFN6c9LFf5kTwhbab60wggTp9d2OnplrArLLvwiJzYxthuiLKwCWqZ +# 295tBPz3ruSCfvN2ifvQL1qgEoMwKvdSA92EnhVLgkzOscImnYEgltATb0niaBqWLo0mu6ufKyNFpS6k +# Duar0SGhhYIeuFWQWccouk9asZ939vEGN70XzfnFrq78HiHVQ3doeoo6hvzWyIg1YvqcQA3x7X98nJER +# tigXMviwjERzsatBmO88AJolCdV2qAcLe4RSogP7roCT1V2ebXq5D1A0s54LdWlrcXhi1pgClZOGfkPJ +# faGjUALZHDCZZY1iDghdfOC6ZIL9FCUnx4ndXJVF08fX0lcPJV1YIniwTQrIZ4Wt5zQYe7uLb1op3XE4 +# GkSQ37sHoB4NiCQwbrPjOOQPt0FSUH8fK0y0KvH39TCTBkA2v3cI5rClLRrTEjNqWGvmhy9v5tTR8afh +# -------------------------------------------------------------------------------- + +``` From 5d3882303e55a460af121d37b3a1c1a17c028873 Mon Sep 17 00:00:00 2001 From: at055612 <22818309+at055612@users.noreply.github.com> Date: Tue, 9 Sep 2025 10:17:21 +0100 Subject: [PATCH 03/10] Refactor getSession and get/set attr calls --- .../activity/impl/CurrentActivityImpl.java | 23 +--- .../impl/SessionResourceStoreImpl.java | 10 +- .../common/impl/UserIdentitySessionUtil.java | 13 ++- .../AuthenticationServiceImpl.java | 60 ++++++---- .../stroom/security/impl/OpenIdManager.java | 1 - .../stroom/security/impl/SecurityFilter.java | 11 +- .../impl/StroomUserIdentityFactory.java | 19 +-- .../java/stroom/util/logging/LogUtil.java | 8 ++ .../java/stroom/util/servlet/SessionUtil.java | 108 ++++++++++++++++++ 9 files changed, 186 insertions(+), 67 deletions(-) diff --git a/stroom-activity/stroom-activity-impl/src/main/java/stroom/activity/impl/CurrentActivityImpl.java b/stroom-activity/stroom-activity-impl/src/main/java/stroom/activity/impl/CurrentActivityImpl.java index 39b5ab649fa..fee0a46ac55 100644 --- a/stroom-activity/stroom-activity-impl/src/main/java/stroom/activity/impl/CurrentActivityImpl.java +++ b/stroom-activity/stroom-activity-impl/src/main/java/stroom/activity/impl/CurrentActivityImpl.java @@ -19,11 +19,11 @@ import stroom.activity.api.CurrentActivity; import stroom.activity.shared.Activity; import stroom.security.api.SecurityContext; +import stroom.util.servlet.SessionUtil; import com.google.inject.Inject; import jakarta.inject.Provider; import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpSession; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,17 +49,7 @@ public Activity getActivity() { try { final HttpServletRequest request = httpServletRequestProvider.get(); - if (request != null) { - final HttpSession session = request.getSession(false); - if (session != null) { - final Object object = session.getAttribute(NAME); - if (object instanceof Activity) { - activity = (Activity) object; - } - } else { - LOGGER.debug("No Session"); - } - } + activity = SessionUtil.getAttribute(request, NAME, Activity.class, false); } catch (final RuntimeException e) { LOGGER.debug(e.getMessage(), e); } @@ -72,14 +62,7 @@ public Activity getActivity() { public void setActivity(final Activity activity) { securityContext.secure(() -> { final HttpServletRequest request = httpServletRequestProvider.get(); - if (request != null) { - final HttpSession session = request.getSession(false); - if (session != null) { - session.setAttribute(NAME, activity); - } else { - LOGGER.debug("No Session"); - } - } + SessionUtil.setAttribute(request, NAME, activity, false); }); } } diff --git a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java index 9a42737e7ce..f9278c08e34 100644 --- a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java +++ b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java @@ -17,7 +17,11 @@ package stroom.resource.impl; import stroom.resource.api.ResourceStore; +import stroom.util.logging.LambdaLogger; +import stroom.util.logging.LambdaLoggerFactory; +import stroom.util.logging.LogUtil; import stroom.util.servlet.HttpServletRequestHolder; +import stroom.util.servlet.SessionUtil; import stroom.util.shared.IsServlet; import stroom.util.shared.ResourceKey; @@ -39,6 +43,8 @@ */ public class SessionResourceStoreImpl extends HttpServlet implements ResourceStore, IsServlet { + private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(SessionResourceStoreImpl.class); + private static final Set PATH_SPECS = Set.of("/resourcestore/*"); private static final String UUID_ARG = "uuid"; private static final String ZIP_EXTENSION = ".zip"; @@ -94,7 +100,9 @@ private ResourceMap getMap() { } final String name = "SESSION_RESOURCE_STORE"; - final HttpSession session = request.getSession(); + final HttpSession session = SessionUtil.getOrCreateSession(request, newSession -> + LOGGER.debug(() -> LogUtil.message("getMap() - Created session {}", + SessionUtil.getSessionId(newSession)))); final Object object = session.getAttribute(name); if (object == null) { resourceMap = new ResourceMap(); diff --git a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java index 44e4c1ddbe1..ab2975699e6 100644 --- a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java +++ b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UserIdentitySessionUtil.java @@ -4,6 +4,7 @@ import stroom.util.logging.LambdaLogger; import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; +import stroom.util.servlet.SessionUtil; import stroom.util.shared.NullSafe; import jakarta.servlet.http.HttpSession; @@ -24,7 +25,7 @@ private UserIdentitySessionUtil() { */ public static void set(final HttpSession session, final UserIdentity userIdentity) { LOGGER.debug(() -> LogUtil.message("Setting userIdentity of type {} in session {}, userIdentity: {}", - NullSafe.get(userIdentity, UserIdentity::getClass, Class::getSimpleName), + LogUtil.getSimpleClassName(userIdentity), NullSafe.get(session, HttpSession::getId), userIdentity)); session.setAttribute(SESSION_USER_IDENTITY, userIdentity); @@ -34,16 +35,16 @@ public static Optional get(final HttpSession session) { final Optional optUserIdentity = Optional.ofNullable(session) .map(session2 -> (UserIdentity) session2.getAttribute(SESSION_USER_IDENTITY)); + if (LOGGER.isTraceEnabled()) { optUserIdentity.ifPresentOrElse(userIdentity -> { LOGGER.trace(() -> LogUtil.message("Got userIdentity of type {} in session {}, userIdentity: {}", NullSafe.get(userIdentity, UserIdentity::getClass, Class::getSimpleName), - NullSafe.get(session, HttpSession::getId), + SessionUtil.getSessionId(session), userIdentity)); - }, () -> { - LOGGER.trace(() -> LogUtil.message("No userIdentity in session {}", - NullSafe.get(session, HttpSession::getId))); - }); + }, () -> + LOGGER.trace(() -> + LogUtil.message("No userIdentity in session {}", SessionUtil.getSessionId(session)))); } return optUserIdentity; } diff --git a/stroom-security/stroom-security-identity/src/main/java/stroom/security/identity/authenticate/AuthenticationServiceImpl.java b/stroom-security/stroom-security-identity/src/main/java/stroom/security/identity/authenticate/AuthenticationServiceImpl.java index b97524f5424..07344ee2cd7 100644 --- a/stroom-security/stroom-security-identity/src/main/java/stroom/security/identity/authenticate/AuthenticationServiceImpl.java +++ b/stroom-security/stroom-security-identity/src/main/java/stroom/security/identity/authenticate/AuthenticationServiceImpl.java @@ -39,6 +39,7 @@ import stroom.util.logging.LambdaLogger; import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; +import stroom.util.servlet.SessionUtil; import event.logging.AuthenticateOutcomeReason; import jakarta.inject.Inject; @@ -59,7 +60,7 @@ class AuthenticationServiceImpl implements AuthenticationService { private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(AuthenticationServiceImpl.class); private static final long MIN_CREDENTIAL_CONFIRMATION_INTERVAL = 600000; - private static final String AUTH_STATE = "AUTH_STATE"; + private static final String AUTH_STATE_ATTRIBUTE_NAME = "AUTH_STATE"; private final UriFactory uriFactory; private final IdentityConfig config; @@ -93,23 +94,19 @@ public AuthenticationServiceImpl( this.certificateExtractor = certificateExtractor; } - private void setAuthState(final HttpSession session, final AuthStateImpl authStateImpl) { - if (session != null) { - session.setAttribute(AUTH_STATE, authStateImpl); - } + private void setAuthState(final HttpServletRequest request, + final AuthStateImpl authStateImpl, + final boolean createSession) { + SessionUtil.setAttribute(request, AUTH_STATE_ATTRIBUTE_NAME, authStateImpl, createSession); + } private AuthStateImpl getAuthState(final HttpServletRequest request) { - if (request == null) { - return null; - } - - final HttpSession session = request.getSession(false); - if (session == null) { - return null; - } - - return (AuthStateImpl) session.getAttribute(AUTH_STATE); + return SessionUtil.getAttribute( + request, + AUTH_STATE_ATTRIBUTE_NAME, + AuthStateImpl.class, + false); } @Override @@ -183,7 +180,7 @@ private AuthStatus loginWithCertificate(final HttpServletRequest request) { account, false, System.currentTimeMillis()); - setAuthState(request.getSession(true), newState); + setAuthState(request, newState, true); // Reset last access, login failures, etc... accountDao.recordSuccessfulLogin(userId); @@ -253,10 +250,9 @@ public ConfirmPasswordResponse confirmPassword(final HttpServletRequest request, final String message = result.toString(); if (result.isAllOk()) { // Update tha last credential confirmation time. - setAuthState(request.getSession(true), - new AuthStateImpl(authState.getAccount(), - authState.isRequirePasswordChange(), - System.currentTimeMillis())); + + final AuthStateImpl newAuthState = authState.withLastCredentialCheckMs(System.currentTimeMillis()); + setAuthState(request, newAuthState, true); return new ConfirmPasswordResponse(true, message); } @@ -302,8 +298,11 @@ public ChangePasswordResponse changePassword(final HttpServletRequest request, accountDao.changePassword(userId, changePasswordRequest.getNewPassword()); if (authState.getSubject().equals(userId)) { - setAuthState(request.getSession(true), - new AuthStateImpl(authState.getAccount(), false, System.currentTimeMillis())); + final AuthStateImpl newAuthState = new AuthStateImpl( + authState.getAccount(), + false, + System.currentTimeMillis()); + setAuthState(request, newAuthState, true); } return new ChangePasswordResponse(true, null, false); @@ -344,8 +343,11 @@ private LoginResponse processSuccessfulLogin(final HttpServletRequest request, // Reset last access, login failures, etc... accountDao.recordSuccessfulLogin(userId); - setAuthState(request.getSession(true), - new AuthStateImpl(account, userNeedsToChangePassword, System.currentTimeMillis())); + final AuthStateImpl newAuthState = new AuthStateImpl( + account, + userNeedsToChangePassword, + System.currentTimeMillis()); + setAuthState(request, newAuthState, true); return new LoginResponse(true, message, userNeedsToChangePassword); } @@ -392,7 +394,7 @@ private Optional getIdFromCertificate(final String cn) { } private void clearSession(final HttpServletRequest request) { - setAuthState(request.getSession(false), null); + setAuthState(request, null, false); } public PasswordPolicyConfig getPasswordPolicy() { @@ -466,8 +468,16 @@ public boolean isRequirePasswordChange() { public long getLastCredentialCheckMs() { return lastCredentialCheckMs; } + + public AuthStateImpl withLastCredentialCheckMs(final long lastCredentialCheckMs) { + return new AuthStateImpl(account, requirePasswordChange, lastCredentialCheckMs); + } } + + // -------------------------------------------------------------------------------- + + private static class AuthStatusImpl implements AuthStatus { private final AuthState state; diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java index ad1fd017274..236333fa6de 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java @@ -125,7 +125,6 @@ private RedirectUrl backChannelOIDC(final HttpServletRequest request, if (optionalUserIdentity.isPresent()) { // Set the token in the session so that when we re-direct to the initiating page (i.e. '/') // we will have the identity in session so won't go back round the code flow loop -// UserIdentitySessionUtil.set(request.getSession(true), optionalUserIdentity.get()); // Successful login, so redirect to the original URL held in the state. final String uri = state.getInitiatingUri(); 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 7c91dc240f0..097b5de451b 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 @@ -143,9 +143,7 @@ private void filter(final HttpServletRequest request, chain.doFilter(request, response); } else { LOGGER.debug(() -> LogUtil.message("Session ID {}, request URI {}", - Optional.ofNullable(request.getSession(false)) - .map(HttpSession::getId) - .orElse("-"), + SessionUtil.getSessionId(request), request.getRequestURI() + Optional.ofNullable(request.getQueryString()) .map(str -> "/" + str) .orElse(""))); @@ -174,7 +172,7 @@ private void filter(final HttpServletRequest request, // If no user from header token, see if we have one in session already. if (optUserIdentity.isEmpty()) { - optUserIdentity = UserIdentitySessionUtil.get(request.getSession(false)); + optUserIdentity = UserIdentitySessionUtil.get(SessionUtil.getExistingSession(request)); if (LOGGER.isDebugEnabled()) { logUserIdentityToDebug(optUserIdentity, fullPath, "from session"); } @@ -363,7 +361,10 @@ public void destroy() { private Optional ensureSessionIfCookiePresent(final HttpServletRequest request) { if (SessionUtil.requestHasSessionCookie(request)) { - return Optional.of(request.getSession(true)); + final HttpSession session = SessionUtil.getOrCreateSession(request, newSession -> + LOGGER.debug(() -> LogUtil.message("ensureSessionIfCookiePresent() - Created new session {}", + SessionUtil.getSessionId(newSession)))); + return Optional.of(session); } return Optional.empty(); } diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java index 6ad7cdd87a8..ea469eff178 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java @@ -36,6 +36,7 @@ import stroom.util.logging.LambdaLogger; import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; +import stroom.util.servlet.SessionUtil; import stroom.util.shared.Clearable; import stroom.util.shared.NullSafe; import stroom.util.shared.PermissionException; @@ -322,7 +323,10 @@ private UserIdentity createAuthFlowUserIdentity(final JwtClaims jwtClaims, // At this point we have authenticated the User so need to ensure // we have a session as that gets attached to the userIdentity object to // allow us to refresh it's token. - final HttpSession session = request.getSession(true); + final HttpSession session = SessionUtil.getOrCreateSession(request, newSession -> { + LOGGER.debug(() -> LogUtil.message("createAuthFlowUserIdentity() - Created new session {}", + newSession.getId())); + }); // Make a token object that we can update as/when we do a token refresh final UpdatableToken updatableToken = new UpdatableToken( @@ -348,11 +352,11 @@ private UserIdentity createAuthFlowUserIdentity(final JwtClaims jwtClaims, LOGGER.debug(() -> LogUtil.message( "createAuthFlowUserIdentity() - Authenticated user - session: {}, userIdentity: {}", - NullSafe.get(session, HttpSession::getId), + SessionUtil.getSessionId(session), userIdentity)); LOGGER.info(() -> "createAuthFlowUserIdentity() - Authenticated user " + userIdentity - + " for sessionId " + NullSafe.get(session, HttpSession::getId)); + + " for sessionId " + SessionUtil.getSessionId(session)); return userIdentity; } @@ -403,15 +407,11 @@ private static ApiUserIdentity createApiUserIdentity(final JwtContext jwtContext final String displayName, final String userUuid, final HttpServletRequest request) { - Objects.requireNonNull(userId); - - final HttpSession session = request.getSession(false); - return new ApiUserIdentity( userUuid, - userId, + Objects.requireNonNull(userId), displayName, - NullSafe.get(session, HttpSession::getId), + SessionUtil.getSessionId(request), jwtContext); } @@ -431,6 +431,7 @@ private Optional getProcessingUser(final JwtContext jwtContext) { public boolean isServiceUser(final String subject, final String issuer, final UserIdentity serviceUser) { + if (serviceUser instanceof final HasJwtClaims hasJwtClaims) { return Optional.ofNullable(hasJwtClaims.getJwtClaims()) .map(ThrowingFunction.unchecked(jwtClaims -> { diff --git a/stroom-util/src/main/java/stroom/util/logging/LogUtil.java b/stroom-util/src/main/java/stroom/util/logging/LogUtil.java index 433ef8018c3..ab7dc0ac7d8 100644 --- a/stroom-util/src/main/java/stroom/util/logging/LogUtil.java +++ b/stroom-util/src/main/java/stroom/util/logging/LogUtil.java @@ -492,4 +492,12 @@ public static DurationTimer startTimerIfTraceEnabled(final Logger logger) { ? DurationTimer.start() : null; } + + public static String getSimpleClassName(final Object obj) { + return NullSafe.get(obj, Object::getClass, Class::getSimpleName); + } + + public static String getClassName(final Object obj) { + return NullSafe.get(obj, Object::getClass, Class::getName); + } } diff --git a/stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java b/stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java index 33d728db670..c083843457b 100644 --- a/stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java +++ b/stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java @@ -2,11 +2,14 @@ import stroom.util.logging.LambdaLogger; import stroom.util.logging.LambdaLoggerFactory; +import stroom.util.logging.LogUtil; import stroom.util.shared.NullSafe; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpSession; +import java.util.function.Consumer; + public class SessionUtil { private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(SessionUtil.class); @@ -30,4 +33,109 @@ public static boolean requestHasSessionCookie(final HttpServletRequest request) return hasCookie; } + + /** + * Gets the existing {@link HttpSession} or returns null if no session is present. + * Does NOT create a session. + */ + public static HttpSession getExistingSession(final HttpServletRequest request) { + if (request != null) { + final HttpSession session = request.getSession(false); + LOGGER.trace(() -> LogUtil.message("getExistingSession() - session: {}", + getSessionId(request))); + return session; + } else { + return null; + } + } + + /** + * Gets the existing {@link HttpSession} or creates one if there is no existing one. + * + * @param onSessionCreate Called if a session didn't previously exist. The session passed to it + * will be non-null. + */ + public static HttpSession getOrCreateSession(final HttpServletRequest request, + final Consumer onSessionCreate) { + if (request != null) { + if (onSessionCreate != null) { + final HttpSession existingSession = request.getSession(false); + final HttpSession session = request.getSession(true); + + if (existingSession == null && session != null) { + onSessionCreate.accept(session); + } + return session; + } else { + return request.getSession(true); + } + } else { + return null; + } + } + + /** + * Null safe method to get the session ID if there is a session. Does not create a session. + * + * @return The session ID or null if there is no session. + */ + public static String getSessionId(final HttpServletRequest request) { + return NullSafe.get( + request, + req -> req.getSession(false), + HttpSession::getId); + } + + /** + * Null safe method to get the session ID if there is a session. Does not create a session. + * + * @return The session ID or null if there is no session. + */ + public static String getSessionId(final HttpSession session) { + return NullSafe.get(session, HttpSession::getId); + } + + /** + * Gets an attribute from the session if the session and attribute exists. + * If createSession is true, a session will be created if it does not exist. + */ + public static T getAttribute(final HttpServletRequest request, + final String name, + final Class type, + final boolean createSession) { + if (request != null) { + final HttpSession session = request.getSession(createSession); + if (session != null) { + final Object object = session.getAttribute(name); + LOGGER.debug("getAttribute() - object: {}", object); + if (object != null) { + return type.cast(object); + } else { + return null; + } + } else { + LOGGER.debug("No Session"); + return null; + } + } else { + return null; + } + } + + /** + * Sets an attribute in the session if the session exists or createSession is true. + */ + public static void setAttribute(final HttpServletRequest request, + final String name, + final Object value, + final boolean createSession) { + if (request != null) { + final HttpSession session = request.getSession(createSession); + if (session != null) { + session.setAttribute(name, value); + } else { + LOGGER.debug("No Session"); + } + } + } } From 9dab64b06da0a6a7cc8c386c84dc107fe5ba89ca Mon Sep 17 00:00:00 2001 From: at055612 <22818309+at055612@users.noreply.github.com> Date: Wed, 10 Sep 2025 09:28:13 +0100 Subject: [PATCH 04/10] Fix accidental session creation, improve logging --- .../core/servlet/SessionIdProviderImpl.java | 12 +--- .../explorer/impl/ExplorerSessionImpl.java | 12 ++-- .../impl/SessionResourceStoreImpl.java | 2 +- .../security/common/impl/UpdatableToken.java | 2 + .../stroom/security/impl/OpenIdManager.java | 12 +++- .../stroom/security/impl/SecurityFilter.java | 47 +++++++-------- .../security/impl/SessionListListener.java | 10 ++-- .../impl/StroomUserIdentityFactory.java | 16 +++-- .../servlet/HttpServletRequestHolder.java | 11 +--- .../java/stroom/util/servlet/SessionUtil.java | 59 +++++++++++++------ .../util/servlet/UserAgentSessionUtil.java | 39 +++++++----- 11 files changed, 130 insertions(+), 92 deletions(-) diff --git a/stroom-core/src/main/java/stroom/core/servlet/SessionIdProviderImpl.java b/stroom-core/src/main/java/stroom/core/servlet/SessionIdProviderImpl.java index 19a2cf9bb42..db641eee2ed 100644 --- a/stroom-core/src/main/java/stroom/core/servlet/SessionIdProviderImpl.java +++ b/stroom-core/src/main/java/stroom/core/servlet/SessionIdProviderImpl.java @@ -17,11 +17,11 @@ package stroom.core.servlet; import stroom.util.servlet.SessionIdProvider; +import stroom.util.servlet.SessionUtil; import jakarta.inject.Inject; import jakarta.inject.Provider; import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpSession; class SessionIdProviderImpl implements SessionIdProvider { @@ -37,14 +37,6 @@ class SessionIdProviderImpl implements SessionIdProvider { */ @Override public String get() { - final HttpServletRequest request = httpServletRequestProvider.get(); - if (request != null) { - final HttpSession httpSession = request.getSession(false); - if (httpSession != null) { - return httpSession.getId(); - } - } - - return null; + return SessionUtil.getSessionId(httpServletRequestProvider.get()); } } diff --git a/stroom-explorer/stroom-explorer-impl/src/main/java/stroom/explorer/impl/ExplorerSessionImpl.java b/stroom-explorer/stroom-explorer-impl/src/main/java/stroom/explorer/impl/ExplorerSessionImpl.java index ba85d8583e7..bc3e8992db4 100644 --- a/stroom-explorer/stroom-explorer-impl/src/main/java/stroom/explorer/impl/ExplorerSessionImpl.java +++ b/stroom-explorer/stroom-explorer-impl/src/main/java/stroom/explorer/impl/ExplorerSessionImpl.java @@ -3,6 +3,7 @@ import stroom.util.logging.LambdaLogger; import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; +import stroom.util.servlet.SessionUtil; import jakarta.inject.Inject; import jakarta.inject.Provider; @@ -28,11 +29,9 @@ public Optional getMinExplorerTreeModelId() { try { return getSession().map(session -> { final Long minModelId = (Long) session.getAttribute(MIN_EXPLORER_TREE_MODEL_ID); - LOGGER.debug(() -> LogUtil.message("getMinExplorerTreeModelId - sessionId: {}, minModelId: {}", session.getId(), minModelId)); - return minModelId; }); } catch (final RuntimeException e) { @@ -65,11 +64,10 @@ private Optional getSession() { } else { // Need to create the session if there isn't one as the explorer tree model update process // relies on having a session available, which there may not be if auth is disabled. - final Optional optSession = Optional.ofNullable(request.getSession(true)); - - LOGGER.debug(() -> "session id: " + optSession.map(HttpSession::getId).orElse("null")); - - return optSession; + final HttpSession session = SessionUtil.getOrCreateSession(request, newSession -> { + LOGGER.info("getSession() - Created session {}", newSession.getId()); + }); + return Optional.of(session); } } } catch (final RuntimeException e) { diff --git a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java index f9278c08e34..0f1aadeb21d 100644 --- a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java +++ b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java @@ -101,7 +101,7 @@ private ResourceMap getMap() { final String name = "SESSION_RESOURCE_STORE"; final HttpSession session = SessionUtil.getOrCreateSession(request, newSession -> - LOGGER.debug(() -> LogUtil.message("getMap() - Created session {}", + LOGGER.info(() -> LogUtil.message("getMap() - Created session {}", SessionUtil.getSessionId(newSession)))); final Object object = session.getAttribute(name); if (object == null) { diff --git a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UpdatableToken.java b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UpdatableToken.java index 5fb1fb96caf..06180c22ea9 100644 --- a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UpdatableToken.java +++ b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UpdatableToken.java @@ -8,6 +8,7 @@ import stroom.util.logging.LambdaLogger; import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; +import stroom.util.servlet.SessionUtil; import stroom.util.shared.NullSafe; import jakarta.servlet.http.HttpSession; @@ -189,6 +190,7 @@ public String toString() { JwtUtil.getClaimValue(claims, OpenId.CLAIM__PREFERRED_USERNAME).orElse(null)) + ", expireTimeWithBuffer=" + Instant.ofEpochMilli(mutableState.expireTimeWithBufferEpochMs) + ", timeTilExpire=" + Duration.between(Instant.now(), getExpireTime()) + + ", session=" + SessionUtil.getSessionId(session) + '}'; } diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java index 236333fa6de..8ff6435f89c 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java @@ -9,6 +9,7 @@ import stroom.util.logging.LambdaLogger; import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; +import stroom.util.servlet.SessionUtil; import stroom.util.shared.NullSafe; import stroom.util.shared.ResourcePaths; @@ -146,8 +147,17 @@ private RedirectUrl backChannelOIDC(final HttpServletRequest request, * This method attempts to get a token from the request headers and, if present, use that to login. */ public Optional loginWithRequestToken(final HttpServletRequest request) { + LOGGER.debug(() -> LogUtil.message("loginWithRequestToken() - session: {}", + SessionUtil.getSessionId(request))); if (userIdentityFactory.hasAuthenticationToken(request)) { - return userIdentityFactory.getApiUserIdentity(request); + final Optional optApiUserIdentity = userIdentityFactory.getApiUserIdentity(request); + if (LOGGER.isDebugEnabled()) { + final UserIdentity userIdentity = optApiUserIdentity.get(); + LOGGER.debug("loginWithRequestToken() - Returning {} {}", + LogUtil.getSimpleClassName(userIdentity), + userIdentity); + } + return optApiUserIdentity; } else { LOGGER.trace("No token on request. This is valid for API calls from the front-end"); return Optional.empty(); 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 097b5de451b..3248bbca244 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 @@ -120,10 +120,13 @@ private void filter(final HttpServletRequest request, throws IOException, ServletException { LOGGER.debug(() -> - LogUtil.message("Filtering request uri: {}, servletPath: {}, servletName: {}", + LogUtil.message("Filtering request uri: {}, servletPath: {}, servletName: {}, " + + "session: {}, getQueryString: '{}'", request.getRequestURI(), request.getServletPath(), - NullSafe.get(request.getHttpServletMapping(), HttpServletMapping::getServletName))); + NullSafe.get(request.getHttpServletMapping(), HttpServletMapping::getServletName), + SessionUtil.getSessionId(request), + request.getQueryString())); // Log the request for debug purposes. RequestLog.log(request); @@ -142,27 +145,19 @@ private void filter(final HttpServletRequest request, servletName, fullPath, servletPath); chain.doFilter(request, response); } else { - LOGGER.debug(() -> LogUtil.message("Session ID {}, request URI {}", - SessionUtil.getSessionId(request), - request.getRequestURI() + Optional.ofNullable(request.getQueryString()) - .map(str -> "/" + str) - .orElse(""))); - - Optional optUserIdentity; - // Api requests that are not from the front-end should have a token. // Also request from an AWS ALB will have an ALB signed token containing the claims // Need to do this first, so we get a fresh token from AWS ALB rather than using a stale // one from session. - optUserIdentity = openIdManager.loginWithRequestToken(request); - optUserIdentity.ifPresent(userIdentity -> - ensureSessionIfCookiePresent(request).ifPresent(session -> { - LOGGER.debug(() -> LogUtil.message("Setting user in session, user: {} {}, path: {}", - userIdentity.getClass().getSimpleName(), - userIdentity, - fullPath)); - UserIdentitySessionUtil.set(session, userIdentity); - })); + Optional optUserIdentity = openIdManager.loginWithRequestToken(request); +// optUserIdentity.ifPresent(userIdentity -> +// ensureSessionIfCookiePresent(request).ifPresent(session -> { +// LOGGER.debug(() -> LogUtil.message("Setting user in session, user: {} {}, path: {}", +// userIdentity.getClass().getSimpleName(), +// userIdentity, +// fullPath)); +// UserIdentitySessionUtil.set(session, userIdentity); +// })); // Log current user. if (LOGGER.isDebugEnabled()) { @@ -182,8 +177,11 @@ private void filter(final HttpServletRequest request, final UserIdentity userIdentity = optUserIdentity.get(); // Now we have the session make note of the user-agent for logging and sessionListServlet duties - ensureSessionIfCookiePresent(request).ifPresent(session -> - UserAgentSessionUtil.setUserAgentInSession(request, session)); + UserAgentSessionUtil.setUserAgentInSession(request); +// SessionUtil.withSession(request, session -> +// UserAgentSessionUtil.setUserAgentInSession(request, session)); +// ensureSessionIfCookiePresent(request).ifPresent(session -> +// UserAgentSessionUtil.setUserAgentInSession(request, session)); // Now handle the request as this user securityContext.asUser(userIdentity, () -> @@ -220,6 +218,8 @@ private void doOpenIdFlow(final HttpServletRequest request, final String stateId = UrlUtils.getLastParam(request, OpenId.STATE); final RedirectUrl redirectUri = openIdManager.redirect( request, code, stateId, postAuthRedirectUri); + LOGGER.debug("Doing code flow postAuthRedirectUri: {}, code: {}, stateId: {}, redirectUri: {}", + postAuthRedirectUri, code, stateId, redirectUri); // HTTP 1.1. response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); // HTTP 1.0. @@ -362,8 +362,9 @@ public void destroy() { private Optional ensureSessionIfCookiePresent(final HttpServletRequest request) { if (SessionUtil.requestHasSessionCookie(request)) { final HttpSession session = SessionUtil.getOrCreateSession(request, newSession -> - LOGGER.debug(() -> LogUtil.message("ensureSessionIfCookiePresent() - Created new session {}", - SessionUtil.getSessionId(newSession)))); + LOGGER.debug(() -> LogUtil.message( + "ensureSessionIfCookiePresent() - Created new session {}, request URL", + SessionUtil.getSessionId(newSession), request.getRequestURI()))); return Optional.of(session); } return Optional.empty(); 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 64bc6c5dbdb..90b177a035e 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 @@ -76,12 +76,12 @@ class SessionListListener implements HttpSessionListener, HttpSessionIdListener, this.webTargetFactory = webTargetFactory; } - @Override public void sessionCreated(final HttpSessionEvent event) { final HttpSession httpSession = event.getSession(); - LOGGER.info("sessionCreated() - {}", httpSession.getId()); - sessionMap.put(httpSession.getId(), httpSession); + final String sessionId = httpSession.getId(); + LOGGER.debug("sessionCreated() - sessionId: {}", sessionId); + sessionMap.put(sessionId, httpSession); } @Override @@ -89,7 +89,7 @@ public void sessionDestroyed(final HttpSessionEvent event) { final HttpSession httpSession = event.getSession(); try { final UserRef userRef = getUserRefFromSession(httpSession); - final String userAgent = UserAgentSessionUtil.get(httpSession); + final String userAgent = UserAgentSessionUtil.getUserAgent(httpSession); final Instant createTime = Instant.ofEpochMilli(httpSession.getCreationTime()); final Instant lastAccessedTime = Instant.ofEpochMilli(httpSession.getLastAccessedTime()); LOGGER.info("sessionDestroyed() - {}, createTime: {} ({}), lastAccessTime: {} ({}), " + @@ -180,7 +180,7 @@ private SessionListResponse listSessionsOnThisNode() { userRef, httpSession.getCreationTime(), httpSession.getLastAccessedTime(), - UserAgentSessionUtil.get(httpSession), + UserAgentSessionUtil.getUserAgent(httpSession), thisNodeName); }) .collect(SessionListResponse.collector(SessionListResponse::new)), diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java index ea469eff178..8d207adca23 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java @@ -37,6 +37,7 @@ import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; import stroom.util.servlet.SessionUtil; +import stroom.util.servlet.UserAgentSessionUtil; import stroom.util.shared.Clearable; import stroom.util.shared.NullSafe; import stroom.util.shared.PermissionException; @@ -320,12 +321,16 @@ private UserIdentity createAuthFlowUserIdentity(final JwtClaims jwtClaims, final TokenResponse tokenResponse, final User user) { Objects.requireNonNull(user); - // At this point we have authenticated the User so need to ensure - // we have a session as that gets attached to the userIdentity object to - // allow us to refresh it's token. + // At this point we have authenticated the code-flow User so need to ensure + // we have a session to store the userIdentity in. Also, the session gets attached + // to the userIdentity object to allow us to refresh it's token. final HttpSession session = SessionUtil.getOrCreateSession(request, newSession -> { - LOGGER.debug(() -> LogUtil.message("createAuthFlowUserIdentity() - Created new session {}", - newSession.getId())); + UserAgentSessionUtil.setUserAgentInSession(request, newSession); + LOGGER.info(() -> LogUtil.message( + "createAuthFlowUserIdentity() - Created new session, sessionId: {}, userRef: {}, user-agent: {}", + newSession.getId(), + user.getUserRef(), + UserAgentSessionUtil.getUserAgent(newSession))); }); // Make a token object that we can update as/when we do a token refresh @@ -420,6 +425,7 @@ private Optional getProcessingUser(final JwtContext jwtContext) { final JwtClaims jwtClaims = jwtContext.getJwtClaims(); final UserIdentity serviceUser = getServiceUserIdentity(); if (isServiceUser(jwtClaims.getSubject(), jwtClaims.getIssuer(), serviceUser)) { + LOGGER.debug("getProcessingUser() - {}", serviceUser); return Optional.of(serviceUser); } } catch (final MalformedClaimException e) { diff --git a/stroom-util/src/main/java/stroom/util/servlet/HttpServletRequestHolder.java b/stroom-util/src/main/java/stroom/util/servlet/HttpServletRequestHolder.java index 03a24e5063e..ab89557206c 100644 --- a/stroom-util/src/main/java/stroom/util/servlet/HttpServletRequestHolder.java +++ b/stroom-util/src/main/java/stroom/util/servlet/HttpServletRequestHolder.java @@ -19,12 +19,10 @@ import stroom.util.logging.LambdaLogger; import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; -import stroom.util.shared.NullSafe; import jakarta.inject.Provider; import jakarta.inject.Singleton; import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpSession; @Singleton public class HttpServletRequestHolder implements Provider { @@ -44,15 +42,12 @@ public void set(final HttpServletRequest httpServletRequest) { if (httpServletRequest == null) { LOGGER.debug(() -> LogUtil.message("Clearing held request against thread {}", - Thread.currentThread().getId())); + Thread.currentThread().threadId())); } else { LOGGER.debug(() -> LogUtil.message("Holding request with session id {} against thread {}", - NullSafe.get( - httpServletRequest, - request -> request.getSession(false), - HttpSession::getId), - Thread.currentThread().getId())); + SessionUtil.getSessionId(httpServletRequest), + Thread.currentThread().threadId())); } } diff --git a/stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java b/stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java index c083843457b..08bb6452712 100644 --- a/stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java +++ b/stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java @@ -28,7 +28,7 @@ public static boolean requestHasSessionCookie(final HttpServletRequest request) cookie.getName().equalsIgnoreCase(JSESSIONID)); LOGGER.debug("requestHasSessionCookie() - sessionId: {}, hasCookie: {}", - NullSafe.get(request, HttpServletRequest::getSession, HttpSession::getId), + SessionUtil.getSessionId(request), hasCookie); return hasCookie; @@ -58,17 +58,22 @@ public static HttpSession getExistingSession(final HttpServletRequest request) { public static HttpSession getOrCreateSession(final HttpServletRequest request, final Consumer onSessionCreate) { if (request != null) { - if (onSessionCreate != null) { - final HttpSession existingSession = request.getSession(false); - final HttpSession session = request.getSession(true); - - if (existingSession == null && session != null) { + HttpSession session = request.getSession(false); + if (session == null) { + session = request.getSession(true); + if (onSessionCreate != null) { + // It is possible that two threads could do this but that seems unlikely + // for a human to do that. onSessionCreate.accept(session); } - return session; + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("getOrCreateSession() - Created session: {}, user-agent: {}", + session.getId(), UserAgentSessionUtil.getUserAgent(session)); + } } else { - return request.getSession(true); + LOGGER.debug("getOrCreateSession() - Existing session: {}", session.getId()); } + return session; } else { return null; } @@ -95,6 +100,16 @@ public static String getSessionId(final HttpSession session) { return NullSafe.get(session, HttpSession::getId); } + /** + * Passes the {@link HttpSession} to sessionConsumer IF there is a session. + * Does NOT create a new session. + */ + public static void withSession(final HttpServletRequest request, + final Consumer sessionConsumer) { + final HttpSession session = SessionUtil.getExistingSession(request); + NullSafe.consume(session, sessionConsumer); + } + /** * Gets an attribute from the session if the session and attribute exists. * If createSession is true, a session will be created if it does not exist. @@ -105,19 +120,29 @@ public static T getAttribute(final HttpServletRequest request, final boolean createSession) { if (request != null) { final HttpSession session = request.getSession(createSession); - if (session != null) { - final Object object = session.getAttribute(name); - LOGGER.debug("getAttribute() - object: {}", object); - if (object != null) { - return type.cast(object); - } else { - return null; - } + return getAttribute(session, name, type); + } else { + return null; + } + } + + /** + * Gets an attribute from the session if the session and attribute exists. + * If createSession is true, a session will be created if it does not exist. + */ + public static T getAttribute(final HttpSession session, + final String name, + final Class type) { + if (session != null) { + final Object object = session.getAttribute(name); + LOGGER.debug("getAttribute() - object: {}", object); + if (object != null) { + return type.cast(object); } else { - LOGGER.debug("No Session"); return null; } } else { + LOGGER.debug("No Session"); return null; } } 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 2a9e00747ab..f69c397aa27 100644 --- a/stroom-util/src/main/java/stroom/util/servlet/UserAgentSessionUtil.java +++ b/stroom-util/src/main/java/stroom/util/servlet/UserAgentSessionUtil.java @@ -12,38 +12,47 @@ public final class UserAgentSessionUtil { private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(UserAgentSessionUtil.class); - private static final String USER_AGENT = "user-agent"; + private static final String USER_AGENT_HEADER_KEY = "user-agent"; private UserAgentSessionUtil() { // Utility class. } +// public static void setUserAgentInSession(final HttpServletRequest request) { +// final HttpSession session = request.getSession(false); +// setUserAgentInSession(request, session); +// } + + /** + * If there is a session, it sets the value of the {@code user-agent} header (if present) + * into the session attribute. + */ public static void setUserAgentInSession(final HttpServletRequest request) { - final HttpSession session = request.getSession(false); - setUserAgentInSession(request, session); + SessionUtil.withSession(request, session -> + setUserAgentInSession(request, session)); } + /** + * Sets value of the {@code user-agent} header into a session attribute, IF there is + * a session. + */ public static void setUserAgentInSession(final HttpServletRequest request, final HttpSession session) { if (session != null) { - final String userAgent = request.getHeader(USER_AGENT); + final String userAgent = request.getHeader(USER_AGENT_HEADER_KEY); if (userAgent != null) { - LOGGER.debug(() -> LogUtil.message("Setting {} '{}' in session {}", - USER_AGENT, + LOGGER.debug(() -> LogUtil.message("setUserAgentInSession() - Setting {} '{}' in session {}", + USER_AGENT_HEADER_KEY, userAgent, NullSafe.get(session, HttpSession::getId))); - session.setAttribute(USER_AGENT, userAgent); + session.setAttribute(USER_AGENT_HEADER_KEY, userAgent); } + } else { + LOGGER.debug("setUserAgentInSession() - No session"); } } - public static String get(final HttpSession session) { - if (session != null) { - final Object userAgent = session.getAttribute(USER_AGENT); - if (userAgent != null) { - return userAgent.toString(); - } - } - return null; + public static String getUserAgent(final HttpSession session) { + return SessionUtil.getAttribute(session, USER_AGENT_HEADER_KEY, String.class); } } From 1a1488961df439526781c452df8fa6d3e4776408 Mon Sep 17 00:00:00 2001 From: at055612 <22818309+at055612@users.noreply.github.com> Date: Fri, 12 Sep 2025 18:07:01 +0100 Subject: [PATCH 05/10] Change resource store to not use sessions gh-5114 Improve handling of loss of connection to IDP Improve SecurityFilter logic to determine whether to auth or not --- scripts/export_content.sh | 190 ++++++++ scripts/import_content.sh | 117 +++++ stroom-app/src/main/java/stroom/app/App.java | 4 +- .../main/java/stroom/app/guice/AppModule.java | 4 +- .../client/ExportFileCompleteUtil.java | 2 + .../client/api/event/LogoutEvent.java | 4 + .../dashboard/shared/DashboardResource.java | 6 +- .../stroom/core/servlet/SignInServlet.java | 5 + .../stroom/core/servlet/StroomServlet.java | 6 + .../dashboard/impl/DashboardResourceImpl.java | 2 +- .../AuthenticationBypassCheckerImpl.java | 3 +- .../stroom/dropwizard/common/Servlets.java | 81 ++-- .../impl/ContentResourceImpl.java | 9 +- .../job/impl/ScheduledTaskExecutor.java | 3 +- .../stroom/resource/api/ResourceStore.java | 4 + .../stroom-resource-impl/build.gradle | 5 + .../stroom/resource/impl/ResourceModule.java | 30 +- .../resource/impl/ResourceStoreImpl.java | 191 -------- .../resource/impl/SessionResourceModule.java | 35 -- .../impl/SessionResourceStoreImpl.java | 164 ------- .../resource/impl/UserResourceStoreImpl.java | 414 ++++++++++++++++++ .../resource/impl/TestResourceStore.java | 89 ---- .../impl/TestUserResourceStoreImpl.java | 414 ++++++++++++++++++ .../impl/AbstractUserIdentityFactory.java | 50 ++- .../security/common/impl/RefreshManager.java | 188 +++++--- .../security/common/impl/UpdatableToken.java | 12 +- .../stroom/security/impl/OpenIdManager.java | 3 +- .../security/impl/SecurityContextImpl.java | 16 +- .../stroom/security/impl/SecurityFilter.java | 65 +-- .../security/impl/SessionListListener.java | 158 ++++--- .../security/impl/SessionResourceImpl.java | 46 +- .../impl/StroomUserIdentityFactory.java | 97 ++-- .../impl/TestSessionListListener.java | 9 +- .../impl/TestSessionResourceImpl.java | 4 +- .../java/stroom/test/common/TestUtil.java | 3 +- .../java/stroom/util/shared/IsServlet.java | 8 + .../java/stroom/util/shared/ResourceKey.java | 9 + .../stroom/util/shared/ResourcePaths.java | 4 + unreleased_changes/20250912_165642_710__0.md | 55 +++ .../20250912_165749_707__5114.md | 60 +++ unreleased_changes/20250912_165825_362__0.md | 55 +++ 41 files changed, 1832 insertions(+), 792 deletions(-) create mode 100755 scripts/export_content.sh create mode 100755 scripts/import_content.sh delete mode 100644 stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/ResourceStoreImpl.java delete mode 100644 stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceModule.java delete mode 100644 stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java create mode 100644 stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/UserResourceStoreImpl.java delete mode 100644 stroom-resource/stroom-resource-impl/src/test/java/stroom/resource/impl/TestResourceStore.java create mode 100644 stroom-resource/stroom-resource-impl/src/test/java/stroom/resource/impl/TestUserResourceStoreImpl.java create mode 100644 unreleased_changes/20250912_165642_710__0.md create mode 100644 unreleased_changes/20250912_165749_707__5114.md create mode 100644 unreleased_changes/20250912_165825_362__0.md diff --git a/scripts/export_content.sh b/scripts/export_content.sh new file mode 100755 index 00000000000..e8d2f1440ff --- /dev/null +++ b/scripts/export_content.sh @@ -0,0 +1,190 @@ +#!/usr/bin/env bash + +set -e -o pipefail + +showUsage() { + echo -e "Usage: ${BLUE}$0 [OPTION]... [UUID]...${NC}" + echo -e "OPTIONs:" + echo -e " ${GREEN}-d${NC} - The data file to read from (or '-' for stdin)" + echo -e " ${GREEN}-u${NC} - The base URL to use. If not set, uses http://localhost:8080" + echo -e " ${GREEN}-o${NC} - The file path to export to. If not set it will be written to the current directory with the resource name" + echo -e "UUID:" + echo -e " Each UUID is of the form ',', e.g. '18d92d74-a5aa-4a81-83be-46fb6d84a60e,Pipeline'" + echo -e "e.g.: ${BLUE}$0 -d - ${NC} - Exports all UUIDs in std into a file in this directory" + echo +} + +setup_echo_colours() { + # Exit the script on any error + set -e + + # shellcheck disable=SC2034 + if [ "${MONOCHROME}" = true ]; then + RED='' + GREEN='' + YELLOW='' + BLUE='' + BLUE2='' + DGREY='' + NC='' # No Colour + else + RED='\033[1;31m' + GREEN='\033[1;32m' + YELLOW='\033[1;33m' + BLUE='\033[1;34m' + BLUE2='\033[1;34m' + DGREY='\e[90m' + NC='\033[0m' # No Colour + fi +} + +debug_value() { + local name="$1"; shift + local value="$1"; shift + + if [ "${IS_DEBUG}" = true ]; then + echo -e "${DGREY}DEBUG ${name}: ${value}${NC}" + fi +} + +debug() { + local str="$1"; shift + + if [ "${IS_DEBUG}" = true ]; then + echo -e "${DGREY}DEBUG ${str}${NC}" + fi +} + +main() { + IS_DEBUG=false + #SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )" + + setup_echo_colours + + local uuids_arr=() + local url_base + local data_filename + local output_filename + + if [[ -z "${TOKEN}" ]]; then + echo -e "${RED}Environment variable TOKEN must be set with an API key or an OAuth token${NC}" >&2 + exit 1 + fi + + while [ ${OPTIND} -le "$#" ]; do + if getopts d:ho:u: option + then + case ${option} in + h) + showUsage + ;; + u) + if [[ -z "${OPTARG}" ]]; then + echo -e "${RED}-u argument requires a base URL to be specified${NC}" >&2 + echo + showUsage + exit 1 + fi + url_base="${OPTARG}" + ;; + d) + if [[ -z "${OPTARG}" ]]; then + echo -e "${RED}-d argument requires a filename or '-' to read from stdin${NC}" >&2 + echo + showUsage + exit 1 + fi + data_filename="${OPTARG}" + ;; + o) + if [[ -z "${OPTARG}" ]]; then + echo -e "${RED}-o argument requires a file path to output to${NC}" >&2 + echo + showUsage + exit 1 + fi + output_filename="${OPTARG}" + ;; + esac + else + uuids_arr+=("${!OPTIND}") + ((OPTIND++)) + fi + done + + # Add all the uuids from file/stdin + if [[ -n "${data_filename}" ]]; then + if [[ "${data_filename}" = "-" ]]; then + data_filename="/dev/stdin" + fi + + while read -r line; do + uuids_arr+=( "${line}" ) + done < "${data_filename}" + fi + + if [[ "${#uuids_arr[@]}" -eq 0 ]]; then + echo "No UUIDs to export" >&2 + exit 1 + fi + + + local uuids_str + uuids_str=$( printf "%s\n" "${uuids_arr[@]}" ) + local req_json + req_json=$( \ + jq -R -s '{ + docRefs: [ + split("\n")[] + | select(length > 0) + | split(",") + | {uuid: .[0], type: .[1]} + ] + }' <<< "${uuids_str}" ) + + #echo -e "${req_json}" + + if [[ -z "${url_base}" ]]; then + url_base="http://localhost:8080" + fi + + local export_url="${url_base}/api/content/v1/export" + local resource_url="${url_base}/resourcestore/" + + local resource_key_json + resource_key_json=$( \ + echo -e "${req_json}" | curl \ + -s \ + --request POST \ + -H "Content-Type:application/json" \ + -H "Authorization:Bearer ${TOKEN}" \ + -d @- \ + "${export_url}" ) + + #echo -e "${resource_key_json}" + + local key + local name + key=$( jq -r '.resourceKey.key' <<< "${resource_key_json}" ) + name=$( jq -r '.resourceKey.name' <<< "${resource_key_json}" ) + if [[ -z "${output_filename}" ]]; then + output_filename="${name}" + fi + + echo -e "${GREEN}Downloading resource ${YELLOW}${key}${GREEN} to file ${BLUE}${output_filename}${NC}" + resource_url="${resource_url}?uuid=${key}" + #echo -e "${resource_url}" + + curl \ + -s \ + --output "${name}" \ + -H "Authorization: Bearer $TOKEN" \ + "${resource_url}" + + + #http POST http://localhost:8080/api/content/v1/export "Authorization:Bearer ${TOKEN}" + #| jq -r '.resourceKey.key') && curl --output content.zip -H "Authorization: Bearer $TOKEN" http://localhost:8080/resourcestore/\?uuid\=$key + +} + +main "$@" diff --git a/scripts/import_content.sh b/scripts/import_content.sh new file mode 100755 index 00000000000..0923e79cd72 --- /dev/null +++ b/scripts/import_content.sh @@ -0,0 +1,117 @@ +#!/usr/bin/env bash + +# Script to import a single Stroom content pack ZIP file into Stroom. +# Requires that the environment variable TOKEN is set with either a +# valid API Key or an OAuth token. +# Usage: +# import_content.sh CONTENT_ZIP_FILE [URL_BASE] +# e.g. import_content.sh /some/path/StroomConfig.zip +# e.g. import_content.sh /some/path/StroomConfig.zip https://stroom.some.domain + +set -e -o pipefail + +showUsage() { + echo -e "Usage: ${BLUE}$0 [OPTION]... FILE${NC}" + echo -e "OPTIONs:" + echo -e " ${GREEN}-d${NC} - The data file to read from (or '-' for stdin)" + echo -e " ${GREEN}-u${NC} - The base URL to use. If not set, uses http://localhost:8080" + echo -e " ${GREEN}-o${NC} - The file path to export to. If not set it will be written to the current directory with the resource name" + echo -e "FILE: The file to import" + echo -e "e.g.: ${BLUE}$0 content.zip${NC}" + echo -e "e.g.: ${BLUE}$0 -u content.zip${NC}" + echo +} + +main() { + if [[ -z "${TOKEN}" ]]; then + echo -e "${RED}Environment variable TOKEN must be set with an API key or an OAuth token${NC}" >&2 + exit 1 + fi + + while getopts hu: option; do + case ${option} in + h) + showUsage + ;; + u) + if [[ -z "${OPTARG}" ]]; then + echo -e "${RED}-u argument requires a base URL to be specified${NC}" >&2 + echo + showUsage + exit 1 + fi + url_base="${OPTARG}" + ;; + esac + done + + # shellcheck disable=SC2124 + local content_zip_file="${@:$OPTIND:1}"; shift + if [[ -z "${content_zip_file}" ]]; then + echo -e "${RED}content_zip_file argument must be provided${NC}" >&2 + exit 1 + fi + + if [[ -z "${url_base}" ]]; then + url_base="http://localhost:8080" + fi + + echo -e "${GREEN}Uploading file '${BLUE}${content_zip_file}${GREEN}' to a temporary file${NC}" + + local response1 + response1="$( \ + curl \ + --silent \ + --header "Authorization: Bearer ${TOKEN}" \ + --form "encoding=multipart/form-data" \ + --form fileUpload="@${content_zip_file}" \ + --request POST \ + "${url_base}/importfile.rpc" \ + )" + + echo -e "${GREEN}File uploaded${NC}" + + echo -e "${response1}" + # response1 looks like + # '#PM#success=true name=StroomConfig.zip key=e015e5d4-8a6a-4454-8d81-913e6c13cca5#PM#' + + local key + key="$( \ + grep \ + --only-matching \ + --perl-regexp \ + '(?<=key=)[^ #]+' \ + <<< "${response1}" + )" + + # name is only really used for logging purposes in stroom, but provide + # it anyway + local name + name="$( \ + grep \ + --only-matching \ + --perl-regexp \ + '(?<=name=)[^ #]+' \ + <<< "${response1}" + )" + + local import_request + import_request="{ \"confirmList\": [], \"importSettings\": { \"importMode\": \"IGNORE_CONFIRMATION\" }, \"resourceKey\": { \"key\": \"${key}\", \"name\": \"${name}\" } }" + + echo "Importing content" + + # Not interested in the response + curl \ + --silent \ + --request POST \ + --header "Authorization:Bearer ${TOKEN}" \ + --header 'Content-Type: application/json' \ + --data "${import_request}" \ + "${url_base}/api/content/v1/import" \ + > /dev/null + + echo "Content imported successfully" + echo "Done!" +} + +main "$@" diff --git a/stroom-app/src/main/java/stroom/app/App.java b/stroom-app/src/main/java/stroom/app/App.java index 0f66f8ab681..efb93a2afc8 100644 --- a/stroom-app/src/main/java/stroom/app/App.java +++ b/stroom-app/src/main/java/stroom/app/App.java @@ -158,10 +158,10 @@ public void initialize(final Bootstrap bootstrap) { // Add the GWT UI assets. bootstrap.addBundle(new DynamicAssetsBundle( - "/ui", + ResourcePaths.UI_PATH, ResourcePaths.UI_PATH, "index.html", - "ui")); + ResourcePaths.UI_SERVLET_NAME)); addCliCommands(bootstrap); diff --git a/stroom-app/src/main/java/stroom/app/guice/AppModule.java b/stroom-app/src/main/java/stroom/app/guice/AppModule.java index e9a56c3cb68..32cfc7dfffa 100644 --- a/stroom-app/src/main/java/stroom/app/guice/AppModule.java +++ b/stroom-app/src/main/java/stroom/app/guice/AppModule.java @@ -6,7 +6,7 @@ import stroom.dropwizard.common.LogLevelInspector; import stroom.lifecycle.impl.LifecycleServiceModule; import stroom.meta.statistics.impl.MetaStatisticsModule; -import stroom.resource.impl.SessionResourceModule; +import stroom.resource.impl.ResourceModule; import stroom.security.impl.SecurityContextModule; import stroom.statistics.impl.sql.search.SQLStatisticSearchModule; import stroom.util.guice.AdminServletBinder; @@ -27,7 +27,7 @@ protected void configure() { install(new SecurityContextModule()); install(new MetaStatisticsModule()); install(new SQLStatisticSearchModule()); - install(new SessionResourceModule()); + install(new ResourceModule()); install(new JerseyModule()); HasSystemInfoBinder.create(binder()) diff --git a/stroom-core-client/src/main/java/stroom/dispatch/client/ExportFileCompleteUtil.java b/stroom-core-client/src/main/java/stroom/dispatch/client/ExportFileCompleteUtil.java index 10550e0f042..1e7bde4a499 100644 --- a/stroom-core-client/src/main/java/stroom/dispatch/client/ExportFileCompleteUtil.java +++ b/stroom-core-client/src/main/java/stroom/dispatch/client/ExportFileCompleteUtil.java @@ -75,6 +75,8 @@ private static String getMessage(final ResourceGeneration result) { private static void download(final LocationManager locationManager, final ResourceGeneration result) { // Change the browser location to download the zip file. + // The name is needed so the browser has a default name for the download, but is not used + // by the servlet locationManager.replace(GWT.getHostPageBaseURL() + "resourcestore/" + result.getResourceKey().getName() + diff --git a/stroom-core-client/src/main/java/stroom/security/client/api/event/LogoutEvent.java b/stroom-core-client/src/main/java/stroom/security/client/api/event/LogoutEvent.java index 8cec8544d8f..7558dde9a22 100644 --- a/stroom-core-client/src/main/java/stroom/security/client/api/event/LogoutEvent.java +++ b/stroom-core-client/src/main/java/stroom/security/client/api/event/LogoutEvent.java @@ -49,6 +49,10 @@ protected void dispatch(final LogoutHandler handler) { handler.onLogout(this); } + + // -------------------------------------------------------------------------------- + + public interface LogoutHandler extends EventHandler { void onLogout(LogoutEvent event); diff --git a/stroom-core-shared/src/main/java/stroom/dashboard/shared/DashboardResource.java b/stroom-core-shared/src/main/java/stroom/dashboard/shared/DashboardResource.java index 97c67ff1398..340f6f85e1c 100644 --- a/stroom-core-shared/src/main/java/stroom/dashboard/shared/DashboardResource.java +++ b/stroom-core-shared/src/main/java/stroom/dashboard/shared/DashboardResource.java @@ -42,7 +42,7 @@ public interface DashboardResource extends RestResource, DirectRestService, Fetc String BASE_PATH = "/dashboard" + ResourcePaths.V1; - String DOWNLOAD_SEARCH_RESULTS_PATH_PATH = "/downloadSearchResults"; + String DOWNLOAD_SEARCH_RESULTS_PATH_PART = "/downloadSearchResults"; String SEARCH_PATH_PART = "/search"; String COLUMN_VALUES_PATH_PART = "/columnValues"; String NODE_NAME_PATH_PARAM = "/{nodeName}"; @@ -80,7 +80,7 @@ ResourceGeneration downloadQuery( DashboardSearchRequest request); @POST - @Path(DOWNLOAD_SEARCH_RESULTS_PATH_PATH + NODE_NAME_PATH_PARAM) + @Path(DOWNLOAD_SEARCH_RESULTS_PATH_PART + NODE_NAME_PATH_PARAM) @Operation( summary = "Download search results", operationId = "downloadDashboardSearchResultsNode") @@ -89,7 +89,7 @@ ResourceGeneration downloadSearchResults( @Parameter(description = "request", required = true) DownloadSearchResultsRequest request); @POST - @Path(DOWNLOAD_SEARCH_RESULTS_PATH_PATH) + @Path(DOWNLOAD_SEARCH_RESULTS_PATH_PART) @Operation( summary = "Download search results", operationId = "downloadDashboardSearchResultsLocal") diff --git a/stroom-core/src/main/java/stroom/core/servlet/SignInServlet.java b/stroom-core/src/main/java/stroom/core/servlet/SignInServlet.java index 1a2f5024736..0d589433435 100644 --- a/stroom-core/src/main/java/stroom/core/servlet/SignInServlet.java +++ b/stroom-core/src/main/java/stroom/core/servlet/SignInServlet.java @@ -44,4 +44,9 @@ String getScript() { public Set getPathSpecs() { return PATH_SPECS; } + + @Override + public String getName() { + return ResourcePaths.SIGN_IN_SERVLET_NAME; + } } diff --git a/stroom-core/src/main/java/stroom/core/servlet/StroomServlet.java b/stroom-core/src/main/java/stroom/core/servlet/StroomServlet.java index 5140b8307a4..f440c88e7c7 100644 --- a/stroom-core/src/main/java/stroom/core/servlet/StroomServlet.java +++ b/stroom-core/src/main/java/stroom/core/servlet/StroomServlet.java @@ -19,6 +19,7 @@ import stroom.ui.config.shared.UiConfig; import stroom.ui.config.shared.UserPreferencesService; import stroom.util.shared.IsServlet; +import stroom.util.shared.ResourcePaths; import jakarta.inject.Inject; import jakarta.inject.Provider; @@ -43,4 +44,9 @@ String getScript() { public Set getPathSpecs() { return PATH_SPECS; } + + @Override + public String getName() { + return ResourcePaths.STROOM_SERVLET_NAME; + } } diff --git a/stroom-dashboard/stroom-dashboard-impl/src/main/java/stroom/dashboard/impl/DashboardResourceImpl.java b/stroom-dashboard/stroom-dashboard-impl/src/main/java/stroom/dashboard/impl/DashboardResourceImpl.java index 351b1a0ae7e..f64f23e8f52 100644 --- a/stroom-dashboard/stroom-dashboard-impl/src/main/java/stroom/dashboard/impl/DashboardResourceImpl.java +++ b/stroom-dashboard/stroom-dashboard-impl/src/main/java/stroom/dashboard/impl/DashboardResourceImpl.java @@ -101,7 +101,7 @@ public ResourceGeneration downloadSearchResults(final String nodeName, ResourceGeneration.class, () -> ResourcePaths.buildAuthenticatedApiPath( DashboardResource.BASE_PATH, - DashboardResource.DOWNLOAD_SEARCH_RESULTS_PATH_PATH, + DashboardResource.DOWNLOAD_SEARCH_RESULTS_PATH_PART, node), () -> dashboardServiceProvider.get().downloadSearchResults(request), builder -> builder.post(Entity.json(request))); diff --git a/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/AuthenticationBypassCheckerImpl.java b/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/AuthenticationBypassCheckerImpl.java index 1a8d8edfb41..32690651c1a 100644 --- a/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/AuthenticationBypassCheckerImpl.java +++ b/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/AuthenticationBypassCheckerImpl.java @@ -14,7 +14,8 @@ public class AuthenticationBypassCheckerImpl implements AuthenticationBypassChecker { private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(AuthenticationBypassCheckerImpl.class); - // Servlet name taken from AbstractServerFactory + // Servlet name hard coded in io.dropwizard.core.server.AbstractServerFactory, so this + // should match that. private static final String JERSEY_SERVLET_NAME = "jersey"; // These are populated by a single thread when dropwizard boots, so no need for synch diff --git a/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/Servlets.java b/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/Servlets.java index 126b8edae7a..d022b1be74f 100644 --- a/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/Servlets.java +++ b/stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/Servlets.java @@ -51,27 +51,10 @@ public class Servlets { public void register() { final ServletContextHandler servletContextHandler = environment.getApplicationContext(); - // Check for duplicate servlet path specs, assumes they are globally unique - final List duplicatePaths = servlets.stream() - .flatMap(servlet -> - servlet.getPathSpecs().stream()) - .collect(Collectors.groupingBy(Function.identity(), Collectors.counting())) - .entrySet() - .stream() - .filter(entry -> entry.getValue() > 1) - .map(Map.Entry::getKey) - .collect(Collectors.toList()); - - if (!duplicatePaths.isEmpty()) { - throw new RuntimeException(LogUtil.message( - "Multiple servlets exist for each of the following servlet paths [{}]", - String.join(", ", duplicatePaths))); - } + validateServlets(); LOGGER.info("Adding servlets to application path/port:"); - final Set allPaths = new HashSet<>(); - final int maxNameLength = servlets.stream() .mapToInt(servlet -> servlet.getClass().getName().length()) .max() @@ -82,28 +65,62 @@ public void register() { .flatMap(servlet -> servlet.getPathSpecs().stream() .map(partialPathSpec -> { - final String name = servlet.getClass().getName(); +// final String name = servlet.getClass().getName(); + final String servletName = servlet.getName(); Objects.requireNonNull(partialPathSpec); final String fullPathSpec = ResourcePaths.buildServletPath(partialPathSpec); - return new ServletInfo(servlet, name, fullPathSpec); + return new ServletInfo(servlet, servletName, fullPathSpec); })) .sorted(Comparator.comparing(ServletInfo::fullPathSpec)) .forEach(servletInfo -> { - addServlet( servletContextHandler, allPaths, maxNameLength, servletInfo.servlet, - servletInfo.name, + servletInfo.servletName, servletInfo.fullPathSpec); }); } + private void validateServlets() { + // Check for duplicate servlet path specs, assumes they are globally unique + final List duplicatePaths = servlets.stream() + .flatMap(servlet -> + servlet.getPathSpecs().stream()) + .collect(Collectors.groupingBy(Function.identity(), Collectors.counting())) + .entrySet() + .stream() + .filter(entry -> entry.getValue() > 1) + .map(Map.Entry::getKey) + .toList(); + + if (!duplicatePaths.isEmpty()) { + throw new RuntimeException(LogUtil.message( + "Multiple servlets exist for each of the following servlet paths [{}]", + String.join(", ", duplicatePaths))); + } + + final List duplicateNames = servlets.stream() + .map(IsServlet::getName) + .collect(Collectors.groupingBy(Function.identity(), Collectors.counting())) + .entrySet() + .stream() + .filter(entry -> entry.getValue() > 1) + .map(Map.Entry::getKey) + .toList(); + + if (!duplicateNames.isEmpty()) { + throw new RuntimeException(LogUtil.message( + "Multiple servlets exist for each of the following names [{}]. Servlets must have unique names", + String.join(", ", duplicateNames))); + } + } + private void addServlet(final ServletContextHandler servletContextHandler, final Set allPaths, final int maxNameLength, - final IsServlet servlet, + final IsServlet aServlet, final String servletName, final String fullPathSpec) { @@ -116,7 +133,7 @@ private void addServlet(final ServletContextHandler servletContextHandler, throw new RuntimeException(LogUtil.message("Duplicate servlet path {}", fullPathSpec)); } else { final boolean isUnauthenticated; - if (servlet.getClass().isAnnotationPresent(Unauthenticated.class)) { + if (aServlet.getClass().isAnnotationPresent(Unauthenticated.class)) { isUnauthenticated = true; authenticationBypassCheckerImpl.registerUnauthenticatedServletName(servletName); } else { @@ -135,17 +152,17 @@ private void addServlet(final ServletContextHandler servletContextHandler, suffix); final ServletHolder servletHolder; - try { - servletHolder = new ServletHolder(servletName, (Servlet) servlet); - } catch (final ClassCastException e) { + if (aServlet instanceof final Servlet servlet) { + servletHolder = new ServletHolder(servletName, servlet); + } else { throw new RuntimeException(LogUtil.message("Injected class {} is not a Servlet", - servlet.getClass().getName())); + LogUtil.getClassName(aServlet))); } servletContextHandler.addServlet(servletHolder, fullPathSpec); allPaths.add(fullPathSpec); - registerHealthCheck(servlet, fullPathSpec); + registerHealthCheck(aServlet, fullPathSpec); } } @@ -155,7 +172,7 @@ private void registerHealthCheck(final IsServlet servlet, final HealthCheckRegistry healthCheckRegistry = environment.healthChecks(); final String name = servlet.getClass().getName(); - if (servlet instanceof HasHealthCheck) { + if (servlet instanceof final HasHealthCheck hasHealthCheck) { LOGGER.info("Adding health check for servlet {}", name); // object has a getHealth method so build a HealthCheck that wraps it and // adds in the servlet path information @@ -164,7 +181,7 @@ private void registerHealthCheck(final IsServlet servlet, protected HealthCheck.Result check() { // Decorate the existing health check results with the full path spec // as the servlet doesn't know its own full path - HealthCheck.Result result = ((HasHealthCheck) servlet).getHealth(); + HealthCheck.Result result = hasHealthCheck.getHealth(); final HealthCheck.ResultBuilder builder = Result.builder(); if (result.isHealthy()) { @@ -210,7 +227,7 @@ protected HealthCheck.Result check() { private record ServletInfo( IsServlet servlet, - String name, + String servletName, String fullPathSpec) { } diff --git a/stroom-importexport/stroom-importexport-impl/src/main/java/stroom/importexport/impl/ContentResourceImpl.java b/stroom-importexport/stroom-importexport-impl/src/main/java/stroom/importexport/impl/ContentResourceImpl.java index ad00a531109..72c190c2233 100644 --- a/stroom-importexport/stroom-importexport-impl/src/main/java/stroom/importexport/impl/ContentResourceImpl.java +++ b/stroom-importexport/stroom-importexport-impl/src/main/java/stroom/importexport/impl/ContentResourceImpl.java @@ -56,10 +56,10 @@ public class ContentResourceImpl implements ContentResource { private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(ContentResourceImpl.class); - final Provider eventLoggingServiceProvider; - final Provider contentServiceProvider; - final Provider explorerNodeServiceProvider; - final Provider securityContextProvider; + private final Provider eventLoggingServiceProvider; + private final Provider contentServiceProvider; + private final Provider explorerNodeServiceProvider; + private final Provider securityContextProvider; @Inject ContentResourceImpl(final Provider eventLoggingServiceProvider, @@ -245,4 +245,5 @@ private Query buildRawQuery(final String userInput) { + "\"") .build(); } + } diff --git a/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/ScheduledTaskExecutor.java b/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/ScheduledTaskExecutor.java index 1e11e5e3a44..6ba6119b8f2 100644 --- a/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/ScheduledTaskExecutor.java +++ b/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/ScheduledTaskExecutor.java @@ -189,7 +189,8 @@ private void execute() { LOGGER.logDurationIfDebugEnabled( function, () -> scheduledJobToStr(scheduledJob)); } catch (final RuntimeException e) { - LOGGER.error("Error executing task '{}'", taskName, e); + LOGGER.error("Error executing task '{}' - {}", + taskName, LogUtil.exceptionMessage(e), e); } }); diff --git a/stroom-resource/stroom-resource-api/src/main/java/stroom/resource/api/ResourceStore.java b/stroom-resource/stroom-resource-api/src/main/java/stroom/resource/api/ResourceStore.java index 7a7e41a9f3b..28f905a8413 100644 --- a/stroom-resource/stroom-resource-api/src/main/java/stroom/resource/api/ResourceStore.java +++ b/stroom-resource/stroom-resource-api/src/main/java/stroom/resource/api/ResourceStore.java @@ -26,8 +26,12 @@ */ public interface ResourceStore { + String RESOURCE_STORE_PATH_PART = "/resourcestore"; + /** * Create a temporary file and give it a UUID. + * + * @param name The name of the file resource. Held mostly for logging/debug purposes. */ ResourceKey createTempFile(String name); diff --git a/stroom-resource/stroom-resource-impl/build.gradle b/stroom-resource/stroom-resource-impl/build.gradle index 198248ddeaf..67cc7b3197c 100644 --- a/stroom-resource/stroom-resource-impl/build.gradle +++ b/stroom-resource/stroom-resource-impl/build.gradle @@ -4,6 +4,7 @@ dependencies { implementation project(':stroom-job:stroom-job-api') implementation project(':stroom-lifecycle:stroom-lifecycle-api') implementation project(':stroom-resource:stroom-resource-api') + implementation project(':stroom-security:stroom-security-api') implementation project(':stroom-task:stroom-task-api') implementation project(':stroom-util') implementation project(':stroom-util-shared') @@ -14,6 +15,10 @@ dependencies { implementation libs.jakarta.servlet.api implementation libs.jakarta.inject implementation libs.slf4j.api + implementation libs.ws.rs.api + + testImplementation project(':stroom-security:stroom-security-mock') + testImplementation project(':stroom-test-common') testImplementation libs.bundles.common.test.implementation testRuntimeOnly libs.bundles.common.test.runtime diff --git a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/ResourceModule.java b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/ResourceModule.java index fe6ff31482e..d7456d6c257 100644 --- a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/ResourceModule.java +++ b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/ResourceModule.java @@ -20,6 +20,7 @@ import stroom.lifecycle.api.LifecycleBinder; import stroom.resource.api.ResourceStore; import stroom.util.RunnableWrapper; +import stroom.util.guice.ServletBinder; import com.google.inject.AbstractModule; import jakarta.inject.Inject; @@ -28,40 +29,55 @@ public class ResourceModule extends AbstractModule { @Override protected void configure() { - bind(ResourceStore.class).to(ResourceStoreImpl.class); + bind(ResourceStore.class).to(UserResourceStoreImpl.class); + + ServletBinder.create(binder()) + .bind(UserResourceStoreImpl.class); ScheduledJobsBinder.create(binder()) .bindJobTo(DeleteTempFile.class, builder -> builder - .name("Delete temp file") - .description("Deletes the resource store temporary file.") + .name("Delete temp resource files") + .description("Deletes resource store temporary files that are over one hour old.") .managed(false) - .frequencySchedule("1h")); + .frequencySchedule("10m")); LifecycleBinder.create(binder()) .bindStartupTaskTo(ResourceStoreStartup.class) .bindShutdownTaskTo(ResourceStoreShutdown.class); } + + // -------------------------------------------------------------------------------- + + private static class DeleteTempFile extends RunnableWrapper { @Inject - DeleteTempFile(final ResourceStoreImpl resourceStore) { + DeleteTempFile(final UserResourceStoreImpl resourceStore) { super(resourceStore::execute); } } + + // -------------------------------------------------------------------------------- + + private static class ResourceStoreStartup extends RunnableWrapper { @Inject - ResourceStoreStartup(final ResourceStoreImpl resourceStore) { + ResourceStoreStartup(final UserResourceStoreImpl resourceStore) { super(resourceStore::startup); } } + + // -------------------------------------------------------------------------------- + + private static class ResourceStoreShutdown extends RunnableWrapper { @Inject - ResourceStoreShutdown(final ResourceStoreImpl resourceStore) { + ResourceStoreShutdown(final UserResourceStoreImpl resourceStore) { super(resourceStore::shutdown); } } diff --git a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/ResourceStoreImpl.java b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/ResourceStoreImpl.java deleted file mode 100644 index 395c001b991..00000000000 --- a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/ResourceStoreImpl.java +++ /dev/null @@ -1,191 +0,0 @@ -/* - * Copyright 2016 Crown Copyright - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package stroom.resource.impl; - -import stroom.resource.api.ResourceStore; -import stroom.task.api.TaskContextFactory; -import stroom.util.io.FileUtil; -import stroom.util.io.TempDirProvider; -import stroom.util.logging.LambdaLogger; -import stroom.util.logging.LambdaLoggerFactory; -import stroom.util.shared.ResourceKey; - -import jakarta.inject.Inject; -import jakarta.inject.Singleton; - -import java.io.IOException; -import java.io.UncheckedIOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.time.Instant; -import java.util.Map; -import java.util.UUID; -import java.util.concurrent.ConcurrentHashMap; - -/** - * Simple Store that gives you 1 hour to use your temp file before it deletes it. - */ -@Singleton -public class ResourceStoreImpl implements ResourceStore { - - private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(ResourceStoreImpl.class); - - private final TempDirProvider tempDirProvider; - private final TaskContextFactory taskContextFactory; - private final Map currentFiles = new ConcurrentHashMap<>(); - - private volatile Instant lastCleanupTime; - - @Inject - public ResourceStoreImpl(final TempDirProvider tempDirProvider, - final TaskContextFactory taskContextFactory) { - this.tempDirProvider = tempDirProvider; - this.taskContextFactory = taskContextFactory; - } - - private Path getTempDir() { - final Path tempDir = tempDirProvider.get().resolve("resources"); - try { - Files.createDirectories(tempDir); - } catch (final IOException e) { - throw new UncheckedIOException(e); - } - return tempDir; - } - - void startup() { - FileUtil.deleteContents(getTempDir()); - } - - void shutdown() { - FileUtil.deleteContents(getTempDir()); - } - - @Override - public ResourceKey createTempFile(final String name) { - final String uuid = UUID.randomUUID().toString(); - final Path path = getTempDir().resolve(uuid); - final ResourceKey resourceKey = new ResourceKey(uuid, name); - final ResourceItem resourceItem = new ResourceItem(resourceKey, path, Instant.now()); - LOGGER.debug("Created resourceItem: {}", resourceItem); - currentFiles.put(uuid, resourceItem); - return resourceKey; - } - - @Override - public void deleteTempFile(final ResourceKey resourceKey) { - final ResourceItem resourceItem = currentFiles.remove(resourceKey.getKey()); - if (resourceItem != null) { - final Path file = resourceItem.getPath(); - try { - Files.deleteIfExists(file); - LOGGER.debug("Deleted resourceItem: {}", resourceItem); - } catch (final IOException e) { - throw new UncheckedIOException(e); - } - } - } - - @Override - public Path getTempFile(final ResourceKey resourceKey) { - // File gone ! - final ResourceItem resourceItem = currentFiles.get(resourceKey.getKey()); - if (resourceItem == null) { - LOGGER.debug("resourceItem not found for resourceKey: {}", resourceKey); - return null; - } - resourceItem.setLastAccessTime(Instant.now()); - LOGGER.debug("Returning resourceItem: {}", resourceItem); - return resourceItem.getPath(); - } - - void execute() { - taskContextFactory.current().info(() -> "Deleting temp files"); - cleanup(); - } - - /** - * Delete files that haven't been accessed since the last cleanup. - * This allows us to choose the cleanup frequency. - */ - private synchronized void cleanup() { - if (lastCleanupTime != null) { - // Delete anything that hasn't been accessed since we were last asked to cleanup. - currentFiles.values().forEach(resourceItem -> { - try { - if (resourceItem.getLastAccessTime().isBefore(lastCleanupTime)) { - deleteTempFile(resourceItem.getResourceKey()); - } - } catch (final RuntimeException e) { - LOGGER.error(e::getMessage, e); - } - }); - } - lastCleanupTime = Instant.now(); - } - - - // -------------------------------------------------------------------------------- - - - private static class ResourceItem { - - private final ResourceKey resourceKey; - private final Path path; - private final Instant createTime; - private volatile Instant lastAccessTime; - - public ResourceItem(final ResourceKey resourceKey, - final Path path, - final Instant createTime) { - this.resourceKey = resourceKey; - this.path = path; - this.createTime = createTime; - this.lastAccessTime = createTime; - } - - public ResourceKey getResourceKey() { - return resourceKey; - } - - public Path getPath() { - return path; - } - - public Instant getCreateTime() { - return createTime; - } - - public Instant getLastAccessTime() { - return lastAccessTime; - } - - public void setLastAccessTime(final Instant lastAccessTime) { - this.lastAccessTime = lastAccessTime; - } - - @Override - public String toString() { - return "ResourceItem{" + - "resourceKey=" + resourceKey + - ", path=" + path + - ", createTime=" + createTime + - ", lastAccessTime=" + lastAccessTime + - '}'; - } - } -} diff --git a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceModule.java b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceModule.java deleted file mode 100644 index 6c583fb86db..00000000000 --- a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceModule.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * - * * Copyright 2018 Crown Copyright - * * - * * Licensed under the Apache License, Version 2.0 (the "License"); - * * you may not use this file except in compliance with the License. - * * You may obtain a copy of the License at - * * - * * http://www.apache.org/licenses/LICENSE-2.0 - * * - * * Unless required by applicable law or agreed to in writing, software - * * distributed under the License is distributed on an "AS IS" BASIS, - * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * * See the License for the specific language governing permissions and - * * limitations under the License. - * - */ - -package stroom.resource.impl; - -import stroom.resource.api.ResourceStore; -import stroom.util.guice.ServletBinder; - -import com.google.inject.AbstractModule; - -public class SessionResourceModule extends AbstractModule { - - @Override - protected void configure() { - bind(ResourceStore.class).to(SessionResourceStoreImpl.class); - - ServletBinder.create(binder()) - .bind(SessionResourceStoreImpl.class); - } -} diff --git a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java deleted file mode 100644 index 0f1aadeb21d..00000000000 --- a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/SessionResourceStoreImpl.java +++ /dev/null @@ -1,164 +0,0 @@ -/* - * Copyright 2016 Crown Copyright - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package stroom.resource.impl; - -import stroom.resource.api.ResourceStore; -import stroom.util.logging.LambdaLogger; -import stroom.util.logging.LambdaLoggerFactory; -import stroom.util.logging.LogUtil; -import stroom.util.servlet.HttpServletRequestHolder; -import stroom.util.servlet.SessionUtil; -import stroom.util.shared.IsServlet; -import stroom.util.shared.ResourceKey; - -import jakarta.inject.Inject; -import jakarta.servlet.http.HttpServlet; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import jakarta.servlet.http.HttpSession; - -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Set; -import java.util.UUID; - -/** - * Wrapper for the ResourceStore that makes sure the user can only access stuff - * in the session. - */ -public class SessionResourceStoreImpl extends HttpServlet implements ResourceStore, IsServlet { - - private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(SessionResourceStoreImpl.class); - - private static final Set PATH_SPECS = Set.of("/resourcestore/*"); - private static final String UUID_ARG = "uuid"; - private static final String ZIP_EXTENSION = ".zip"; - - private final ResourceStoreImpl resourceStore; - private final HttpServletRequestHolder httpServletRequestHolder; - - @Inject - SessionResourceStoreImpl(final ResourceStoreImpl resourceStore, - final HttpServletRequestHolder httpServletRequestHolder) { - this.resourceStore = resourceStore; - this.httpServletRequestHolder = httpServletRequestHolder; - } - - @Override - public ResourceKey createTempFile(final String name) { - final ResourceKey key = resourceStore.createTempFile(name); - final ResourceKey sessionKey = new ResourceKey(UUID.randomUUID().toString(), name); - - getMap().put(sessionKey, key); - - return sessionKey; - } - - @Override - public Path getTempFile(final ResourceKey key) { - final ResourceKey realKey = getRealKey(key); - if (realKey == null) { - return null; - } - return resourceStore.getTempFile(realKey); - } - - @Override - public void deleteTempFile(final ResourceKey key) { - final ResourceKey realKey = getRealKey(key); - if (realKey == null) { - return; - } - resourceStore.deleteTempFile(realKey); - } - - private ResourceKey getRealKey(final ResourceKey key) { - return getMap().get(key); - } - - private ResourceMap getMap() { - final ResourceMap resourceMap; - - final HttpServletRequest request = httpServletRequestHolder.get(); - if (request == null) { - throw new NullPointerException("Request holder has no current request"); - } - - final String name = "SESSION_RESOURCE_STORE"; - final HttpSession session = SessionUtil.getOrCreateSession(request, newSession -> - LOGGER.info(() -> LogUtil.message("getMap() - Created session {}", - SessionUtil.getSessionId(newSession)))); - final Object object = session.getAttribute(name); - if (object == null) { - resourceMap = new ResourceMap(); - session.setAttribute(name, resourceMap); - } else { - resourceMap = (ResourceMap) object; - } - - return resourceMap; - } - - @Override - protected void doGet(final HttpServletRequest req, final HttpServletResponse resp) throws IOException { - // Get the current request. - final HttpServletRequest originalRequest = httpServletRequestHolder.get(); - // Set this request. - httpServletRequestHolder.set(req); - - try { - final String uuid = req.getParameter(UUID_ARG); - boolean found = false; - if (uuid != null) { - final ResourceKey resourceKey = getRealKey(new ResourceKey(uuid, null)); - if (resourceKey != null) { - try { - final Path tempFile = resourceStore.getTempFile(resourceKey); - if (tempFile != null && Files.isRegularFile(tempFile)) { - if (resourceKey.getName().toLowerCase().endsWith(ZIP_EXTENSION)) { - resp.setContentType("application/zip"); - } else { - resp.setContentType("application/octet-stream"); - } - resp.getOutputStream().write(Files.readAllBytes(tempFile)); - found = true; - } - } finally { - deleteTempFile(resourceKey); - } - } - } - - if (!found) { - resp.sendError(HttpServletResponse.SC_NOT_FOUND, "Resource not found"); - } - } finally { - // Reset current request. - httpServletRequestHolder.set(originalRequest); - } - } - - /** - * @return The part of the path that will be in addition to any base path, - * e.g. "/datafeed". - */ - @Override - public Set getPathSpecs() { - return PATH_SPECS; - } -} diff --git a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/UserResourceStoreImpl.java b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/UserResourceStoreImpl.java new file mode 100644 index 00000000000..fd0b3679f4f --- /dev/null +++ b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/UserResourceStoreImpl.java @@ -0,0 +1,414 @@ +/* + * Copyright 2016 Crown Copyright + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package stroom.resource.impl; + +import stroom.resource.api.ResourceStore; +import stroom.security.api.SecurityContext; +import stroom.security.api.UserIdentity; +import stroom.task.api.TaskContextFactory; +import stroom.util.io.FileUtil; +import stroom.util.io.TempDirProvider; +import stroom.util.logging.AsciiTable; +import stroom.util.logging.AsciiTable.Column; +import stroom.util.logging.LambdaLogger; +import stroom.util.logging.LambdaLoggerFactory; +import stroom.util.logging.LogUtil; +import stroom.util.shared.IsServlet; +import stroom.util.shared.NullSafe; +import stroom.util.shared.ResourceKey; +import stroom.util.shared.UserRef; + +import jakarta.inject.Inject; +import jakarta.inject.Singleton; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.ws.rs.core.MediaType; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.time.Instant; +import java.util.Comparator; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Consumer; + +/** + * Simple Store that gives you 1 hour to use your temp file before it deletes it. + * Resource items are only accessible to the user that created them. + * Resource items are stored on disk in the form + * 
{@code
+ * __
+ * }
+ * e.g. + * 
+ * 18d92d74-a5aa-4a81-83be-46fb6d84a60e__dd2a1a85-cfeb-45f6-b030-d941bc615058
+ *
+ */ +@Singleton +public class UserResourceStoreImpl extends HttpServlet implements ResourceStore, IsServlet { + + private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(UserResourceStoreImpl.class); + + // Path spec has to be wild carded as the path will include the name of the file so + // the browser knows what to call it when it is downloaded, e.g. + // http://localhost/resourcestore/Test_Dashboard__query-MRGPM.json?uuid=ca516bc7-3bcf-441f-828e-35fa2eb3e9b0 + // The name is ignored in the get request as it would not be used when hitting the resourceStore + // from curl. + private static final Set PATH_SPECS = Set.of("/resourcestore/*"); + private static final Duration MAX_RESOURCE_AGE = Duration.ofHours(1); + private static final String UUID_ARG = "uuid"; + private static final String KEEP_ARG = "delete"; + private static final String ZIP_EXTENSION = ".zip"; + static final String SEPARATOR = "__"; + + private final TempDirProvider tempDirProvider; + private final SecurityContext securityContext; + private final TaskContextFactory taskContextFactory; + private final Map resourceMap; + + @Inject + UserResourceStoreImpl(final TempDirProvider tempDirProvider, + final SecurityContext securityContext, + final TaskContextFactory taskContextFactory) { + this.tempDirProvider = tempDirProvider; + this.securityContext = securityContext; + this.taskContextFactory = taskContextFactory; + this.resourceMap = new ConcurrentHashMap<>(); + } + + @Override + public ResourceKey createTempFile(final String name) { + final String resourceKeyUuid = UUID.randomUUID().toString(); + final ResourceKey resourceKey = new ResourceKey(resourceKeyUuid, name); + final UserResourceKey userResourceKey = createUserResourceKey(resourceKey); + final String fileName = createFileName(userResourceKey); + final Path path = getTempDir().resolve(fileName); + final ResourceItem resourceItem = new ResourceItem(userResourceKey, path); + LOGGER.debug("Created resourceItem: {}", resourceItem); + resourceMap.put(userResourceKey, resourceItem); + return resourceKey; + } + + @Override + public Path getTempFile(final ResourceKey resourceKey) { + return getTempFile(resourceKey, Instant.now()); + } + + @Override + public void deleteTempFile(final ResourceKey resourceKey) { + final UserResourceKey userResourceKey = createUserResourceKey(Objects.requireNonNull(resourceKey)); + deleteTempFile(userResourceKey); + } + + @Override + protected void doGet(final HttpServletRequest req, final HttpServletResponse resp) throws IOException { + final String uuid = req.getParameter(UUID_ARG); + final String keepResourceArg = req.getParameter(KEEP_ARG); + final boolean shouldKeepResource = NullSafe.getOrElse(keepResourceArg, Boolean::parseBoolean, false); + + boolean found = false; + if (NullSafe.isNonEmptyString(uuid)) { + // Name is not important for finding the resource + final UserResourceKey userResourceKey = createUserResourceKey(ResourceKey.createSearchKey(uuid)); + final ResourceItem resourceItem = shouldKeepResource + ? resourceMap.get(userResourceKey) + : resourceMap.remove(userResourceKey); + if (resourceItem != null) { + // This resourceKey includes the name as it is the one that was created with the resource + final ResourceKey resourceKey = resourceItem.getResourceKey(); + try { + final Path tempFile = resourceItem.getPath(); + if (NullSafe.test(tempFile, Files::isRegularFile)) { + final String contentType = resourceKey.getName().toLowerCase().endsWith(ZIP_EXTENSION) + ? "application/zip" + : MediaType.APPLICATION_OCTET_STREAM; + resp.setContentType(contentType); + LOGGER.debug("doGet() - Responding with contentType: {}, tempFile {}", + contentType, tempFile); + resp.getOutputStream().write(Files.readAllBytes(tempFile)); + found = true; + } else { + LOGGER.debug("doGet() - tempFile is not a regular file {}", tempFile); + } + } catch (final Exception e) { + LOGGER.error("Error getting resourceKey: {}, resourceItem: {} - {}", + resourceKey, resourceItem, LogUtil.exceptionMessage(e), e); + throw e; + } finally { + if (!shouldKeepResource) { + deleteTempFile(resourceItem); + } + } + } else { + LOGGER.debug("doGet() - No resource found with uuid {}", uuid); + } + } + if (!found) { + resp.sendError(HttpServletResponse.SC_NOT_FOUND, "Resource not found"); + } + } + + /** + * @return The part of the path that will be in addition to any base path, + * e.g. "/datafeed". + */ + @Override + public Set getPathSpecs() { + return PATH_SPECS; + } + + void execute() { + taskContextFactory.current().info(() -> "Deleting old temporary resource files"); + cleanup(Instant.now()); + } + + /** + * For testing + */ + void execute(final Instant now) { + taskContextFactory.current().info(() -> "Deleting old temporary resource files"); + cleanup(now); + } + + synchronized void startup() { + final Path tempDir = getTempDir(); + LOGGER.info("Deleting temporary resources in {}", tempDir); + FileUtil.deleteContents(tempDir); + } + + synchronized void shutdown() { + final Path tempDir = getTempDir(); + LOGGER.info("Deleting temporary resources in {}", tempDir); + FileUtil.deleteContents(tempDir); + } + + int size() { + return resourceMap.size(); + } + + private UserRef getUserRef() { + try { + return securityContext.getUserRef(); + } catch (final Exception e) { + final UserIdentity userIdentity = securityContext.getUserIdentity(); + throw new RuntimeException(LogUtil.message("Unable to obtain a UserRef for user {} {}", + LogUtil.getSimpleClassName(userIdentity), userIdentity)); + } + } + + private String createFileName(final UserResourceKey userResourceKey) { + final UserRef userRef = userResourceKey.userRef(); + final ResourceKey resourceKey = userResourceKey.resourceKey(); + return userRef.getUuid() + SEPARATOR + resourceKey.getKey(); + } + + private Path getTempDir() { + final Path tempDir = tempDirProvider.get() + .resolve("resources"); + try { + Files.createDirectories(tempDir); + } catch (final IOException e) { + throw new UncheckedIOException(e); + } + return tempDir; + } + + private UserResourceKey createUserResourceKey(final ResourceKey resourceKey) { + final UserRef userRef = getUserRef(); + return new UserResourceKey(userRef, Objects.requireNonNull(resourceKey)); + } + + /** + * For testing + */ + Path getTempFile(final ResourceKey resourceKey, final Instant now) { + final UserResourceKey userResourceKey = createUserResourceKey(Objects.requireNonNull(resourceKey)); + return getTempFile(userResourceKey, now); + } + + private Path getTempFile(final UserResourceKey userResourceKey, final Instant now) { + final ResourceItem resourceItem = resourceMap.get(userResourceKey); + LOGGER.debug("getTempFile() - userResourceKey: {}, resourceItem: {}", userResourceKey, resourceItem); + final Path path; + if (resourceItem != null) { + LOGGER.debug("getTempFile() - Found resourceItem {}", resourceItem); + resourceItem.setLastAccessTime(now); + path = resourceItem.getPath(); + } else { + LOGGER.debug("getTempFile() - ResourceItem not found for userResourceKey {}", userResourceKey); + path = null; + } + LOGGER.debug("getTempFile() - Returning path: {}, userResourceKey {}", path, userResourceKey); + return path; + } + + private void deleteTempFile(final UserResourceKey userResourceKey) { + Objects.requireNonNull(userResourceKey); + final ResourceItem resourceItem = resourceMap.remove(userResourceKey); + if (resourceItem != null) { + deleteTempFile(resourceItem); + } else { + LOGGER.debug("deleteTempFile() - ResourceItem not found for userResourceKey {}", userResourceKey); + } + } + + /** + * You must remove {@link ResourceItem} from the resourceMap before or after calling this. + */ + private void deleteTempFile(final ResourceItem resourceItem) { + Objects.requireNonNull(resourceItem); + final Path file = resourceItem.getPath(); + try { + Files.deleteIfExists(file); + LOGGER.debug("deleteTempFile() - Deleted resourceItem: {}", resourceItem); + } catch (final IOException e) { + throw new UncheckedIOException(e); + } + } + + /** + * Delete files that haven't been accessed since the last cleanup. + * This allows us to choose the cleanup frequency. + */ + private synchronized void cleanup(final Instant now) { + final Instant thresholdTime = now.minus(MAX_RESOURCE_AGE); + final Iterator iterator = resourceMap.values().iterator(); + while (iterator.hasNext()) { + final ResourceItem resourceItem = iterator.next(); + if (resourceItem != null) { + try { + if (resourceItem.getLastAccessTime().isBefore(thresholdTime)) { + iterator.remove(); + deleteTempFile(resourceItem); + } + } catch (final RuntimeException e) { + LOGGER.error(e::getMessage, e); + } + } + } + } + + /** + * For testing + */ + void logContents(final Instant now, + final Consumer consumer) { + final List items = resourceMap.values() + .stream() + .sorted(Comparator.comparing(ResourceItem::getLastAccessTime).reversed()) + .toList(); + + final String table = AsciiTable.builder(items) + .withColumn(Column.of("name", ResourceItem::getName)) + .withColumn(Column.of("lastAccessTime", ResourceItem::getLastAccessTime)) + .withColumn(Column.of("lastAccessAge", item -> Duration.between(item.getLastAccessTime(), now))) + .withColumn(Column.of("path", ResourceItem::getPath)) + .build(); + + consumer.accept(table); + } + + + // -------------------------------------------------------------------------------- + + + private record UserResourceKey(UserRef userRef, ResourceKey resourceKey) { + + @Override + public String toString() { + return "UserResourceKey{" + + "userRef=" + userRef + + ", key=" + resourceKey.getKey() + + ", name=" + resourceKey.getName() + + '}'; + } + } + + + // -------------------------------------------------------------------------------- + + + private static class ResourceItem { + + private final UserResourceKey userResourceKey; + private final Path path; + private final Instant createTime; + private volatile Instant lastAccessTime; + + public ResourceItem(final UserResourceKey userResourceKey, + final Path path) { + this(userResourceKey, path, Instant.now()); + } + + public ResourceItem(final UserResourceKey userResourceKey, + final Path path, + final Instant createTime) { + this.userResourceKey = Objects.requireNonNull(userResourceKey); + this.path = Objects.requireNonNull(path); + this.createTime = Objects.requireNonNull(createTime); + this.lastAccessTime = createTime; + } + + public UserResourceKey getUserResourceKey() { + return userResourceKey; + } + + private ResourceKey getResourceKey() { + return userResourceKey.resourceKey(); + } + + public String getName() { + return getResourceKey().getName(); + } + + public Path getPath() { + return path; + } + + public Instant getCreateTime() { + return createTime; + } + + public Instant getLastAccessTime() { + return lastAccessTime; + } + + public void setLastAccessTime(final Instant lastAccessTime) { + this.lastAccessTime = lastAccessTime; + } + + @Override + public String toString() { + return "ResourceItem{" + + "userResourceKey=" + userResourceKey + + ", path=" + path + + ", createTime=" + createTime + + ", lastAccessTime=" + lastAccessTime + + ", lastAccessAge=" + Duration.between(lastAccessTime, Instant.now()) + + '}'; + } + } +} diff --git a/stroom-resource/stroom-resource-impl/src/test/java/stroom/resource/impl/TestResourceStore.java b/stroom-resource/stroom-resource-impl/src/test/java/stroom/resource/impl/TestResourceStore.java deleted file mode 100644 index dbdff00e208..00000000000 --- a/stroom-resource/stroom-resource-impl/src/test/java/stroom/resource/impl/TestResourceStore.java +++ /dev/null @@ -1,89 +0,0 @@ -/* - * Copyright 2016 Crown Copyright - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package stroom.resource.impl; - -import stroom.task.api.SimpleTaskContextFactory; -import stroom.util.shared.ResourceKey; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; - -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; - -import static org.assertj.core.api.Assertions.assertThat; - -class TestResourceStore { - - @TempDir - static Path tempDir; - - @Test - void testSimple() throws IOException { - final ResourceStoreImpl resourceStore = new ResourceStoreImpl(() -> tempDir, new SimpleTaskContextFactory()); - resourceStore.execute(); - - final ResourceKey key1 = resourceStore.createTempFile("TestResourceStore1.dat"); - assertThat(key1.getName()).isEqualTo("TestResourceStore1.dat"); - - final ResourceKey key2 = resourceStore.createTempFile("TestResourceStore2.dat"); - assertThat(key2.getName()).isEqualTo("TestResourceStore2.dat"); - - Files.createFile(resourceStore.getTempFile(key1)); - Files.createFile(resourceStore.getTempFile(key2)); - - assertThat(Files.isRegularFile(resourceStore.getTempFile(key1))).isTrue(); - assertThat(Files.isRegularFile(resourceStore.getTempFile(key2))).isTrue(); - - // Cleanup - resourceStore.execute(); - Path file1 = resourceStore.getTempFile(key1); - assertThat(Files.isRegularFile(file1)).isTrue(); - Path file2 = resourceStore.getTempFile(key2); - assertThat(Files.isRegularFile(file2)).isTrue(); - - // Cleanup - resourceStore.execute(); - - // Files should still exist as we have accessed them since last roll. - file1 = resourceStore.getTempFile(key1); - assertThat(Files.isRegularFile(file1)).isTrue(); - file2 = resourceStore.getTempFile(key2); - assertThat(Files.isRegularFile(file2)).isTrue(); - - // Cleanup - resourceStore.execute(); - file1 = resourceStore.getTempFile(key1); - assertThat(Files.isRegularFile(file1)).isTrue(); - resourceStore.execute(); - - // We should have deleted file2 as we didn't access since last cleanup. - file1 = resourceStore.getTempFile(key1); - assertThat(Files.isRegularFile(file1)).isTrue(); - assertThat(resourceStore.getTempFile(key2)).isNull(); - assertThat(Files.isRegularFile(file2)).isFalse(); - - // Now delete everything. - resourceStore.execute(); - resourceStore.execute(); - assertThat(resourceStore.getTempFile(key1)).isNull(); - assertThat(Files.isRegularFile(file1)).isFalse(); - assertThat(resourceStore.getTempFile(key2)).isNull(); - assertThat(Files.isRegularFile(file2)).isFalse(); - } -} diff --git a/stroom-resource/stroom-resource-impl/src/test/java/stroom/resource/impl/TestUserResourceStoreImpl.java b/stroom-resource/stroom-resource-impl/src/test/java/stroom/resource/impl/TestUserResourceStoreImpl.java new file mode 100644 index 00000000000..4af1f9c8f13 --- /dev/null +++ b/stroom-resource/stroom-resource-impl/src/test/java/stroom/resource/impl/TestUserResourceStoreImpl.java @@ -0,0 +1,414 @@ +/* + * Copyright 2016 Crown Copyright + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package stroom.resource.impl; + +import stroom.security.api.SecurityContext; +import stroom.security.mock.MockSecurityContext; +import stroom.task.api.SimpleTaskContextFactory; +import stroom.test.common.DirectorySnapshot; +import stroom.test.common.DirectorySnapshot.Snapshot; +import stroom.util.exception.ThrowingSupplier; +import stroom.util.logging.LambdaLogger; +import stroom.util.logging.LambdaLoggerFactory; +import stroom.util.shared.ResourceKey; +import stroom.util.shared.UserRef; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.time.Instant; +import java.util.Objects; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; + +@ExtendWith(MockitoExtension.class) +class TestUserResourceStoreImpl { + + private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(TestUserResourceStoreImpl.class); + + @TempDir + static Path tempDir; + @Mock + private SecurityContext mockSecurityContext; + + @Test + void testSimple() throws IOException { + final UserResourceStoreImpl resourceStore = new UserResourceStoreImpl( + () -> tempDir, + MockSecurityContext.getInstance(), + new SimpleTaskContextFactory()); + resourceStore.startup(); + resourceStore.execute(); + + final ResourceKey key1 = resourceStore.createTempFile("TestResourceStore1.dat"); + assertThat(key1.getName()) + .isEqualTo("TestResourceStore1.dat"); + + final ResourceKey key2 = resourceStore.createTempFile("TestResourceStore2.dat"); + assertThat(key2.getName()) + .isEqualTo("TestResourceStore2.dat"); + + Instant now = Instant.now().plus(Duration.ofMinutes(1)); + resourceStore.logContents(now, msg -> LOGGER.debug("\n{}", msg)); + + Files.createFile(resourceStore.getTempFile(key1, now)); + Files.createFile(resourceStore.getTempFile(key2, now)); + + now = now.plus(Duration.ofMinutes(1)); + assertThat(resourceStore.getTempFile(key1, now)) + .isRegularFile(); + assertThat(resourceStore.getTempFile(key2, now)) + .isRegularFile(); + + resourceStore.logContents(now, msg -> LOGGER.debug("\n{}", msg)); + + // Cleanup + now = now.plus(Duration.ofMinutes(1)); + resourceStore.execute(now); + Path file1 = resourceStore.getTempFile(key1, now); + assertThat(file1) + .isRegularFile(); + Path file2 = resourceStore.getTempFile(key2, now); + assertThat(file2) + .isRegularFile(); + assertThat(resourceStore.size()) + .isEqualTo(2); + + // Cleanup + now = now.plus(Duration.ofMinutes(1)); + resourceStore.logContents(now, msg -> LOGGER.debug("\n{}", msg)); + resourceStore.execute(now); + + // Files should still exist as their access age is not old enough + file1 = resourceStore.getTempFile(key1, now); + assertThat(file1) + .isRegularFile(); + file2 = resourceStore.getTempFile(key2, now); + assertThat(file2) + .isRegularFile(); + assertThat(resourceStore.size()) + .isEqualTo(2); + + // Cleanup + now = now.plus(Duration.ofMinutes(1)); + resourceStore.logContents(now, msg -> LOGGER.debug("\n{}", msg)); + resourceStore.execute(now); + + now = now.plus(Duration.ofMinutes(61)); + file1 = resourceStore.getTempFile(key1, now); + assertThat(file1) + .isRegularFile(); + resourceStore.logContents(now, msg -> LOGGER.debug("\n{}", msg)); + resourceStore.execute(now); + + // We should have deleted file2 as we didn't access since last cleanup. + file1 = resourceStore.getTempFile(key1, now); + resourceStore.logContents(now, msg -> LOGGER.debug("\n{}", msg)); + assertThat(file1) + .isRegularFile(); + assertThat(resourceStore.getTempFile(key2, now)) + .isNull(); + assertThat(file2) + .doesNotExist(); + + // Now delete everything. + now = now.plus(Duration.ofMinutes(61)); + resourceStore.execute(now); + assertThat(resourceStore.getTempFile(key1, now)) + .isNull(); + assertThat(file1).doesNotExist(); + assertThat(resourceStore.getTempFile(key2, now)) + .isNull(); + assertThat(file2) + .doesNotExist(); + } + + @Test + void testShutdown() throws IOException { + final UserResourceStoreImpl resourceStore = new UserResourceStoreImpl( + () -> tempDir, + MockSecurityContext.getInstance(), + new SimpleTaskContextFactory()); + + final ResourceKey key1 = resourceStore.createTempFile("TestResourceStore1.dat"); + assertThat(key1.getName()) + .isEqualTo("TestResourceStore1.dat"); + + final ResourceKey key2 = resourceStore.createTempFile("TestResourceStore2.dat"); + assertThat(key2.getName()) + .isEqualTo("TestResourceStore2.dat"); + final Path file1 = resourceStore.getTempFile(key1); + final Path file2 = resourceStore.getTempFile(key2); + + Files.createFile(file1); + Files.createFile(file2); + + assertThat(resourceStore.size()) + .isEqualTo(2); + + final Snapshot snapshot1 = DirectorySnapshot.of(tempDir); + LOGGER.debug("Snapshot1:\n{}", snapshot1.pathSnapshots() + .stream() + .map(Objects::toString) + .collect(Collectors.joining("\n"))); + + // resources dir + two files + assertThat(snapshot1.pathSnapshots()) + .hasSize(3); + + resourceStore.shutdown(); + + final Snapshot snapshot2 = DirectorySnapshot.of(tempDir); + LOGGER.debug("Snapshot2:\n{}", snapshot2.pathSnapshots() + .stream() + .map(Objects::toString) + .collect(Collectors.joining("\n"))); + + // resources dir still there + assertThat(snapshot2.pathSnapshots()) + .hasSize(1); + } + + @Test + void testStartup() throws IOException { + final UserResourceStoreImpl resourceStore = new UserResourceStoreImpl( + () -> tempDir, + MockSecurityContext.getInstance(), + new SimpleTaskContextFactory()); + + final ResourceKey key1 = resourceStore.createTempFile("TestResourceStore1.dat"); + assertThat(key1.getName()) + .isEqualTo("TestResourceStore1.dat"); + + final ResourceKey key2 = resourceStore.createTempFile("TestResourceStore2.dat"); + assertThat(key2.getName()) + .isEqualTo("TestResourceStore2.dat"); + final Path file1 = resourceStore.getTempFile(key1); + final Path file2 = resourceStore.getTempFile(key2); + + Files.createFile(file1); + Files.createFile(file2); + + assertThat(resourceStore.size()) + .isEqualTo(2); + + final Snapshot snapshot1 = DirectorySnapshot.of(tempDir); + LOGGER.debug("Snapshot1:\n{}", snapshot1.pathSnapshots() + .stream() + .map(Objects::toString) + .collect(Collectors.joining("\n"))); + + // resources dir + two files + assertThat(snapshot1.pathSnapshots()) + .hasSize(3); + + resourceStore.startup(); + + final Snapshot snapshot2 = DirectorySnapshot.of(tempDir); + LOGGER.debug("Snapshot2:\n{}", snapshot2.pathSnapshots() + .stream() + .map(Objects::toString) + .collect(Collectors.joining("\n"))); + + // resources dir still there + assertThat(snapshot2.pathSnapshots()) + .hasSize(1); + } + + @Test + void testDelete() throws IOException { + final UserResourceStoreImpl resourceStore = new UserResourceStoreImpl( + () -> tempDir, + MockSecurityContext.getInstance(), + new SimpleTaskContextFactory()); + + final ResourceKey key1 = resourceStore.createTempFile("TestResourceStore1.dat"); + assertThat(key1.getName()) + .isEqualTo("TestResourceStore1.dat"); + + final ResourceKey key2 = resourceStore.createTempFile("TestResourceStore2.dat"); + assertThat(key2.getName()) + .isEqualTo("TestResourceStore2.dat"); + final Path file1 = resourceStore.getTempFile(key1); + final Path file2 = resourceStore.getTempFile(key2); + + Files.createFile(file1); + Files.createFile(file2); + + assertThat(resourceStore.size()) + .isEqualTo(2); + + final Snapshot snapshot1 = DirectorySnapshot.of(tempDir); + LOGGER.debug("Snapshot1:\n{}", snapshot1.pathSnapshots() + .stream() + .map(Objects::toString) + .collect(Collectors.joining("\n"))); + + // resources dir + two files + assertThat(snapshot1.pathSnapshots()) + .hasSize(3); + + resourceStore.deleteTempFile(key1); + + final Snapshot snapshot2 = DirectorySnapshot.of(tempDir); + LOGGER.debug("Snapshot2:\n{}", snapshot2.pathSnapshots() + .stream() + .map(Objects::toString) + .collect(Collectors.joining("\n"))); + + // resources dir still there + assertThat(snapshot2.pathSnapshots()) + .hasSize(2); + + // No change as already deleted + resourceStore.deleteTempFile(key1); + assertThat(snapshot2.pathSnapshots()) + .hasSize(2); + + resourceStore.deleteTempFile(key2); + + final Snapshot snapshot3 = DirectorySnapshot.of(tempDir); + LOGGER.debug("Snapshot3:\n{}", snapshot3.pathSnapshots() + .stream() + .map(Objects::toString) + .collect(Collectors.joining("\n"))); + + // resources dir still there + assertThat(snapshot3.pathSnapshots()) + .hasSize(1); + } + + @Test + void testDifferentUsers() { + + final UserRef userRef1 = UserRef.builder() + .uuid("user1-uuid") + .subjectId("user1") + .build(); + final UserRef userRef2 = UserRef.builder() + .uuid("user2-uuid") + .subjectId("user2") + .build(); + + final UserResourceStoreImpl resourceStore = new UserResourceStoreImpl( + () -> tempDir, + mockSecurityContext, + new SimpleTaskContextFactory()); + + final ResourceKey key1 = asUser(userRef1, ThrowingSupplier.unchecked(() -> { + final ResourceKey key = resourceStore.createTempFile("TestResourceStore1.dat"); + assertThat(key.getName()) + .isEqualTo("TestResourceStore1.dat"); + Files.createFile(resourceStore.getTempFile(key)); + return key; + })); + + final ResourceKey key2 = asUser(userRef2, ThrowingSupplier.unchecked(() -> { + final ResourceKey key = resourceStore.createTempFile("TestResourceStore2.dat"); + assertThat(key.getName()) + .isEqualTo("TestResourceStore2.dat"); + Files.createFile(resourceStore.getTempFile(key)); + return key; + })); + + asUser(userRef1, () -> { + final Path file1 = resourceStore.getTempFile(key1); + assertThat(file1.getFileName().toString()) + .isNotNull() + .startsWith(userRef1.getUuid() + UserResourceStoreImpl.SEPARATOR); + + // User1 can't see user2's file + final Path file2 = resourceStore.getTempFile(key2); + assertThat(file2) + .isNull(); + }); + + asUser(userRef2, () -> { + // User2 can't see user1's file + final Path file1 = resourceStore.getTempFile(key1); + assertThat(file1) + .isNull(); + + final Path file2 = resourceStore.getTempFile(key2); + assertThat(file2.getFileName().toString()) + .isNotNull() + .startsWith(userRef2.getUuid() + UserResourceStoreImpl.SEPARATOR); + }); + + assertThat(DirectorySnapshot.of(tempDir).pathSnapshots().size()) + .isEqualTo(3); + assertThat(resourceStore.size()) + .isEqualTo(2); + + asUser(userRef1, () -> { + assertThat(resourceStore.getTempFile(key1)) + .isNotNull(); + resourceStore.deleteTempFile(key1); + assertThat(resourceStore.getTempFile(key1)) + .isNull(); + + // Can't see it + assertThat(resourceStore.getTempFile(key2)) + .isNull(); + // is a no-op as does not belong to user1 + resourceStore.deleteTempFile(key2); + // Can't see it + assertThat(resourceStore.getTempFile(key2)) + .isNull(); + }); + + assertThat(DirectorySnapshot.of(tempDir).pathSnapshots().size()) + .isEqualTo(2); + assertThat(resourceStore.size()) + .isEqualTo(1); + + asUser(userRef2, () -> { + assertThat(resourceStore.getTempFile(key2)) + .isNotNull(); + resourceStore.deleteTempFile(key2); + assertThat(resourceStore.getTempFile(key2)) + .isNull(); + }); + assertThat(DirectorySnapshot.of(tempDir).pathSnapshots().size()) + .isEqualTo(1); + assertThat(resourceStore.size()) + .isEqualTo(0); + + } + + private T asUser(final UserRef userRef, final Supplier supplier) { + Mockito.when(mockSecurityContext.getUserRef()) + .thenReturn(userRef); + return supplier.get(); + } + + private void asUser(final UserRef userRef, final Runnable runnable) { + Mockito.when(mockSecurityContext.getUserRef()) + .thenReturn(userRef); + runnable.run(); + } +} diff --git a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/AbstractUserIdentityFactory.java b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/AbstractUserIdentityFactory.java index a78dab068fa..84cd949680f 100644 --- a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/AbstractUserIdentityFactory.java +++ b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/AbstractUserIdentityFactory.java @@ -272,30 +272,34 @@ public Optional getAuthFlowUserIdentity(final HttpServletRequest r @Override public UserIdentity getServiceUserIdentity() { - // Ideally the token will get recreated by the refresh queue just before - // it expires so callers to this will find a token that is good to use and - // thus won't be contended. - final boolean didCreate; - if (serviceUserIdentity == null) { - synchronized (this) { - if (serviceUserIdentity == null) { - serviceUserIdentity = createServiceUserIdentity(); - didCreate = true; - } else { - didCreate = false; + try { + // Ideally the token will get recreated by the refresh queue just before + // it expires so callers to this will find a token that is good to use and + // thus won't be contended. + final boolean didCreate; + if (serviceUserIdentity == null) { + synchronized (this) { + if (serviceUserIdentity == null) { + serviceUserIdentity = createServiceUserIdentity(); + didCreate = true; + } else { + didCreate = false; + } } + } else { + didCreate = false; } - } else { - didCreate = false; - } - // Make sure it is up-to-date before giving it out - if (!didCreate && serviceUserIdentity instanceof final HasRefreshable hasRefreshable) { - NullSafe.consume(hasRefreshable.getRefreshable(), refreshable -> - refreshable.refreshIfRequired(RefreshMode.JUST_IN_TIME, refreshManager::addOrUpdate)); - } + // Make sure it is up-to-date before giving it out + if (!didCreate && serviceUserIdentity instanceof final HasRefreshable hasRefreshable) { + NullSafe.consume(hasRefreshable.getRefreshable(), refreshable -> + refreshable.refreshIfRequired(RefreshMode.JUST_IN_TIME, refreshManager::addOrUpdate)); + } - return serviceUserIdentity; + return serviceUserIdentity; + } catch (final Exception e) { + throw new RuntimeException("Error getting service user identity - " + LogUtil.exceptionMessage(e), e); + } } @Override @@ -379,7 +383,11 @@ protected FetchTokenResult refreshUsingRefreshToken(final UpdatableToken updatab newTokenResponse = fetchTokenResult.tokenResponse(); jwtClaims = fetchTokenResult.jwtClaims(); } catch (final RuntimeException e) { - LOGGER.error("Error refreshing token for {} - {}", identity, e.getMessage(), e); + LOGGER.error("Error refreshing token for {} {} ({}) - {}", + identity.getSubjectId(), + identity.getDisplayName(), + identity.getFullName().orElse("-"), + LogUtil.exceptionMessage(e), e); if (identity instanceof final HasSession userWithSession) { userWithSession.invalidateSession(); } diff --git a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/RefreshManager.java b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/RefreshManager.java index 290bd8aab3e..a347ecf3ef0 100644 --- a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/RefreshManager.java +++ b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/RefreshManager.java @@ -6,13 +6,17 @@ import stroom.util.logging.LambdaLoggerFactory; import stroom.util.logging.LogUtil; import stroom.util.shared.ModelStringUtil; +import stroom.util.shared.NullSafe; +import com.google.common.base.Throwables; import io.dropwizard.lifecycle.Managed; import jakarta.inject.Singleton; +import java.net.SocketException; import java.time.Duration; import java.time.Instant; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ConcurrentSkipListSet; @@ -27,6 +31,7 @@ public class RefreshManager implements Managed { private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(RefreshManager.class); + private static final Duration DELAY_AFTER_FAILURE = Duration.ofSeconds(30); private final BlockingQueue delayQueue = new DelayQueue<>(); // Quicker to check this than iterating over the queue @@ -36,11 +41,11 @@ public class RefreshManager implements Managed { private ExecutorService refreshExecutorService = null; public RefreshManager() { - } /** - * Remove this refreshable from being managed by {@link RefreshManager} + * Remove this refreshable from being managed by {@link RefreshManager}. + * Is a no-op if the refreshable is not present or has already been removed. */ public void remove(final Refreshable refreshable) { if (refreshable != null && !isShutdownInProgress.get()) { @@ -59,44 +64,66 @@ public void remove(final Refreshable refreshable) { } } + public void addOrUpdate(final Refreshable refreshable) { + addOrUpdate(refreshable, null); + } + /** * Register the refreshable so that its refreshing gets managed. Can be called many times * for the same {@link Refreshable}, e.g. to notify the {@link RefreshManager} that it was * refreshed externally. */ - public void addOrUpdate(final Refreshable refreshable) { + public void addOrUpdate(final Refreshable refreshable, final Duration forcedDelay) { // No point adding stuff if we are shutting down if (refreshable != null && !isShutdownInProgress.get()) { //noinspection SynchronizationOnLocalVariableOrMethodParameter synchronized (refreshable) { - // Assume this refreshable has the latest state so remove any existing items on the - // queue for this refreshable - remove(refreshable); - // Spawn a new DelayedRefreshable that takes a snapshot of the token's expiry // as we can't mutate the times being used by the delay queue - final DelayedRefreshable delayedRefreshable = new DelayedRefreshable(refreshable); + final DelayedRefreshable newDelayedRefreshable = new DelayedRefreshable(refreshable, forcedDelay); - delayQueue.add(delayedRefreshable); - uuidsInQueue.add(refreshable.getUuid()); + final DelayedRefreshable queuedDelayedRefreshable = findInQueue(newDelayedRefreshable) + .orElse(null); + // Only replace items if the expiry is longer than the existing one + if (queuedDelayedRefreshable == null + || newDelayedRefreshable.compareTo(queuedDelayedRefreshable) > 0) { + remove(refreshable); - LOGGER.debug(() -> LogUtil.message("Added {}", itemToString(delayedRefreshable))); + delayQueue.add(newDelayedRefreshable); + uuidsInQueue.add(refreshable.getUuid()); + + LOGGER.debug(() -> LogUtil.message("Added {}", itemToString(newDelayedRefreshable))); + } else { + // Not in the queue or it has a later expire-time than refreshable so do nothing + LOGGER.debug("Not adding {}, queuedDelayedRefreshable: {}, newDelayedRefreshable: {}", + refreshable, + NullSafe.get(queuedDelayedRefreshable, DelayedRefreshable::getDelayAsDuration), + newDelayedRefreshable.getDelayAsDuration()); + } } } } + private Optional findInQueue(final DelayedRefreshable delayedRefreshable) { + return delayQueue.stream() + .filter(aDelayedRefreshable -> + Objects.equals( + aDelayedRefreshable.getRefreshableUuid(), + delayedRefreshable.getRefreshableUuid())) + .findFirst(); + } + private void consumeFromQueue() { try { // We are called in an infinite while loop so drop out every 2s to allow // checking of shutdown state final DelayedRefreshable delayedRefreshable = delayQueue.poll(2, TimeUnit.SECONDS); if (delayedRefreshable != null) { + final Refreshable refreshable = delayedRefreshable.getRefreshable(); try { - final Refreshable refreshable = delayedRefreshable.getRefreshable(); // We've removed it from the queue so keep the set in sync with the queue. uuidsInQueue.remove(refreshable.getUuid()); - if (refreshable.isActive()) { // It is possible that someone getting the token has triggered a synchronous refresh, // so we may not need to do it. @@ -116,9 +143,7 @@ private void consumeFromQueue() { } } catch (final Exception e) { - LOGGER.error("Error consuming {} - {}", - itemToString(delayedRefreshable), LogUtil.exceptionMessage(e), e); - // We have to log, swallow and carry on, else our single thread dies + handleRefreshException(delayedRefreshable, e); } } } catch (final InterruptedException e) { @@ -127,12 +152,37 @@ private void consumeFromQueue() { } } + private void handleRefreshException(final DelayedRefreshable delayedRefreshable, + final Exception e) { + try { + final Throwable rootCause = Throwables.getRootCause(e); + // SocketException is a super of ConnectionException + if (rootCause instanceof SocketException) { + LOGGER.error("Socket/connection error when refreshing {} - {}. (Enable DEBUG for stack trace.)", + itemToString(delayedRefreshable), LogUtil.exceptionMessage(e)); + LOGGER.debug("Socket/connection error when refreshing {} - {}.", + itemToString(delayedRefreshable), LogUtil.exceptionMessage(e), e); + // Re-queue the item with a forced delay in the hope the IDP comes back + addOrUpdate(delayedRefreshable.getRefreshable(), DELAY_AFTER_FAILURE); + } else { + LOGGER.error("Error when refreshing {} - {}. (Enable DEBUG for stack trace.)", + itemToString(delayedRefreshable), LogUtil.exceptionMessage(e)); + LOGGER.debug("Error when refreshing {} - {}.", + itemToString(delayedRefreshable), LogUtil.exceptionMessage(e), e); + } + } catch (final Exception ex) { + // We have to log, swallow and carry on, else our single thread dies + LOGGER.error("Error adding item {} - {}", + itemToString(delayedRefreshable), LogUtil.exceptionMessage(e), e); + } + } + private String itemToString(final DelayedRefreshable delayedRefreshable) { if (delayedRefreshable != null) { final Refreshable refreshable = delayedRefreshable.getRefreshable(); return LogUtil.message("item {} ({}) of type {} from refresh queue, " + - "expireTime: {} ({}), expireTimeWithBuffer: {} ({}), queue size after: {}, " + - "item detail: {}", + "expireTime: {} ({}), expireTimeWithBuffer: {} ({}), queue size after: {}, " + + "item detail: {}", System.identityHashCode(refreshable), System.identityHashCode(delayedRefreshable), refreshable.getClass().getSimpleName(), @@ -154,8 +204,8 @@ private String itemToString(final DelayedRefreshable delayedRefreshable) { private String itemToString(final Refreshable refreshable) { if (refreshable != null) { return LogUtil.message("item {} of type {} from refresh queue, " + - "expireTime: {} ({}), expireTimeWithBuffer: {} ({}), queue size after: {}, " + - "item detail: {}", + "expireTime: {} ({}), expireTimeWithBuffer: {} ({}), queue size after: {}, " + + "item detail: {}", System.identityHashCode(refreshable), refreshable.getClass().getSimpleName(), LogUtil.instant(refreshable.getExpireTimeEpochMs()), @@ -175,34 +225,42 @@ private String itemToString(final Refreshable refreshable) { @Override public void start() throws Exception { - if (refreshExecutorService == null) { - LOGGER.info("Initialising RefreshManager executor"); - refreshExecutorService = Executors.newSingleThreadExecutor(); - refreshExecutorService.submit(() -> { - final Thread currentThread = Thread.currentThread(); - LOGGER.debug(() -> "Started RefreshManager on thread " + currentThread.getName()); - isStarted.set(true); - while (!currentThread.isInterrupted() && !isShutdownInProgress.get()) { - consumeFromQueue(); - } - LOGGER.info(() -> LogUtil.message( - "RefreshManager refresh thread {} finishing (isInterrupted: {}, isShutdownInProgress: {})", - currentThread.getName(), currentThread.isInterrupted(), isShutdownInProgress.get())); - }); + try { + if (refreshExecutorService == null) { + LOGGER.info("Initialising RefreshManager executor"); + refreshExecutorService = Executors.newSingleThreadExecutor(); + refreshExecutorService.submit(() -> { + final Thread currentThread = Thread.currentThread(); + LOGGER.debug(() -> "Started RefreshManager on thread " + currentThread.getName()); + isStarted.set(true); + while (!currentThread.isInterrupted() && !isShutdownInProgress.get()) { + consumeFromQueue(); + } + LOGGER.info(() -> LogUtil.message( + "RefreshManager refresh thread {} finishing (isInterrupted: {}, isShutdownInProgress: {})", + currentThread.getName(), currentThread.isInterrupted(), isShutdownInProgress.get())); + }); + } + } catch (final Exception e) { + LOGGER.error("Error starting - {}", LogUtil.exceptionMessage(e)); } } @Override public void stop() throws Exception { - isShutdownInProgress.set(true); - // If we are shutting down we don't care about items on the queue, so bin them - delayQueue.clear(); - if (refreshExecutorService != null) { - LOGGER.info("Shutting down RefreshManager executor"); - refreshExecutorService.shutdownNow(); - // No need to wait for termination the stuff on the queue has no value once - // we are shutting down - LOGGER.info("Successfully shut down RefreshManager executor"); + try { + isShutdownInProgress.set(true); + // If we are shutting down we don't care about items on the queue, so bin them + delayQueue.clear(); + if (refreshExecutorService != null) { + LOGGER.info("Shutting down RefreshManager executor"); + refreshExecutorService.shutdownNow(); + // No need to wait for termination the stuff on the queue has no value once + // we are shutting down + LOGGER.info("Successfully shut down RefreshManager executor"); + } + } catch (final Exception e) { + LOGGER.error("Error stopping - {}", LogUtil.exceptionMessage(e)); } } @@ -232,22 +290,25 @@ private static class DelayedRefreshable implements Delayed { private final Refreshable refreshable; DelayedRefreshable(final Refreshable refreshable) { + this(refreshable, null); + } + + /** + * Use this constructor in cases where there has been an error refreshing the refreshable + * so, we want to re-queue it with a delay that is un-related to the expire-time on the + * refreshable. + */ + DelayedRefreshable(final Refreshable refreshable, final Duration delay) { Objects.requireNonNull(refreshable); - this.refreshable = Objects.requireNonNull(refreshable); - // We need to snapshot the expireTime as that is mutable on the Refreshable + this.refreshable = refreshable; this.expireTimeEpochMs = refreshable.getExpireTimeEpochMs(); - - // We want to refresh items in the background slightly before the buffer threshold is reached - // as other people will be calling refreshIfRequired, so we want the refresh to have happened already -// long bufferMs; -// if (refreshable.getRefreshBufferMs() > 0) { -//// bufferMs = (long) (1.1 * refreshable.getRefreshBufferMs()); -// bufferMs = refreshable.getRefreshBufferMs(); -// } else { -// bufferMs = 0; -// } - - this.expireTimeWithBufferEpochMs = refreshable.getExpireTimeWithBufferEpochMs(RefreshMode.EAGER); + if (delay != null) { + this.expireTimeWithBufferEpochMs = Instant.now() + .plus(Objects.requireNonNull(delay)) + .toEpochMilli(); + } else { + this.expireTimeWithBufferEpochMs = refreshable.getExpireTimeWithBufferEpochMs(RefreshMode.EAGER); + } LOGGER.debug(() -> LogUtil.message( "expireTimeEpoch: {}, refreshBuffer: {}, expireTimeWithBufferEpoch: {}", LogUtil.instant(expireTimeEpochMs), @@ -256,6 +317,7 @@ private static class DelayedRefreshable implements Delayed { LogUtil.instant(expireTimeWithBufferEpochMs))); } + public String getRefreshableUuid() { return refreshable.getUuid(); } @@ -273,6 +335,10 @@ public long getDelay(final TimeUnit unit) { return unit.convert(diffMs, TimeUnit.MILLISECONDS); } + private Duration getDelayAsDuration() { + return Duration.between(Instant.now(), Instant.ofEpochMilli(expireTimeWithBufferEpochMs)); + } + @Override public int compareTo(final Delayed other) { return Long.compare( @@ -290,10 +356,10 @@ public Refreshable getRefreshable() { @Override public String toString() { return "DelayedRefreshable{" + - "expireTimeWithBufferEpoch=" + LogUtil.instant(expireTimeWithBufferEpochMs) + - "expireTimeEpoch=" + LogUtil.instant(expireTimeWithBufferEpochMs) + - ", refreshable=" + refreshable + - '}'; + "expireTimeWithBufferEpoch=" + LogUtil.instant(expireTimeWithBufferEpochMs) + + "expireTimeEpoch=" + LogUtil.instant(expireTimeWithBufferEpochMs) + + ", refreshable=" + refreshable + + '}'; } @Override diff --git a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UpdatableToken.java b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UpdatableToken.java index 06180c22ea9..c784367f139 100644 --- a/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UpdatableToken.java +++ b/stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/UpdatableToken.java @@ -124,7 +124,7 @@ public boolean refresh(final Consumer onRefreshAction) { didWork = false; } else { synchronized (this) { - final FetchTokenResult fetchTokenResult = updateFunction.apply(this); + final FetchTokenResult fetchTokenResult = fetchToken(); if (fetchTokenResult != null) { try { this.mutableState = createMutableState( @@ -146,6 +146,16 @@ public boolean refresh(final Consumer onRefreshAction) { return didWork; } + private FetchTokenResult fetchToken() { + try { + return updateFunction.apply(this); + } catch (final Exception e) { + LOGGER.error("Error fetching token - {}. Enable DEBUG for stack trace.", LogUtil.exceptionMessage(e)); + LOGGER.debug("Error fetching token - {}.", LogUtil.exceptionMessage(e), e); + throw e; + } + } + @Override public long getExpireTimeEpochMs() { return mutableState.expireTimeEpochMs; diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java index 8ff6435f89c..edbe39003b5 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/OpenIdManager.java @@ -129,7 +129,8 @@ private RedirectUrl backChannelOIDC(final HttpServletRequest request, // Successful login, so redirect to the original URL held in the state. final String uri = state.getInitiatingUri(); - LOGGER.info(() -> LogUtil.message("backChannelOIDC() - Redirecting to initiating URI: {}", uri)); + LOGGER.debug(() -> LogUtil.message( + "backChannelOIDC() - Using browser refresh to redirect to initiating URI: {}", uri)); redirectUri = RedirectUrl.createWithRefresh(uri); } else { redirectUri = RedirectUrl.create(createErrorUri("Authentication failed")); diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SecurityContextImpl.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SecurityContextImpl.java index e66b120afb8..9935153e1c2 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SecurityContextImpl.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SecurityContextImpl.java @@ -397,7 +397,13 @@ private void throwPermissionException(final UserRef userRef, final String messag */ @Override public T asProcessingUserResult(final Supplier supplier) { - return asUserResult(userIdentityFactory.getServiceUserIdentity(), supplier); + final UserIdentity serviceUserIdentity; + try { + serviceUserIdentity = userIdentityFactory.getServiceUserIdentity(); + } catch (final Exception e) { + throw new RuntimeException("Error running as processing user - " + LogUtil.exceptionMessage(e), e); + } + return asUserResult(serviceUserIdentity, supplier); } /** @@ -405,7 +411,13 @@ public T asProcessingUserResult(final Supplier supplier) { */ @Override public void asProcessingUser(final Runnable runnable) { - asUser(userIdentityFactory.getServiceUserIdentity(), runnable); + final UserIdentity serviceUserIdentity; + try { + serviceUserIdentity = userIdentityFactory.getServiceUserIdentity(); + } catch (final Exception e) { + throw new RuntimeException("Error running as processing user - " + LogUtil.exceptionMessage(e), e); + } + asUser(serviceUserIdentity, runnable); } /** 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 3248bbca244..64e7768c713 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 @@ -51,8 +51,8 @@ import java.io.IOException; import java.io.PrintWriter; +import java.util.Objects; import java.util.Optional; -import java.util.Set; /** * Filter to avoid posts to the wrong place (e.g. the root of the app) @@ -62,12 +62,6 @@ class SecurityFilter implements Filter { private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(SecurityFilter.class); - private static final Set STATIC_RESOURCE_EXTENSIONS = Set.of( - ".js", ".js.map", ".css", ".css.map", ".htm", ".html", ".json", ".png", - ".jpg", ".gif", ".ico", ".svg", ".ttf", ".woff", ".woff2"); - - private static final String SIGN_IN_URL_PATH = ResourcePaths.buildServletPath(ResourcePaths.SIGN_IN_PATH); - private final UriFactory uriFactory; private final SecurityContext securityContext; private final OpenIdManager openIdManager; @@ -135,14 +129,12 @@ private void filter(final HttpServletRequest request, final String fullPath = request.getRequestURI(); final String servletName = NullSafe.get(request.getHttpServletMapping(), HttpServletMapping::getServletName); - if (request.getMethod().equalsIgnoreCase(HttpMethod.OPTIONS)) { + if (HttpMethod.OPTIONS.equalsIgnoreCase(request.getMethod())) { // We need to allow CORS preflight requests LOGGER.debug("Passing on OPTIONS request to next filter, servletName: {}, fullPath: {}, servletPath: {}", servletName, fullPath, servletPath); chain.doFilter(request, response); - } else if (isStaticResource(fullPath, servletPath)) { - LOGGER.debug("Static content, servletName: {}, fullPath: {}, servletPath: {}", - servletName, fullPath, servletPath); + } else if (isStaticResource(fullPath, servletPath, servletName)) { chain.doFilter(request, response); } else { // Api requests that are not from the front-end should have a token. @@ -150,14 +142,6 @@ private void filter(final HttpServletRequest request, // Need to do this first, so we get a fresh token from AWS ALB rather than using a stale // one from session. Optional optUserIdentity = openIdManager.loginWithRequestToken(request); -// optUserIdentity.ifPresent(userIdentity -> -// ensureSessionIfCookiePresent(request).ifPresent(session -> { -// LOGGER.debug(() -> LogUtil.message("Setting user in session, user: {} {}, path: {}", -// userIdentity.getClass().getSimpleName(), -// userIdentity, -// fullPath)); -// UserIdentitySessionUtil.set(session, userIdentity); -// })); // Log current user. if (LOGGER.isDebugEnabled()) { @@ -178,10 +162,6 @@ private void filter(final HttpServletRequest request, // Now we have the session make note of the user-agent for logging and sessionListServlet duties UserAgentSessionUtil.setUserAgentInSession(request); -// SessionUtil.withSession(request, session -> -// UserAgentSessionUtil.setUserAgentInSession(request, session)); -// ensureSessionIfCookiePresent(request).ifPresent(session -> -// UserAgentSessionUtil.setUserAgentInSession(request, session)); // Now handle the request as this user securityContext.asUser(userIdentity, () -> @@ -194,15 +174,15 @@ private void filter(final HttpServletRequest request, securityContext.asProcessingUser(() -> process(request, response, chain)); - } else if (isApiRequest(servletPath)) { +// } else if (isApiRequest(servletPath)) { + } else if (Objects.equals(ResourcePaths.STROOM_SERVLET_NAME, servletName)) { + doOpenIdFlow(request, response, fullPath); + } else { // 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); response.setStatus(Response.Status.UNAUTHORIZED.getStatusCode()); - - } else { - doOpenIdFlow(request, response, fullPath); } } } @@ -294,31 +274,18 @@ private String getPostAuthRedirectUri(final HttpServletRequest request) { return uriFactory.publicUri(originalPath).toString(); } - private boolean isStaticResource(final String fullPath, final String servletPath) { + private boolean isStaticResource(final String fullPath, + final String servletPath, + final String servletName) { // Test for internal IdP sign in request. - if (servletPath.startsWith(SIGN_IN_URL_PATH)) { + if (ResourcePaths.UI_SERVLET_NAME.equals(servletName) + || ResourcePaths.SIGN_IN_SERVLET_NAME.equals(servletName)) { + LOGGER.debug("Unauthenticated static content, servletName: {}, fullPath: {}, servletPath: {}", + servletName, fullPath, servletPath); return true; + } else { + return false; } - - int index = fullPath.lastIndexOf("."); - if (index > 0) { - String extension = fullPath.substring(index).toLowerCase(); - if (STATIC_RESOURCE_EXTENSIONS.contains(extension)) { - return true; - } else { - // Handle stuff like .css.map - index = fullPath.lastIndexOf(".", index - 1); - if (index > 0) { - extension = fullPath.substring(index).toLowerCase(); - return STATIC_RESOURCE_EXTENSIONS.contains(extension); - } - } - } - return false; - } - - private boolean isApiRequest(final String servletPath) { - return servletPath.startsWith(ResourcePaths.API_ROOT_PATH); } private boolean shouldBypassAuthentication(final HttpServletRequest servletRequest, 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 90b177a035e..4677f8fc54c 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 @@ -20,7 +20,9 @@ import stroom.node.api.NodeCallUtil; import stroom.node.api.NodeInfo; import stroom.node.api.NodeService; +import stroom.security.api.SecurityContext; import stroom.security.common.impl.UserIdentitySessionUtil; +import stroom.security.shared.AppPermission; import stroom.security.shared.HasUserRef; import stroom.security.shared.SessionDetails; import stroom.security.shared.SessionListResponse; @@ -58,22 +60,30 @@ class SessionListListener implements HttpSessionListener, HttpSessionIdListener, private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(SessionListListener.class); - private final ConcurrentHashMap sessionMap = new ConcurrentHashMap<>(); + private final ConcurrentHashMap sessionIdToSessionMap = new ConcurrentHashMap<>(); private final NodeInfo nodeInfo; private final NodeService nodeService; private final TaskContextFactory taskContextFactory; private final WebTargetFactory webTargetFactory; + // Need to use the impl rather than iface as iface doesn't support all the + // user/session/codeFlow stuff + private final StroomUserIdentityFactory stroomUserIdentityFactory; + private final SecurityContext securityContext; @Inject SessionListListener(final NodeInfo nodeInfo, final NodeService nodeService, final TaskContextFactory taskContextFactory, - final WebTargetFactory webTargetFactory) { + final WebTargetFactory webTargetFactory, + final StroomUserIdentityFactory stroomUserIdentityFactory, + final SecurityContext securityContext) { this.nodeInfo = nodeInfo; this.nodeService = nodeService; this.taskContextFactory = taskContextFactory; this.webTargetFactory = webTargetFactory; + this.stroomUserIdentityFactory = stroomUserIdentityFactory; + this.securityContext = securityContext; } @Override @@ -81,13 +91,18 @@ public void sessionCreated(final HttpSessionEvent event) { final HttpSession httpSession = event.getSession(); final String sessionId = httpSession.getId(); LOGGER.debug("sessionCreated() - sessionId: {}", sessionId); - sessionMap.put(sessionId, httpSession); + sessionIdToSessionMap.put(sessionId, httpSession); } @Override public void sessionDestroyed(final HttpSessionEvent event) { final HttpSession httpSession = event.getSession(); try { + // In case the session has been destroyed due to it expiring rather than an explicit + // logout, we need to stop the refresh manager from trying to refresh tokens. + UserIdentitySessionUtil.get(httpSession) + .ifPresent(stroomUserIdentityFactory::logoutUser); + final UserRef userRef = getUserRefFromSession(httpSession); final String userAgent = UserAgentSessionUtil.getUserAgent(httpSession); final Instant createTime = Instant.ofEpochMilli(httpSession.getCreationTime()); @@ -102,7 +117,7 @@ public void sessionDestroyed(final HttpSessionEvent event) { userRef, userAgent); } finally { - sessionMap.remove(httpSession.getId()); + sessionIdToSessionMap.remove(httpSession.getId()); } } @@ -131,49 +146,51 @@ public void sessionIdChanged(final HttpSessionEvent event, final String oldSessi // } public SessionListResponse listSessions(final String nodeName) { - LOGGER.debug("listSessions(\"{}\") called", nodeName); - // TODO add audit logging? - Objects.requireNonNull(nodeName); - - final SessionListResponse sessionList; - - if (NodeCallUtil.shouldExecuteLocally(nodeInfo, nodeName)) { - // This is our node so execute locally - sessionList = listSessionsOnThisNode(); - - } 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); - - try { - LOGGER.debug("Sending request to {} for node {}", url, nodeName); - WebTarget webTarget = webTargetFactory.create(url); - webTarget = UriBuilderUtil.addParam(webTarget, SessionResource.NODE_NAME_PARAM, nodeName); - try (final Response response = webTarget - .request(MediaType.APPLICATION_JSON) - .get()) { - - if (response.getStatus() != 200) { - throw new WebApplicationException(response); + return securityContext.secureResult(AppPermission.MANAGE_USERS_PERMISSION, () -> { + LOGGER.debug("listSessions(\"{}\") called", nodeName); + // TODO add audit logging? + Objects.requireNonNull(nodeName); + + final SessionListResponse sessionList; + + if (NodeCallUtil.shouldExecuteLocally(nodeInfo, nodeName)) { + // This is our node so execute locally + sessionList = listSessionsOnThisNode(); + + } 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); + + try { + LOGGER.debug("Sending request to {} for node {}", url, nodeName); + WebTarget webTarget = webTargetFactory.create(url); + webTarget = UriBuilderUtil.addParam(webTarget, SessionResource.NODE_NAME_PARAM, nodeName); + try (final Response response = webTarget + .request(MediaType.APPLICATION_JSON) + .get()) { + + if (response.getStatus() != 200) { + throw new WebApplicationException(response); + } + + sessionList = response.readEntity(SessionListResponse.class); } - - sessionList = response.readEntity(SessionListResponse.class); + } catch (final Throwable e) { + throw NodeCallUtil.handleExceptionsOnNodeCall(nodeName, url, e); } - } catch (final Throwable e) { - throw NodeCallUtil.handleExceptionsOnNodeCall(nodeName, url, e); } - } - LOGGER.debug(() -> LogUtil.message("Returning {} session items", sessionList.size())); - return sessionList; + LOGGER.debug(() -> LogUtil.message("Returning {} session items", sessionList.size())); + return sessionList; + }); } private SessionListResponse listSessionsOnThisNode() { final String thisNodeName = nodeInfo.getThisNodeName(); return LOGGER.logDurationIfDebugEnabled(() -> - sessionMap.values().stream() + sessionIdToSessionMap.values().stream() .map(httpSession -> { final UserRef userRef = getUserRefFromSession(httpSession); return new SessionDetails( @@ -195,36 +212,37 @@ private static UserRef getUserRefFromSession(final HttpSession httpSession) { } public SessionListResponse listSessions() { - return taskContextFactory.contextResult("Get session list on all active nodes", parentTaskContext -> - nodeService.findNodeNames(FindNodeCriteria.allEnabled()) - .stream() - .map(nodeName -> { - final Supplier listSessionsOnNodeTask = - taskContextFactory.childContextResult(parentTaskContext, - LogUtil.message("Get session list on node [{}]", nodeName), - taskContext -> - listSessions(nodeName)); - - LOGGER.debug("Creating async task for node {}", nodeName); - return CompletableFuture - .supplyAsync(listSessionsOnNodeTask) - .exceptionally(throwable -> { - LOGGER.error("Error getting session list for node [{}]: {}. " + - "Enable DEBUG for stacktrace", - nodeName, - throwable.getMessage()); - LOGGER.debug("Error getting session list for node [{}]", - nodeName, throwable); - // TODO do we want to silently ignore nodes that error? - // If we can't talk to one node we still want to see the results from the - // other nodes - return SessionListResponse.empty(); - }); - }) - .map(CompletableFuture::join) - .reduce(SessionListResponse.reducer( - SessionListResponse::new, - SessionDetails.class)) - .orElse(SessionListResponse.empty())).get(); + return securityContext.secureResult(AppPermission.MANAGE_USERS_PERMISSION, () -> + taskContextFactory.contextResult("Get session list on all active nodes", parentTaskContext -> + nodeService.findNodeNames(FindNodeCriteria.allEnabled()) + .stream() + .map(nodeName -> { + final Supplier listSessionsOnNodeTask = + taskContextFactory.childContextResult(parentTaskContext, + LogUtil.message("Get session list on node [{}]", nodeName), + taskContext -> + listSessions(nodeName)); + + LOGGER.debug("Creating async task for node {}", nodeName); + return CompletableFuture + .supplyAsync(listSessionsOnNodeTask) + .exceptionally(throwable -> { + LOGGER.error("Error getting session list for node [{}]: {}. " + + "Enable DEBUG for stacktrace", + nodeName, + throwable.getMessage()); + LOGGER.debug("Error getting session list for node [{}]", + nodeName, throwable); + // TODO do we want to silently ignore nodes that error? + // If we can't talk to one node we still want to see the + // results from the other nodes + return SessionListResponse.empty(); + }); + }) + .map(CompletableFuture::join) + .reduce(SessionListResponse.reducer( + SessionListResponse::new, + SessionDetails.class)) + .orElse(SessionListResponse.empty())).get()); } } diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionResourceImpl.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionResourceImpl.java index 404d6beae6b..58926576582 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionResourceImpl.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SessionResourceImpl.java @@ -2,60 +2,82 @@ import stroom.event.logging.rs.api.AutoLogged; import stroom.event.logging.rs.api.AutoLogged.OperationType; +import stroom.security.api.SecurityContext; +import stroom.security.api.UserIdentity; import stroom.security.common.impl.UserIdentitySessionUtil; import stroom.security.shared.SessionListResponse; import stroom.security.shared.SessionResource; import stroom.security.shared.UrlResponse; +import stroom.util.logging.LambdaLogger; +import stroom.util.logging.LambdaLoggerFactory; +import stroom.util.logging.LogUtil; +import stroom.util.servlet.SessionUtil; +import stroom.util.shared.NullSafe; import jakarta.inject.Inject; import jakarta.inject.Provider; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpSession; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @AutoLogged(OperationType.MANUALLY_LOGGED) class SessionResourceImpl implements SessionResource { - private static final Logger LOGGER = LoggerFactory.getLogger(SessionResourceImpl.class); + private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(SessionResourceImpl.class); private final Provider openIdManagerProvider; private final Provider httpServletRequestProvider; private final Provider authenticationEventLogProvider; private final Provider sessionListService; private final Provider stroomUserIdentityFactoryProvider; + private final Provider securityContextProvider; @Inject SessionResourceImpl(final Provider openIdManagerProvider, final Provider httpServletRequestProvider, final Provider authenticationEventLogProvider, final Provider sessionListService, - final Provider stroomUserIdentityFactoryProvider) { + final Provider stroomUserIdentityFactoryProvider, + final Provider securityContextProvider) { this.openIdManagerProvider = openIdManagerProvider; this.httpServletRequestProvider = httpServletRequestProvider; this.authenticationEventLogProvider = authenticationEventLogProvider; this.sessionListService = sessionListService; this.stroomUserIdentityFactoryProvider = stroomUserIdentityFactoryProvider; + this.securityContextProvider = securityContextProvider; } @Override @AutoLogged(OperationType.MANUALLY_LOGGED) public UrlResponse logout(final String redirectUri) { - LOGGER.debug("logout() - redirectUri: {}", redirectUri); final HttpServletRequest request = httpServletRequestProvider.get(); // Get the session. - final HttpSession session = request.getSession(false); + final HttpSession session = SessionUtil.getExistingSession(request); if (session != null) { - // Record the logoff event. - UserIdentitySessionUtil.get(session).ifPresent(userIdentity -> { + final UserIdentity userIdentity = UserIdentitySessionUtil.get(session) + .orElse(null); + LOGGER.info(() -> LogUtil.message( + "logout() - Logout called for {}, userIdentity: {} {} ({}), session: {}, redirectUri: {}", + securityContextProvider.get().getUserRef(), + NullSafe.get(userIdentity, UserIdentity::getSubjectId), + NullSafe.get(userIdentity, UserIdentity::getDisplayName), + NullSafe.get(userIdentity, identity -> identity.getFullName().orElse("-")), + SessionUtil.getSessionId(session), + redirectUri)); + if (userIdentity != null) { + // Record the logoff event. stroomUserIdentityFactoryProvider.get().logoutUser(userIdentity); // Create an event for logout authenticationEventLogProvider.get().logoff(userIdentity.getSubjectId()); - }); - - // Remove the user identity from the current session. - UserIdentitySessionUtil.set(session, null); + // Remove the user identity from the current session. + UserIdentitySessionUtil.set(session, null); + } + session.invalidate(); + } else { + LOGGER.info(() -> LogUtil.message( + "logout() - Logout called for {} but no active session, redirectUri: {}", + securityContextProvider.get().getUserRef(), + redirectUri)); } final String url = openIdManagerProvider.get().logout(redirectUri); diff --git a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java index 8d207adca23..e5499c58185 100644 --- a/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java +++ b/stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/StroomUserIdentityFactory.java @@ -25,6 +25,7 @@ import stroom.security.shared.User; import stroom.util.authentication.DefaultOpenIdCredentials; import stroom.util.authentication.HasRefreshable; +import stroom.util.authentication.Refreshable; import stroom.util.cert.CertificateExtractor; import stroom.util.entityevent.EntityAction; import stroom.util.entityevent.EntityEvent; @@ -56,6 +57,7 @@ import java.util.Objects; import java.util.Optional; import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Predicate; @@ -192,7 +194,16 @@ public Optional getApiUserIdentity(final HttpServletRequest reques public void logoutUser(final UserIdentity userIdentity) { LOGGER.debug("Logging out user {}", userIdentity); if (userIdentity instanceof final HasRefreshable hasRefreshable) { - removeTokenFromRefreshManager(hasRefreshable.getRefreshable()); + final Refreshable refreshable = hasRefreshable.getRefreshable(); + if (refreshable != null) { + try { + removeTokenFromRefreshManager(refreshable); + } catch (final Exception e) { + LOGGER.error("Error removing refreshable {} from refresh manager for userIdentity {}. {}", + refreshable, userIdentity, LogUtil.exceptionMessage(e), e); + // Swallow the error so the rest of the logout can proceed + } + } } } @@ -324,45 +335,53 @@ private UserIdentity createAuthFlowUserIdentity(final JwtClaims jwtClaims, // At this point we have authenticated the code-flow User so need to ensure // we have a session to store the userIdentity in. Also, the session gets attached // to the userIdentity object to allow us to refresh it's token. - final HttpSession session = SessionUtil.getOrCreateSession(request, newSession -> { - UserAgentSessionUtil.setUserAgentInSession(request, newSession); + final AtomicBoolean isNewSession = new AtomicBoolean(false); + final HttpSession session = SessionUtil.getOrCreateSession(request, newSession -> isNewSession.set(true)); + try { + UserAgentSessionUtil.setUserAgentInSession(request, session); + + // Make a token object that we can update as/when we do a token refresh + final UpdatableToken updatableToken = new UpdatableToken( + tokenResponse, + jwtClaims, + super::refreshUsingRefreshToken, + session); + + final UserIdentity userIdentity = new UserIdentityImpl( + user.getUuid(), + user.getSubjectId(), + user.getDisplayName(), + user.getFullName(), + session, + updatableToken); + + // We have authenticated so set the userIdentity in the session for future requests + // to retrieve it from + UserIdentitySessionUtil.set(session, userIdentity); + + // Register the token with the refresh manager so it gets refreshed periodically. + updatableToken.setUserIdentity(userIdentity); + addTokenToRefreshManager(updatableToken); + LOGGER.info(() -> LogUtil.message( - "createAuthFlowUserIdentity() - Created new session, sessionId: {}, userRef: {}, user-agent: {}", - newSession.getId(), - user.getUserRef(), - UserAgentSessionUtil.getUserAgent(newSession))); - }); - - // Make a token object that we can update as/when we do a token refresh - final UpdatableToken updatableToken = new UpdatableToken( - tokenResponse, - jwtClaims, - super::refreshUsingRefreshToken, - session); - - final UserIdentity userIdentity = new UserIdentityImpl( - user.getUuid(), - user.getSubjectId(), - user.getDisplayName(), - user.getFullName(), - session, - updatableToken); - - // We have authenticated so set the userIdentity in the session for future requests - // to retrieve it from - UserIdentitySessionUtil.set(session, userIdentity); - - updatableToken.setUserIdentity(userIdentity); - addTokenToRefreshManager(updatableToken); - - LOGGER.debug(() -> LogUtil.message( - "createAuthFlowUserIdentity() - Authenticated user - session: {}, userIdentity: {}", - SessionUtil.getSessionId(session), - userIdentity)); - - LOGGER.info(() -> "createAuthFlowUserIdentity() - Authenticated user " + userIdentity - + " for sessionId " + SessionUtil.getSessionId(session)); - return userIdentity; + "createAuthFlowUserIdentity() - Authenticated user: {} {} ({}), " + + "sessionId: {} ({}), user-agent: '{}'", + userIdentity.getSubjectId(), + userIdentity.getDisplayName(), + userIdentity.getFullName().orElse("-"), + SessionUtil.getSessionId(session), + (isNewSession.get() + ? "NEW" + : "EXISTING"), + UserAgentSessionUtil.getUserAgent(session))); + return userIdentity; + } catch (final Exception e) { + LOGGER.error(LogUtil.message( + "Error creating userIdentity for user {}. Session {} has been invalidated. {}", + user, SessionUtil.getSessionId(session), LogUtil.exceptionMessage(e)), e); + session.invalidate(); + throw e; + } } /** diff --git a/stroom-security/stroom-security-impl/src/test/java/stroom/security/impl/TestSessionListListener.java b/stroom-security/stroom-security-impl/src/test/java/stroom/security/impl/TestSessionListListener.java index 87111dd6240..9fe27b6382d 100644 --- a/stroom-security/stroom-security-impl/src/test/java/stroom/security/impl/TestSessionListListener.java +++ b/stroom-security/stroom-security-impl/src/test/java/stroom/security/impl/TestSessionListListener.java @@ -3,6 +3,7 @@ import stroom.node.api.FindNodeCriteria; import stroom.node.api.NodeInfo; import stroom.node.api.NodeService; +import stroom.security.mock.MockSecurityContext; import stroom.security.shared.SessionListResponse; import stroom.security.shared.SessionResource; import stroom.task.api.SimpleTaskContextFactory; @@ -30,6 +31,7 @@ class TestSessionListListener extends AbstractMultiNodeResourceTest sessionListService, - TestUtil.mockProvider(StroomUserIdentityFactory.class)); + TestUtil.mockProvider(StroomUserIdentityFactory.class), + MockSecurityContext::new); } } diff --git a/stroom-security/stroom-security-impl/src/test/java/stroom/security/impl/TestSessionResourceImpl.java b/stroom-security/stroom-security-impl/src/test/java/stroom/security/impl/TestSessionResourceImpl.java index a84710bde07..6126210d408 100644 --- a/stroom-security/stroom-security-impl/src/test/java/stroom/security/impl/TestSessionResourceImpl.java +++ b/stroom-security/stroom-security-impl/src/test/java/stroom/security/impl/TestSessionResourceImpl.java @@ -1,5 +1,6 @@ package stroom.security.impl; +import stroom.security.mock.MockSecurityContext; import stroom.security.shared.SessionDetails; import stroom.security.shared.SessionListResponse; import stroom.security.shared.SessionResource; @@ -31,7 +32,8 @@ public SessionResource getRestResource() { TestUtil.mockProvider(HttpServletRequest.class), TestUtil.mockProvider(AuthenticationEventLog.class), () -> sessionListService, - TestUtil.mockProvider(StroomUserIdentityFactory.class)); + TestUtil.mockProvider(StroomUserIdentityFactory.class), + MockSecurityContext::new); } diff --git a/stroom-test-common/src/main/java/stroom/test/common/TestUtil.java b/stroom-test-common/src/main/java/stroom/test/common/TestUtil.java index 51bcd517ee0..2d4771a6e61 100644 --- a/stroom-test-common/src/main/java/stroom/test/common/TestUtil.java +++ b/stroom-test-common/src/main/java/stroom/test/common/TestUtil.java @@ -48,7 +48,8 @@ private TestUtil() { } /** - * Build a {@link Provider} for a mocked class. + * Build a {@link Provider} that will provide a mock for the supplied class. + * Useful for constructors whose arguments are all providers. */ public static Provider mockProvider(final Class type) { return () -> Mockito.mock(type); diff --git a/stroom-util-shared/src/main/java/stroom/util/shared/IsServlet.java b/stroom-util-shared/src/main/java/stroom/util/shared/IsServlet.java index 538ef0e9813..0dd18e8173c 100644 --- a/stroom-util-shared/src/main/java/stroom/util/shared/IsServlet.java +++ b/stroom-util-shared/src/main/java/stroom/util/shared/IsServlet.java @@ -9,4 +9,12 @@ public interface IsServlet { * e.g. "/datafeed". */ Set getPathSpecs(); + + /** + * @return The name to use when registering this servlet. If not implemented, returns the simple name + * of this class. + */ + default String getName() { + return getClass().getSimpleName(); + } } diff --git a/stroom-util-shared/src/main/java/stroom/util/shared/ResourceKey.java b/stroom-util-shared/src/main/java/stroom/util/shared/ResourceKey.java index 82d8ab2b911..2bb4409cc64 100644 --- a/stroom-util-shared/src/main/java/stroom/util/shared/ResourceKey.java +++ b/stroom-util-shared/src/main/java/stroom/util/shared/ResourceKey.java @@ -47,6 +47,15 @@ public ResourceKey(@JsonProperty("key") final String key, this.name = name; } + /** + * Creates a {@link ResourceKey} containing on the key UUID. + * Only for use as a map key as equals/hashcode only use key. + */ + public static ResourceKey createSearchKey(final String key) { + // name is not used in the equals/hash + return new ResourceKey(key, null); + } + public ResourceKey(final Map map) { this.name = map.get(NAME); this.key = map.get(KEY); diff --git a/stroom-util-shared/src/main/java/stroom/util/shared/ResourcePaths.java b/stroom-util-shared/src/main/java/stroom/util/shared/ResourcePaths.java index b7c8b03e9a0..923ee0e2a7f 100644 --- a/stroom-util-shared/src/main/java/stroom/util/shared/ResourcePaths.java +++ b/stroom-util-shared/src/main/java/stroom/util/shared/ResourcePaths.java @@ -58,6 +58,10 @@ public interface ResourcePaths { String V2 = "/v2"; String V3 = "/v3"; + String UI_SERVLET_NAME = "UI"; + String STROOM_SERVLET_NAME = "StroomServlet"; + String SIGN_IN_SERVLET_NAME = "SignInServlet"; + static String addLegacyUnauthenticatedServletPrefix(final String... parts) { return new Builder() diff --git a/unreleased_changes/20250912_165642_710__0.md b/unreleased_changes/20250912_165642_710__0.md new file mode 100644 index 00000000000..3a1c107a631 --- /dev/null +++ b/unreleased_changes/20250912_165642_710__0.md @@ -0,0 +1,55 @@ +* Change the resource store to not rely on sessions. Resources are now linked to a user. + + +```sh +# 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. + + +# -------------------------------------------------------------------------------- +# The following is random text to make this file unique for git's change detection +# 9cxWGxuRTsSyVkWGamx2bzZgiuYeUNyJc2caPsUqkVG0PnoBRoNUeqTg9a9YyJ9fDu35CSiymCQ8LyIK +# YdL8fvAHAIjncu1K9y0MBYZk2EoIvZ3aJ2I7vg3xUCJe2EXaGErO1bOBrMMxN9y4AHnMgzJytLw8sBQY +# F8Z3sj68CVsGAMAZdFpL1Ej8ZB5i7nAorscpfwZG73u5A4wqkG83OqG3Jdlj1BwuXwgcslSwTM6qRTAe +# 8x6TX7jrLIroA8mdxX0JvHI4axRuag9h9l1VCuhSH3RfxMM6MEsi5ePPh66W3nvzANfm2xLZ8gAfAkjb +# pz6TiKK1ivZcH4H37LPs9gYebL1zzBXDpnVaHu90sSjq1F2yCH7fCxkLfzRb6tDUiC1DFJw9Ec3QStuc +# P99IANfwgb9JPixIhyzDr4ScmtaN5okVCISDmieMsonJJxhXZ8wRZajgcGbioLuqUYxmrHeiWLmTaian +# gBbUFdJqyQ1urCGjEJSqxBjZlHTjwEqbcBbGr54nAlxZ8e3Qf6PSTxPfHoRgpusJlrmHDLU2tdyQGMru +# oM4u27cxW7bF9RtI7FT9uwyfRmyNSxUewokfTp238MikkL5PiXOkIftHhhjdHwgpTnpOKOkMJQo11n9W +# nagDaWcTDZ09RiAWVP97IhZkgzJ2gzalSb5fs11A5G4i7DfhKNYzQqnZTG1MIrjrNtWqKrB1eqBcRO7x +# d3yHTxDBqO21jkwsSzRn0eVZ9FgOkBK5YiAEigIG3jayzSn1caUDmSB0PS5RccKxvGY4RTNZfANjQvmQ +# SkfOUHFh1H5WJTP81j22Y2RWvGr1ZNVdKDnfFTbKiJ1DBVLNGqTXHAI8Y569QJHCFUbMnWI6ll5xVxWM +# KOzJGhDgN6gAxTbM9KfwXZ102RWjXuMr17qpjCT0txHhC7BsQnwZBnKDS2uJ97WidhebV1Nkc56g8fjp +# j9eNbCJ1tusy2htSO8BiAY6oQmz7h1FPsAH9W53SexHkUTN4THC3jyHYYykihg0RjXbJBn0xFZ2v4rYm +# YGjgqbm8NAxjDrwNahBSUbLzPaoXiorwNRseseXBjU5rTXrirHi1JfUYsI7Ai8Pg4GJiuRkQ1G9H4OwF +# ldR0HPahXpXzTIIm7m8vrvuDmbnzNcUHLeH9aFgKG23oHPazZb0hLVKfVffLEHmuW71pq0JowS9OI92I +# 0CmzKqq1SWVUB61IHeDNfUICnNZrsDvqlIphkw9fRpzc8lPFETp4TAY78OzOLCcbD3ea4r6jvzme6TkU +# ZV8sGHITkb16GYyOlagdcuyYXDKxvHCSKmog9uS43xsui45M1EtNxY88wLgdHw3UNq9kwEMRYZNfgC2w +# T3tBgEbUca30POHCZ7xlmW51bEr7sHuYqFanjPbgulmNFIe1R5cPZobjs8IRv4movVl6fdx4Eg0ul94z +# gQFAYUQlJaalUDOwt1SbmvdaZVTigqbTLTXts0TtIUhTpGhTW4YmU3DIP6b640L4aZ0ZvV7ZjswRXVPD +# Aoe9wPjXan4Zti4aG4soem0BxEQ5QM4r3i16CriwYiLfoR4A7T98HoL3KUj2ExYbtvS0SmNO1H9YRs7q +# xmc7MFOeWnaD5KnL8QR2A2cjJ6PgtG4Or7JwvjVpfbAWgqJfSJfUPFvbH840CoxvmbugBLFFRXuauUZg +# XDzdcSNIzn5h5zQqtIhVTvpiAJcchr8S0GldcWiyXHd9jS5EiRDl1wQEkjgph1MP3wySNh7dokX7vUwK +# ZaLFtCPLbzPY1FFjIcBPodK1WOuUr3DIxg7eUy48ptoNzVN9POQ34if3T7mvkELaFWb1RV1zy6arB5Qe +# d8Jm4dZ7Vgvjohpc4G73LQ3tznm4BGKcQxkJ1stKZsmDkHsbHl7sUaZ9ncHnk1mcLRpDfKosliiMXtgl +# OHNEePBuqFgbzYB3Vrj0aJhB6yEUW4MsvGRdKsFHjphoQ0PtLgarTJPrBrPCXZV3Gmh3ObPypYqPsLvX +# FnP99gAGqcMCsO1ReJ0qjk3TGViJHGul4yPQMCV24P46BkUzGV7Z6vRi5UROpwKoYJvffIGNP3PFFxQt +# 44jLdp6LQUsC0pVs9n2QXynxdBAJ12sskIJVt5VBOEhENUK989M1b66vS2GpNXPO9mMpFsCpXbD0KfBt +# Lp5JRWLfoM4pRDr6vo70IYTORjSJkKNbP7jgyRgxNnE432KInr6goMgLAWyBOFUR2pDInU8rzKLSZDz4 +# YXV27gj6139pTTQlXOcLxiEMkOX8FbG5OWoJv2DLyJ69kfT0MeH5rCB7ajmo3iywCUnqNMwBUXVHYnGJ +# 8bJFZOEP0BnB4bw0eHmOTxWgZ9J98pQhWs0kFuLeEBfJxjs5ZjUTpNLtYjkhpZTTUPiiQVsYdYlNFZ2z +# -------------------------------------------------------------------------------- + +``` diff --git a/unreleased_changes/20250912_165749_707__5114.md b/unreleased_changes/20250912_165749_707__5114.md new file mode 100644 index 00000000000..e3a003da016 --- /dev/null +++ b/unreleased_changes/20250912_165749_707__5114.md @@ -0,0 +1,60 @@ +* Issue **#5114** : Improve handling of loss of connection to IDP. + + +```sh +# ******************************************************************************** +# Issue title: Stroom not recovering if the IDP goes down +# Issue link: https://github.com/gchq/stroom/issues/5114 +# ******************************************************************************** + +# 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. + + +# -------------------------------------------------------------------------------- +# The following is random text to make this file unique for git's change detection +# Jdou9bzmobp3NcUYsTXiTYZl0pViZ1LQm6ImeXu5IkmhqKWpeUruAItRn152vtDoVP3x4nkCz5qaGgvT +# VkAZCJeps33YfsM28LlzlNdSwKnrmH0YZOIJKtu6bqqDLjkyGAQleLGQhrYGJEw3e7hsdka0rKqW6zzS +# eczPwEqdQ9tQVGc0BwI55SR2KK8D0RijvyoMb3usIt81LSHcsyI5TAVaLL7gnbwHjy3K4TjHTFozZP11 +# 19JKKFmhS7H7r2UDN2NiYL4B2WXhgQuW5IYZzaKMmJcdlEAPHt9oEid5l8SneQRYFfFhw4gM4dtRStwU +# sC1ewBB5ejKqK7U0c4SbyR2LLmUX1CidkEDeF1sIsnlWvQ538MVp54jKdom3hN1qtUqviHjZXw0t0LG6 +# gwUCrVnHhLIC2q1Br21EBTfl4ztiBOr1jY8zRokbQCi8nRefh4kWpmRX8uuPNdY3K6jzLtK3YzhMmOd1 +# 3kgxltNMsYMntT1fna6OKCgnHInUOaoavc7I3yPiDuT9y8i1I00FQ556Uz3ATIs6jhDxmMQMmEsP90MT +# niy2yBWC5cPr5gKA99iQPHfPrzgtZ7Yd79saIClOmJRr0IyfRA9p7aHHVezuCIWhXwsJSGPxH2Hs4PmR +# jc8wsERGXqGRWkRNTR49vdAEtrwvoctdpwKnq8jjQs7cnPGkXH77GzKaImkB3sdKydBhHd3eiRDfGGmH +# PfxfCwT0zwvQhkMKtP15XAi0OINisv6dVHlAUypdqN7sLbhjD0MjLEsyFV90KyJPSQ8BjryEs1kdbAYY +# fcD40W48tXxzwuTafOD5YrKKH5aO66wBJ3ySLkTUQUG0O5kXzl06Qsg0ap5FMDm0aOgqChZfsajNCStC +# GqPPwjZ4XkgAmgRci926BU9uz6Z22TFFDB75yao2zOYTdImR47RewcDJv0Qtrym0djF24NQrdMPPjQxl +# OtZ983K0qJKZVNMrMJy0ScctvgQjI9xBJgght2IZeIGc31OCGirrjjdnjVJSL4KyOMOtBfjnglbhYkKm +# GS2h9G77XvU2nhdobTtP1QwiyePOj4a73HUb9zt3NJboIAe8gt06f35cWtZMNNSIIc3x5YhfgAog37ma +# HP5vz11sU6Pqs8Gc7ACZn3agbmgjF042kHU5rxoF0fCn21usXWj9390xI6BxqDz1yGqy05pHhtTX3oFU +# eAlJoujbFP58Zs3z6zAeBSx0wX9FsBXqPiDkAYDycgA7XpQJRaojqBotWVnsLZ3u7CRVIrJMdJDjOWZW +# q8ZU7kCeZRp099ddxn4q7rqrEV8vZnNN15Yg1ImTtfp572DW2e7FppTxog2wvo0RRs5mI9pq1M76SBj3 +# 7CvFV9y8oUy7iJ4apkvPtpaPoSNIpllgUvXlQkKe4Ub4NYmOLuxN5Evyk5WQ2McENiIoTaxa62f80qAU +# hMnWU4Ti21CnIMaUKmsVFnYSEYfIz3WqxVxVMDWfi9CeIdfIUhxkdWnCtEKv2eonuMXHPP5RzV7q4Nca +# 1qSRCI5C2Gt1psN0H7MR8ivaKX3yIlcg3OaNb1Ar52WA6y1txr5YIo9EPybNIrNrSFVeOfTTUmMRcpGw +# taB4U8Vp8SCooWZwCcgdgJJY5REaKyp02aZiAvKU7KiEo53iUU0wg43brUPp8vTX6UyfbC3CC6sIipC0 +# t3eIVGYKUvlFX8Y7a7ordHJMZ0Ie1KwwWbpQmIFBVl3g6g6H9CcuSNIUivLJeIYj9OLTPO5B6E8tU9g5 +# TKi0zyfKGQEaYCrwgAW72mpTgsFQ3ekZDxdaoK31Hhqev27ubvrpRzfpMJ9rK1c0NznltcWg6Frbp90g +# pMuYb6iK9Dq2Dku0ZlkWqSPNx3HU7c5kdoJd8v4a2XTZURpuEeNjcqYfXG2tIuRnl5hNIjITbcN4469G +# 8nqASfCV9GgcN8dS8NAzoRdFF4gWExB2zcWC9OnjFFl3jqpUDI3BwXygUxCK5s3L4FF3lspOqYCsKDMu +# Maiihhpc47i152WP24D2JXNGVWXT5lIGstNxbTCe6sZ2KG5bmFdK9C64FhUKvgYNZY31uGQn5tlnnMHC +# uJqTBwRQSEQMT7faU04VKNScBW3IBXIXPcN0N32KPUJ7GAJ4vjpNRbD3lmuyHTvopB6I5i35TfL94ZPK +# EXkQw1kdL5Bo65w910sZdPvmsDuHgmEWdKfMmu9w9gRexUCPN0VFWfqPDo4xPYxZRgqhK3A5VB9IrifF +# ElJGkUChZTUp2Z3pYYDSCNU1vBeTXHIm7wglsKfcGo7lII9Ey2Efob16RhnUTjU5LDsp9Gx7zY3dOTjI +# oudp0xgK9o2wVwM59wlkKKp9PYIuUs3rBa6c0g71InLHq0mIkYOCh2STwWHQhS2mtk4OvSypcuJMfJZc +# -------------------------------------------------------------------------------- + +``` diff --git a/unreleased_changes/20250912_165825_362__0.md b/unreleased_changes/20250912_165825_362__0.md new file mode 100644 index 00000000000..83611dcec79 --- /dev/null +++ b/unreleased_changes/20250912_165825_362__0.md @@ -0,0 +1,55 @@ +* Change the way security filter decides whether to authenticate or not, e.g. how it determines what is a static resource that does not need authentication. + + +```sh +# 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. + + +# -------------------------------------------------------------------------------- +# The following is random text to make this file unique for git's change detection +# J0RQnBJi65ECKnjEvhS9TNs6mj5B1t59epuyeVGJxatof4XjWlREaU2YAvyYMCLfkd8mC4klba0jChHT +# 6MqfiTx5CTHh3oo2XUTV7OJwA2ROhdqn5SqC0PSpMue675EG0wQA5LpL1jBvo6axV390ZCpfc4rMdL5t +# 8LHRBl5ivdgwrLTOXYH6VCk7BIGVoJz8hgWGoFh1nF6T3tXoDojthFIpu32IqA4n8yC45xqX5jhKX8fO +# UAsoRzu5tnPThLjIfEDx23Zxkxe3tMLjnzF6pLIXshfgmz4AbbjzCIolVK70WbLBWGwVWjtKmhWOVKin +# cYFP11PHZpUdh7dA4kIfDiK42nH3RBj3JCUSsSemhSF2P5K94FVExnMa0yHtnouVrGdfgyiX2R6QKTa6 +# wWdOwH4Cjat3KSFtMSSPW0tSTDZ9w68oWVlrFgdpxvmpy6M3jW4LwBN1m1yGxRZK2qcwGg86EwBrymWY +# fXncHzXqoEaBvd2WDydQgJO35hubmexr7AIu4xwO95uX25eJqdSwuXJiqZsoQffIcel75pgO7oCvfS3H +# 4glGlKw34ONOAqNGq3IZZ9pRg6O9FQlApnGHqYox32BJtMs8bbZinHrUgv0x3yXMNFawYtlWeJLnSvRj +# Ruf0CnyGwq8sJT07Zof9JHYG3OFTCywtnneAKUhJVUfGi7LInPG4FFbRi4ErwMQwlI8f4UPMdTXrcHHW +# PSxXDz8S0jsTmHRsYrTwNsLxTsJFYLbzdtVEhp7LkGJIvotWq9ODwRS0Gz12eLGIrgHvlyYt2kl5Jr52 +# BIvWdCZj83hmMcNPmp2pVOuCjDZIhhfp6Z1tZ4EjZggUanbJ54C8IEZIrdCYBxKgW9rI3BobOov1L6z2 +# 4VqnDwOseLyNlytG2LPJEuA6BHlxLhhkkvq92PAq7ECSpMqnmUNolyjnOiGQZMEd5wlqNAvbHcbU1dgg +# VwDNqYoVoJKakuZHeCtee0nlqc9hdgNWWQOpYhjHCOyX1Jck3U1G9j6p6vyYYwZWqPIYWqEeTCDCumrb +# HH1pJ9d8aWrnIbIOsv0KuzFSipObFVq2CslATATJDy47DqxgqXQbPJUlJq0ahWOMAty8HS8TWHIO7dFx +# 8LdUracMS5NGBk0SdE3OoxFzXRPWDWSNVFmfR4Gcjm98oVMFkyawjPD2PSK8osmLcx0Lx8QAfl5NTzpg +# sRAIfDW07qyT53EN5QxN2Q52SwuVqrsBTIA69ZfD78AfjwlbRA0EHelIqrJxYamTq5HjwHotplcbglKn +# XFeHslhA0ZiH4HOcfXIIVDE6r6Q9iUXDC4jEcFblHMyLcVFOmGUHpCxqkn8ocF6hI51x0HtlgD822A0i +# LUF8Xya7L2peHDcp13qTAF39P01iPrDQfWKPQ8ypSbLcDjFbSr7AglEFRxghDsaTfR9GmzLCagQs4i0V +# CM6kF11RwRZ0BteiznfiY85ntJeDO0dd9UHgKheQ6gSXiF8n3QIYrmXAaQiR9OjFHQAjRBLPFNrly86m +# sDcqQS23zmhLhTC6z99a5RzCUAUbRrDY7v0lcEwNkVEmByfONWxQX3iR2aUxul68zIa5O9QrVzYHza4t +# v1LvS5HB0YAPuisE3PqAs4EjEMjIIvLIPSxARa3HQvspqUEUpPiwlSH11fBHzmvCED8RQLyVO6LzdqGX +# SKYMdSpYdIe4X4mXB0rzYiG3Mshg1hpeNDJFoHZRlz3rGzSCdw37WpdRjQ42knIxsPreiUFJj3S1Vq6e +# V25RX3FAWOxQ4qPsopVqGp8wo2GmOrmtFPSOoxsYK8m9sTp09Yq50t5SAoDzCP2xwmigQ39gMcgRKUKe +# 5HENM71ppcNWLkEG1uxIKT2B422TjB8QxqPiTYfVTLmljIzy8KT9VULUWkjLJ90H6Au1WPmYzyOoxiBk +# KabNy799rZIgUnWm2luOmB1ZMgQJ3A9yMQCuJJPzxe6Ftbg8HMTX3eFflqIGdGyVRPCeWCzBrYSNu4qc +# Xmhb3Qsy4lu3CcC4wuaOlDHAnOY3PTO8hqZIvyH4h58qnE0FGbJBiKBLsJll9EfRhCctd8NhrMo0pN5D +# PAsB8e2Thh9lVYWhVnd3TrKtY4kzBYhcRFcKv5Fg0dKUEfTbvxDh73cRkBr1enyr9AmUPI8GiPKbl1AZ +# EvdSFMKHzzwfESuNlpbkuPXKjcvoSDIHMJaDc5YkuoVZaGOfMpytFRuCIluKP52UFuLdU0A61iyxU4bM +# m1Hr1B5My8088yGmAN4gYD1R7qL7tTuFbv7bg9kTpSmVA4hIUIxx6NQrKtgOhBQob3ToYkZFYoNkZui5 +# ww6BJPxC8Z4OHjCC26rHmzGfhwVj4R7CsYbChmJ1JVvmQYCChGphgSlw1Gew3I4pnVdVOIrIf1v88ODZ +# -------------------------------------------------------------------------------- + +``` From d60e1721b6e4da4df8da3dd39deef1faef6a2d30 Mon Sep 17 00:00:00 2001 From: at055612 <22818309+at055612@users.noreply.github.com> Date: Mon, 15 Sep 2025 17:15:21 +0100 Subject: [PATCH 06/10] Fix failing test --- .../TestImportExportDashboards.java | 25 +++++++++++-------- .../src/test/resources/logback-test.xml | 2 ++ .../resource/impl/UserResourceStoreImpl.java | 1 + .../stroom-security-mock/build.gradle | 1 + .../mock/MockUserSecurityContextModule.java | 7 ++++++ 5 files changed, 26 insertions(+), 10 deletions(-) diff --git a/stroom-app/src/test/java/stroom/importexport/TestImportExportDashboards.java b/stroom-app/src/test/java/stroom/importexport/TestImportExportDashboards.java index 867b927b1fe..b51abc53631 100644 --- a/stroom-app/src/test/java/stroom/importexport/TestImportExportDashboards.java +++ b/stroom-app/src/test/java/stroom/importexport/TestImportExportDashboards.java @@ -45,16 +45,16 @@ import stroom.query.api.ExpressionOperator; import stroom.query.api.ExpressionTerm; import stroom.query.api.ExpressionTerm.Condition; -import stroom.resource.api.ResourceStore; import stroom.script.shared.ScriptDoc; import stroom.test.AbstractCoreIntegrationTest; import stroom.test.CommonTestControl; -import stroom.util.shared.ResourceKey; import stroom.visualisation.shared.VisualisationDoc; import jakarta.inject.Inject; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import java.nio.file.Path; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -67,8 +67,6 @@ class TestImportExportDashboards extends AbstractCoreIntegrationTest { @Inject private ImportExportService importExportService; @Inject - private ResourceStore resourceStore; - @Inject private PipelineStore pipelineStore; @Inject private VisualisationStore visualisationStore; @@ -87,6 +85,13 @@ class TestImportExportDashboards extends AbstractCoreIntegrationTest { @Inject private ExplorerNodeService explorerNodeService; + @TempDir + private Path tempDir; + + private Path createTempFile(final String filename) { + return tempDir.resolve(filename); + } + @Test void testComplete() { test(false, false, false); @@ -243,7 +248,7 @@ private void test(final boolean skipVisCreation, final boolean skipVisExport, fi final int startDictionarySize = dictionaryStore.list().size(); final int startDashboardSize = dashboardStore.list().size(); - final ResourceKey file = resourceStore.createTempFile("Export.zip"); + final Path file = createTempFile("Export.zip"); final Set docRefs = new HashSet<>(); docRefs.add(folder1.getDocRef()); if (!skipVisExport) { @@ -254,11 +259,11 @@ private void test(final boolean skipVisCreation, final boolean skipVisExport, fi } // Export all - importExportService.exportConfig(docRefs, resourceStore.getTempFile(file)); + importExportService.exportConfig(docRefs, file); - final ResourceKey exportConfig = resourceStore.createTempFile("ExportPlain.zip"); + final Path exportConfigFile = createTempFile("ExportPlain.zip"); - importExportService.exportConfig(docRefs, resourceStore.getTempFile(exportConfig)); + importExportService.exportConfig(docRefs, exportConfigFile); if (!update) { // Delete everything. @@ -267,14 +272,14 @@ private void test(final boolean skipVisCreation, final boolean skipVisExport, fi // Import All final List confirmations = importExportService.importConfig( - resourceStore.getTempFile(file), + file, ImportSettings.createConfirmation(), new ArrayList<>()); for (final ImportState confirmation : confirmations) { confirmation.setAction(true); } importExportService.importConfig( - resourceStore.getTempFile(file), + file, ImportSettings.actionConfirmation(), confirmations); diff --git a/stroom-app/src/test/resources/logback-test.xml b/stroom-app/src/test/resources/logback-test.xml index a356e1dc120..8318dfc14a8 100644 --- a/stroom-app/src/test/resources/logback-test.xml +++ b/stroom-app/src/test/resources/logback-test.xml @@ -22,6 +22,8 @@ + + diff --git a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/UserResourceStoreImpl.java b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/UserResourceStoreImpl.java index fd0b3679f4f..ade7682a1a3 100644 --- a/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/UserResourceStoreImpl.java +++ b/stroom-resource/stroom-resource-impl/src/main/java/stroom/resource/impl/UserResourceStoreImpl.java @@ -341,6 +341,7 @@ private record UserResourceKey(UserRef userRef, ResourceKey resourceKey) { public String toString() { return "UserResourceKey{" + "userRef=" + userRef + + ", userUuid=" + userRef.getUuid() + ", key=" + resourceKey.getKey() + ", name=" + resourceKey.getName() + '}'; diff --git a/stroom-security/stroom-security-mock/build.gradle b/stroom-security/stroom-security-mock/build.gradle index 8054fa5ded9..787c2acbf67 100644 --- a/stroom-security/stroom-security-mock/build.gradle +++ b/stroom-security/stroom-security-mock/build.gradle @@ -14,5 +14,6 @@ dependencies { implementation libs.jakarta.servlet.api implementation libs.guice implementation libs.mockito.core + implementation libs.slf4j.api implementation libs.ws.rs.api } diff --git a/stroom-security/stroom-security-mock/src/main/java/stroom/security/mock/MockUserSecurityContextModule.java b/stroom-security/stroom-security-mock/src/main/java/stroom/security/mock/MockUserSecurityContextModule.java index b49fca91cf4..6e9004fb2ef 100644 --- a/stroom-security/stroom-security-mock/src/main/java/stroom/security/mock/MockUserSecurityContextModule.java +++ b/stroom-security/stroom-security-mock/src/main/java/stroom/security/mock/MockUserSecurityContextModule.java @@ -21,6 +21,9 @@ import stroom.security.impl.UserDao; import stroom.security.shared.AppPermission; import stroom.security.shared.User; +import stroom.util.logging.LambdaLogger; +import stroom.util.logging.LambdaLoggerFactory; +import stroom.util.logging.LogUtil; import stroom.util.shared.UserRef; import com.google.inject.AbstractModule; @@ -35,6 +38,8 @@ */ public class MockUserSecurityContextModule extends AbstractModule { + private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(MockUserSecurityContextModule.class); + private static final String ADMINISTRATORS = "Administrators"; private static final String ADMIN = "admin"; @@ -77,6 +82,8 @@ public UserRef getUserRef() { user.setUpdateTimeMs(System.currentTimeMillis()); final User created = userDao.create(user); userDao.addUserToGroup(created.getUuid(), group.getUuid()); + LOGGER.info(() -> LogUtil.message("Created user with subjectId: {}, UUID: {}", + user.getSubjectId(), user.getUuid())); return created; }); return persisted.asRef(); From 3abd16542a1e7ba55120499cf06fb2e90f9bdcda Mon Sep 17 00:00:00 2001 From: at055612 <22818309+at055612@users.noreply.github.com> Date: Mon, 15 Sep 2025 17:19:06 +0100 Subject: [PATCH 07/10] Fix failing test --- .../TestImportExportServiceImpl.java | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/stroom-app/src/test/java/stroom/importexport/TestImportExportServiceImpl.java b/stroom-app/src/test/java/stroom/importexport/TestImportExportServiceImpl.java index 1b86dd964fe..bdd4d95e79e 100644 --- a/stroom-app/src/test/java/stroom/importexport/TestImportExportServiceImpl.java +++ b/stroom-app/src/test/java/stroom/importexport/TestImportExportServiceImpl.java @@ -30,14 +30,15 @@ import stroom.importexport.shared.ImportState; import stroom.pipeline.PipelineStore; import stroom.pipeline.shared.PipelineDoc; -import stroom.resource.api.ResourceStore; import stroom.test.AbstractCoreIntegrationTest; import stroom.test.common.util.test.FileSystemTestUtil; import stroom.util.shared.ResourceKey; import jakarta.inject.Inject; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import java.nio.file.Path; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -50,8 +51,6 @@ class TestImportExportServiceImpl extends AbstractCoreIntegrationTest { @Inject private ImportExportService importExportService; @Inject - private ResourceStore resourceStore; - @Inject private PipelineStore pipelineStore; @Inject private FeedStore feedStore; @@ -60,6 +59,13 @@ class TestImportExportServiceImpl extends AbstractCoreIntegrationTest { @Inject private ExplorerNodeService explorerNodeService; + @TempDir + private Path tempDir; + + private Path createTempFile(final String filename) { + return tempDir.resolve(filename); + } + @Test void testExport() { final ExplorerNode systemNode = explorerNodeService.getRoot(); @@ -136,17 +142,17 @@ void testExport() { final int startTranslationSize = pipelineStore.list().size(); final int startFeedSize = feedStore.list().size(); - final ResourceKey file = resourceStore.createTempFile("Export.zip"); + final Path file = createTempFile("Export.zip"); final Set docRefs = new HashSet<>(); docRefs.add(folder1.getDocRef()); docRefs.add(folder2.getDocRef()); // Export - importExportService.exportConfig(docRefs, resourceStore.getTempFile(file)); + importExportService.exportConfig(docRefs, file); - final ResourceKey exportConfig = resourceStore.createTempFile("ExportPlain.zip"); + final Path exportConfig = createTempFile("ExportPlain.zip"); - importExportService.exportConfig(docRefs, resourceStore.getTempFile(exportConfig)); + importExportService.exportConfig(docRefs, exportConfig); // Delete it and check pipelineStore.deleteDocument(tran2.asDocRef()); @@ -157,7 +163,7 @@ void testExport() { // Import final List confirmations = importExportService.importConfig( - resourceStore.getTempFile(file), + file, ImportSettings.createConfirmation(), new ArrayList<>()); @@ -166,7 +172,7 @@ void testExport() { } importExportService.importConfig( - resourceStore.getTempFile(file), + file, ImportSettings.actionConfirmation(), confirmations); From 8e17b326503ac9048eebb499df74d790aa8d0db2 Mon Sep 17 00:00:00 2001 From: at055612 <22818309+at055612@users.noreply.github.com> Date: Mon, 15 Sep 2025 17:21:52 +0100 Subject: [PATCH 08/10] Update javadoc --- .../java/stroom/resource/api/ResourceStore.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/stroom-resource/stroom-resource-api/src/main/java/stroom/resource/api/ResourceStore.java b/stroom-resource/stroom-resource-api/src/main/java/stroom/resource/api/ResourceStore.java index 28f905a8413..c86617cf8b3 100644 --- a/stroom-resource/stroom-resource-api/src/main/java/stroom/resource/api/ResourceStore.java +++ b/stroom-resource/stroom-resource-api/src/main/java/stroom/resource/api/ResourceStore.java @@ -21,8 +21,12 @@ import java.nio.file.Path; /** - * API to a store of generated resources. This store only last 1 hour so it - * should be used for temp files (e.g. GUI generated temp files + * API to a store of generated resources. This store only last 1 hour, so it + * should be used for temp files (e.g. GUI generated temp files). + *

+ * Created resources are specific to the logged in user, so a resource + * created by one user cannot be accessed by another user. + *

*/ public interface ResourceStore { @@ -32,16 +36,17 @@ public interface ResourceStore { * Create a temporary file and give it a UUID. * * @param name The name of the file resource. Held mostly for logging/debug purposes. + * Does not have to be unique. */ ResourceKey createTempFile(String name); /** - * Get a temporary file + * Get the temporary file associated with a {@link ResourceKey} */ Path getTempFile(ResourceKey key); /** - * Delete a temporary file. + * Delete the temporary file associated with a {@link ResourceKey} */ void deleteTempFile(ResourceKey key); } From 6eff9091995839d031d719a27fa572dd2e96cab7 Mon Sep 17 00:00:00 2001 From: at055612 <22818309+at055612@users.noreply.github.com> Date: Mon, 15 Sep 2025 17:23:20 +0100 Subject: [PATCH 09/10] Fix compile error --- .../stroom/importexport/TestImportExportServiceImpl.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/stroom-app/src/test/java/stroom/importexport/TestImportExportServiceImpl.java b/stroom-app/src/test/java/stroom/importexport/TestImportExportServiceImpl.java index bdd4d95e79e..7b9bf1d2c8b 100644 --- a/stroom-app/src/test/java/stroom/importexport/TestImportExportServiceImpl.java +++ b/stroom-app/src/test/java/stroom/importexport/TestImportExportServiceImpl.java @@ -32,7 +32,6 @@ import stroom.pipeline.shared.PipelineDoc; import stroom.test.AbstractCoreIntegrationTest; import stroom.test.common.util.test.FileSystemTestUtil; -import stroom.util.shared.ResourceKey; import jakarta.inject.Inject; import org.junit.jupiter.api.Test; @@ -179,11 +178,11 @@ void testExport() { assertThat(feedStore.list().size()).isEqualTo(startFeedSize); assertThat(pipelineStore.list().size()).isEqualTo(startTranslationSize); - final ResourceKey fileChild = resourceStore.createTempFile("ExportChild.zip"); + final Path fileChild = createTempFile("ExportChild.zip"); final Set criteriaChild = new HashSet<>(); criteriaChild.add(folder2child2.getDocRef()); // Export - importExportService.exportConfig(criteriaChild, resourceStore.getTempFile(fileChild)); + importExportService.exportConfig(criteriaChild, fileChild); } } From b01b1f0485adb83bcdd663417c9d1a686b3df137 Mon Sep 17 00:00:00 2001 From: at055612 <22818309+at055612@users.noreply.github.com> Date: Wed, 17 Sep 2025 18:27:12 +0100 Subject: [PATCH 10/10] Fix DistributedTaskFetcher ex handling, add debug logging --- .../cluster/impl/ClusterNodeManagerImpl.java | 11 +++- .../impl/db/ClusterLockClusterHandler.java | 15 ++++- .../lock/impl/db/ClusterLockServiceImpl.java | 5 ++ .../job/impl/DistributedTaskFetcher.java | 26 +++++++-- .../java/stroom/job/impl/JobSystemModule.java | 4 +- .../job/impl/ScheduledTaskExecutor.java | 18 +++++- .../util/shared/scheduler/Schedule.java | 5 +- unreleased_changes/20250917_172638_617__0.md | 55 +++++++++++++++++++ 8 files changed, 123 insertions(+), 16 deletions(-) create mode 100644 unreleased_changes/20250917_172638_617__0.md diff --git a/stroom-cluster/stroom-cluster-impl/src/main/java/stroom/cluster/impl/ClusterNodeManagerImpl.java b/stroom-cluster/stroom-cluster-impl/src/main/java/stroom/cluster/impl/ClusterNodeManagerImpl.java index b9bd32ddbda..4e600aee6bc 100644 --- a/stroom-cluster/stroom-cluster-impl/src/main/java/stroom/cluster/impl/ClusterNodeManagerImpl.java +++ b/stroom-cluster/stroom-cluster-impl/src/main/java/stroom/cluster/impl/ClusterNodeManagerImpl.java @@ -361,6 +361,10 @@ private void updateMasterNode(final List enabledNodesByPriority) { // enabledNodesByPriority may contain nodes that we can't ping so find the // first one that has been deemed active by checking in clusterState which // has been mutated in the background based on ping results + final long enabledAndActiveNodeCount = enabledNodesByPriority.stream() + .filter(clusterState::isEnabledAndActive) + .count(); + enabledNodesByPriority.stream() .filter(clusterState::isEnabledAndActive) .findFirst() @@ -368,10 +372,11 @@ private void updateMasterNode(final List enabledNodesByPriority) { final String oldMasterNodeName = clusterState.setMasterNodeName(newMasterNodeName); if (!Objects.equals(oldMasterNodeName, newMasterNodeName)) { if (oldMasterNodeName == null) { - LOGGER.info("Master node is {}", newMasterNodeName); + LOGGER.info("Master node is {}, enabledAndActiveNodeCount: {}", + newMasterNodeName, enabledAndActiveNodeCount); } else { - LOGGER.info("Master node has changed from {} to {}", - oldMasterNodeName, newMasterNodeName); + LOGGER.info("Master node has changed from {} to {}, enabledAndActiveNodeCount: {}", + oldMasterNodeName, newMasterNodeName, enabledAndActiveNodeCount); } } }); diff --git a/stroom-cluster/stroom-cluster-lock-impl-db/src/main/java/stroom/cluster/lock/impl/db/ClusterLockClusterHandler.java b/stroom-cluster/stroom-cluster-lock-impl-db/src/main/java/stroom/cluster/lock/impl/db/ClusterLockClusterHandler.java index 196e8463906..ef377addcd8 100644 --- a/stroom-cluster/stroom-cluster-lock-impl-db/src/main/java/stroom/cluster/lock/impl/db/ClusterLockClusterHandler.java +++ b/stroom-cluster/stroom-cluster-lock-impl-db/src/main/java/stroom/cluster/lock/impl/db/ClusterLockClusterHandler.java @@ -77,6 +77,8 @@ boolean tryLock(final ClusterLockKey clusterLockKey) { // have just tried to put. if (lock == newLock) { success = true; + } else { + LOGGER.debug("tryLock() - Didn't get lock, lock held by {}", lock); } } @@ -229,7 +231,9 @@ private void error(final String message, final ClusterLockKey clusterLockKey, fi LOGGER.error(sb.toString()); } - private void appendStatus(final StringBuilder sb, final ClusterLockKey clusterLockKey, final Lock lock, + private void appendStatus(final StringBuilder sb, + final ClusterLockKey clusterLockKey, + final Lock lock, final Boolean success) { if (clusterLockKey != null) { sb.append(" key=("); @@ -247,10 +251,15 @@ private void appendStatus(final StringBuilder sb, final ClusterLockKey clusterLo } } + + // -------------------------------------------------------------------------------- + + private static class Lock { private final ClusterLockKey clusterLockKey; private volatile long refreshTime; + private volatile Thread thread; public Lock(final ClusterLockKey clusterLockKey) { this.clusterLockKey = clusterLockKey; @@ -259,6 +268,7 @@ public Lock(final ClusterLockKey clusterLockKey) { public void refresh() { this.refreshTime = System.currentTimeMillis(); + this.thread = Thread.currentThread(); } @Override @@ -274,6 +284,9 @@ public void append(final StringBuilder sb) { clusterLockKey.append(sb); sb.append(" age="); sb.append(ModelStringUtil.formatDurationString(age)); + sb.append(" thread='"); + sb.append(thread.getName()); + sb.append("'"); } } } diff --git a/stroom-cluster/stroom-cluster-lock-impl-db/src/main/java/stroom/cluster/lock/impl/db/ClusterLockServiceImpl.java b/stroom-cluster/stroom-cluster-lock-impl-db/src/main/java/stroom/cluster/lock/impl/db/ClusterLockServiceImpl.java index 5d8271e0ee0..d530a0d2e4c 100644 --- a/stroom-cluster/stroom-cluster-lock-impl-db/src/main/java/stroom/cluster/lock/impl/db/ClusterLockServiceImpl.java +++ b/stroom-cluster/stroom-cluster-lock-impl-db/src/main/java/stroom/cluster/lock/impl/db/ClusterLockServiceImpl.java @@ -67,6 +67,11 @@ public T lockResult(final String lockName, final Supplier supplier) { @Override public void tryLock(final String lockName, final Runnable runnable) { + + // TODO Why are we using a call to the master node to do this? + // Why not use the dbClusterLock with a .nowait() added to the JOOQ sql? + // See https://github.com/gchq/stroom/issues/5124 + final LogExecutionTime logExecutionTime = new LogExecutionTime(); LOGGER.debug("tryLock({}) - >>>", lockName); boolean success = false; diff --git a/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/DistributedTaskFetcher.java b/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/DistributedTaskFetcher.java index 0c4580518e2..9a9770992e2 100644 --- a/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/DistributedTaskFetcher.java +++ b/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/DistributedTaskFetcher.java @@ -54,6 +54,7 @@ public class DistributedTaskFetcher { private static final LambdaLogger LOGGER = LambdaLoggerFactory.getLogger(DistributedTaskFetcher.class); + public static final String JOB_NAME = "Fetch new tasks"; private final TaskStatusTraceLog taskStatusTraceLog = new TaskStatusTraceLog(); private final AtomicBoolean stopping = new AtomicBoolean(); @@ -93,13 +94,14 @@ public DistributedTaskFetcher(final ExecutorProvider executorProvider, * the executors. */ void shutdown() { + LOGGER.info("Shutting down '{}' job", JOB_NAME); try { stopping.set(true); - Thread.sleep(1000); - + // Not sure why this is here + Thread.sleep(1_000); } catch (final InterruptedException e) { - LOGGER.debug(e::getMessage, e); - + LOGGER.debug(() -> + LogUtil.message("shutdown() - Interrupted - {}", LogUtil.exceptionMessage(e), e)); // Continue to interrupt this thread. Thread.currentThread().interrupt(); } @@ -111,7 +113,18 @@ void shutdown() { public void execute() { if (running.compareAndSet(false, true)) { final Executor executor = executorProvider.get(); - executor.execute(this::fetch); + LOGGER.info("Starting '{}' job", JOB_NAME); + executor.execute(() -> { + try { + // All being well, this method won't complete until shutdown() is called as + // it will keep calling doFetch in a loop, however we need to allow for it failing. + fetch(); + } finally { + LOGGER.info("'{}' job stopped", JOB_NAME); + // Mark as not running so the ScheduledTaskExecutor can execute it again in 10s or so + running.set(false); + } + }); } } @@ -158,11 +171,12 @@ private void fetch() { } }); } catch (final RuntimeException e) { - LOGGER.error("Unable to fetch task!", e); + LOGGER.error("Error while fetching tasks - {}", LogUtil.exceptionMessage(e), e); } } private int doFetch(final TaskContext taskContext) { + LOGGER.trace("doFetch()"); int executingTaskCount = 0; info(taskContext, () -> "Starting task fetch"); diff --git a/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/JobSystemModule.java b/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/JobSystemModule.java index 03e440d7bd4..e6f80f6cd61 100644 --- a/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/JobSystemModule.java +++ b/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/JobSystemModule.java @@ -51,9 +51,9 @@ protected void configure() { // Ensure the scheduled jobs binder is present even if we don't bind actual jobs. ScheduledJobsBinder.create(binder()) .bindJobTo(FetchNewTasks.class, builder -> builder - .name("Fetch new tasks") + .name(DistributedTaskFetcher.JOB_NAME) .description("Every 10 seconds the Stroom lifecycle service will try and " + - "fetch new tasks for execution.") + "fetch new tasks for execution.") .managed(false) .frequencySchedule("10s")); diff --git a/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/ScheduledTaskExecutor.java b/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/ScheduledTaskExecutor.java index 6ba6119b8f2..ec58d2b103e 100644 --- a/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/ScheduledTaskExecutor.java +++ b/stroom-job/stroom-job-impl/src/main/java/stroom/job/impl/ScheduledTaskExecutor.java @@ -171,11 +171,16 @@ private void execute() { int unManagedTaskCount = 0; final boolean isThisNodeEnabled = isThisNodeEnabled(); for (final ScheduledJob scheduledJob : scheduledJobsMap.keySet()) { + LOGGER.trace(() -> LogUtil.message( + "execute() - name: '{}', managed: {}, isThisNodeEnabled: {}, enabled: {}, schedule: {}", + scheduledJob.getName(), scheduledJob.isManaged(), isThisNodeEnabled, + scheduledJob.isEnabled(), scheduledJob.getSchedule())); + // Managed jobs don't run on disabled nodes, but un-managed do as they are things // like lock keep-alive and meta flush which still need to happen if (isThisNodeEnabled || !scheduledJob.isManaged()) { + final String taskName = scheduledJob.getName(); try { - final String taskName = scheduledJob.getName(); final ScheduledJobFunction function = create(scheduledJob); if (function != null) { if (scheduledJob.isManaged()) { @@ -198,9 +203,18 @@ private void execute() { .runAsync(runnable, executor) .whenComplete((r, t) -> function.getRunning().set(false)); + } else { + LOGGER.trace(() -> LogUtil.message( + "execute() - Not executing {}", scheduledJobToStr(scheduledJob))); } } catch (final RuntimeException e) { - LOGGER.error(e.getMessage(), e); + LOGGER.error("Error executing {} - {}. Enable DEBUG for stack trace.", + scheduledJobToStr(scheduledJob), LogUtil.exceptionMessage(e)); + LOGGER.debug(() -> LogUtil.message( + "Error executing {} - {}", + scheduledJobToStr(scheduledJob), + LogUtil.exceptionMessage(e)), + e); } } else { LOGGER.debug(() -> LogUtil.message("Ignoring [{}] as this node '{}' is disabled", diff --git a/stroom-util-shared/src/main/java/stroom/util/shared/scheduler/Schedule.java b/stroom-util-shared/src/main/java/stroom/util/shared/scheduler/Schedule.java index a84effdc5c9..25c7769d69a 100644 --- a/stroom-util-shared/src/main/java/stroom/util/shared/scheduler/Schedule.java +++ b/stroom-util-shared/src/main/java/stroom/util/shared/scheduler/Schedule.java @@ -45,7 +45,7 @@ public boolean equals(final Object o) { } final Schedule schedule = (Schedule) o; return type == schedule.type && - Objects.equals(expression, schedule.expression); + Objects.equals(expression, schedule.expression); } @Override @@ -59,7 +59,8 @@ public String toString() { if (type != null) { sb.append(type.getDisplayValue()); } - if ((ScheduleType.FREQUENCY.equals(type) || ScheduleType.CRON.equals(type)) && expression != null) { + if ((ScheduleType.FREQUENCY.equals(type) || ScheduleType.CRON.equals(type)) + && expression != null) { sb.append(" "); sb.append(expression); } diff --git a/unreleased_changes/20250917_172638_617__0.md b/unreleased_changes/20250917_172638_617__0.md new file mode 100644 index 00000000000..9b7911ac859 --- /dev/null +++ b/unreleased_changes/20250917_172638_617__0.md @@ -0,0 +1,55 @@ +* Fix exception handling of DistributedTaskFetcher so it will restart after failure. + + +```sh +# 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. + + +# -------------------------------------------------------------------------------- +# The following is random text to make this file unique for git's change detection +# z7Jcdw68DtWuIiZ5g7dBkqo33Ua6siA4ufGxkFOKXNia6ycHXI5ZDwN4nqRwpbIlJFbATnxT2jSY0oht +# xGQiwdMq4aoKQOD248vHJ6V7BURFAQT3akavBt8rT4e243aFyG65hhpLTe5odVxSS3q8sMBSOJ65d0fP +# 7pNWG3YKgyuNh3HZUyEBnCr49OnQeIlwFbyvlupzU1mP7ImtxdwvCywahj1JBXafxXlaxPKLd4BdafJ0 +# ZQ9Xc7tTsupwoDRXSM8cCYjqdAjHsBYBYWgJDtAnNai0PiSEeIV8TwuDlQGqzzUCRMcQ8e0GP2ln0Gu7 +# RfWCJRWvI9z3U4crjxlVgBSt48hnPUx8rOMdEpMywgBVFW1GQczAshQNMcwSS82uBWsHaoYpHI2bdHS1 +# ipHk8ZGcvBrJFpJ2hPJuaDPi8jpIRVGSS0daMhh6Na2hgRhehgtfb5lR9CyCIC9lGRZ2bVrvK9MD3MRK +# 9rXuW9QLL4DLSnEbDRyh5XFy9YT7YoUkHeZZoonIivVoKtGISp0SR9y8hqtYuRZsrY3LqlATi7Kg3Jgf +# eU9vvrqq6jTncuyKaKpX2disK7Xz4t4RE1urAoVv4oSgBfFSImUCvvNK3CUtXP687qYIqKBbD0IJRmwc +# AIkYDhsadORFpdW6kHQ5rfkU4t8Yjl5D9eHjsRhFvcRxo9ADWrluUSbINO6bjkKxdv367w9BhJHtZsHy +# QLv7qUwj7yoarDSUDB4Tj2vXzo08HSNRwQVmZMfsApTephdq1SxjBjuz2izehODK0RHRom9vbAiTi5TL +# 1rzr5jaziTamYeWa41SU8KP1kX7Kr7DUGainUDhUgBXQQRdZ0HXLrl7KQ6kBesUwJdSyaGMT2YHbCzL6 +# NLriCHsK05e4buxDxzF34NJi70wl8Tbzdmv4BQ0YwFsPazFfa19ijRLap6KO0f4PC0gUGF6i3v0ZoByM +# TcS1Vh9005xyty6ky515ncza6NRv3zHa1yvSLDysUp0qmXAFbxL51E27tHt2zwWMlbCEw0o9hEashCeh +# tfUwfbs4oGLSTyNrLNc4Uj4qfo2RSdqXBrjyyl1ZzuWFy2kRwNRd5bNqMR6KuqvNNcjQqVW03vK3jmiV +# 2SlU7hcya3R3tUl2wDlj9z4RtW29EOoERpHAm4AcXDsyDZ0jFjVuA0xGb0Jm836N5F9iwqW7uS8B2Ahv +# z7Lqz847eha5eCIY81ncxXMiAwahtrssAiWK3qEKBJd6l4Xo920GbYjuzSGo98tHxBV1EYxJJKqWf0Iu +# gmPNDx8772jy5f5e6zwhyAA2uFlIlgvijg8t3m2GWrkAdjz95PZOW6090H0fHZaXqf3WuVlN0BkLSVH6 +# fMN2JcRkCvpLuFtAXDtkeRamReMHFbSiMHsgnFSR34UZKGgTM13M7m7CVZ2iGCAhKXOvvTrvIA4L9BT7 +# F0erOg93Iv8Qb2aqollRSnBu4YLMDZ4do5IfbtfcokzXU3UI5oHiCELFGCaCYJDiSgSWxI9yZLlQg7LQ +# jdqzWWE6DrzhqSaM1xipLr02KvfmD8jLqlzcl5T24rTYv5xFadsqYJljlHcPyMzRKYI6gWNg0OsH2GRt +# uEL20Oe51O2ekOGRTUOC1RgeraDVz0W0yDJdZnlwoXp0N3cDvQ4XFdmpIQvsCNrHZWQ0dk0nsYtbleM0 +# 3fe4uKWiH6P3XTV2K9WEUiXBVliHuikVNDXMtPJpOlxFucMo4qsJAF1LcXs6Dq5AYNMNHHGPIm4U3NVc +# YbwJhpAyUdJ6YYcjrrpjJicnPS2wfXGVhGL2CmNdo7w7DH9kAYVOwJTfdXbZAekbsmAHyK9rhJR81ZM6 +# EmzSh4y07r9hgtrOQSztEpOKId9BDMbTchL8DSdzVG9qP5h5k5zRIKn97r7e6XYdzVc0zBn0BvS08amD +# 5KX6Z8ftsEA9OvkivFT8K365rJA0otQCYCjKKlXdIw8yG4lOYbFim4Gi9JwbbPW9ZmjF3TTnb5eu3QqT +# apdExH6kCDUV7SanvBAlZYxlDnOVWBmdh27KNTBljck78lIeNn0KFriMT2EVxVOJWzytlGKwuONc2Fx3 +# XIt8Qmnp1SttRO6SdqxSQHpNrsLzeuJezQtI8sIX4gGkuhslBiYWT9JwOQDX6U9C3CBUs9JZjt8xSuLd +# DmLVKwJvAp2HoP8KNJuh5USzYzCNOJ8cVblVOcS2MjJOQWDh2slDvjWTUYWQ5MXQyG6H7lJLeWpYgp1M +# GN4p5AO7Db4Ad6P1MJU59AKcuOAG5gRWYEGhB5nBBLFXVyQJu8Km70f17LPt3uo15ikjvT3ymZ4a38hU +# 23M2vEuRWosxUXDRFDALg1jMPTGEDRalxEaGtRcQguwBjqcHlM5A6XLa7RlhkPxrZCPfRE8n2Ajueamw +# -------------------------------------------------------------------------------- + +```