Skip to content

Commit 405b643

Browse files
committed
gh-5132 Fix missing session when AWS ALB does the code flow
1 parent 77e3bb9 commit 405b643

File tree

5 files changed

+98
-35
lines changed

5 files changed

+98
-35
lines changed

stroom-dropwizard-common/src/main/java/stroom/dropwizard/common/AuthenticationBypassCheckerImpl.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ void registerUnauthenticatedApiPath(final String path) {
3333
}
3434

3535
@Override
36-
public boolean isUnauthenticated(final String servletName, final String servletPath, final String fullPath) {
36+
public boolean isUnauthenticated(final String servletName,
37+
final String servletPath,
38+
final String fullPath) {
3739
if (servletPath == null) {
3840
return false;
3941
} else {
@@ -49,8 +51,8 @@ public boolean isUnauthenticated(final String servletName, final String servletP
4951
canBypassAuth = unauthenticatedServletNames.contains(servletName);
5052
}
5153

52-
LOGGER.debug("servletName: {}, servletPath: {}, canBypassAuth: {}",
53-
servletName, servletPath, canBypassAuth);
54+
LOGGER.debug("isUnauthenticated() - servletName: {}, servletPath: {}, fullPath: {}, canBypassAuth: {}",
55+
servletName, servletPath, fullPath, canBypassAuth);
5456
return canBypassAuth;
5557
}
5658
}

stroom-security/stroom-security-common-impl/src/main/java/stroom/security/common/impl/JwtContextFactory.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,5 @@ public interface JwtContextFactory {
2929
* like audience and subject only if doVerification is true.
3030
*/
3131
Optional<JwtContext> getJwtContext(final String jwt, final boolean doVerification);
32+
3233
}

stroom-security/stroom-security-impl/src/main/java/stroom/security/impl/SecurityFilter.java

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -136,25 +136,24 @@ private void filter(final HttpServletRequest request,
136136
chain.doFilter(request, response);
137137
} else if (isStaticResource(fullPath, servletPath, servletName)) {
138138
chain.doFilter(request, response);
139+
} else if (shouldBypassAuthentication(request, fullPath, servletPath, servletName)) {
140+
LOGGER.debug("Running as proc user for unauthenticated resource, servletName: {}, " +
141+
"fullPath: {}, servletPath: {}", servletName, fullPath, servletPath);
142+
// Some paths don't need authentication. If that is the case then proceed as proc user.
143+
securityContext.asProcessingUser(() ->
144+
process(request, response, chain));
139145
} else {
140146
// Api requests that are not from the front-end should have a token.
141147
// Also request from an AWS ALB will have an ALB signed token containing the claims
142148
// Need to do this first, so we get a fresh token from AWS ALB rather than using a stale
143149
// one from session.
144150
Optional<UserIdentity> optUserIdentity = openIdManager.loginWithRequestToken(request);
145-
146-
// Log current user.
147-
if (LOGGER.isDebugEnabled()) {
148-
logUserIdentityToDebug(
149-
optUserIdentity, fullPath, "after trying to login with request token");
150-
}
151+
logUserIdentityToDebug(optUserIdentity, fullPath, servletPath, "from request token");
151152

152153
// If no user from header token, see if we have one in session already.
153154
if (optUserIdentity.isEmpty()) {
154155
optUserIdentity = UserIdentitySessionUtil.get(SessionUtil.getExistingSession(request));
155-
if (LOGGER.isDebugEnabled()) {
156-
logUserIdentityToDebug(optUserIdentity, fullPath, "from session");
157-
}
156+
logUserIdentityToDebug(optUserIdentity, fullPath, servletPath, "from session");
158157
}
159158

160159
if (optUserIdentity.isPresent()) {
@@ -163,19 +162,20 @@ private void filter(final HttpServletRequest request,
163162
// Now we have the session make note of the user-agent for logging and sessionListServlet duties
164163
UserAgentSessionUtil.setUserAgentInSession(request);
165164

165+
// If OIDC code flow has been handled by the AWS ALB then the session won't have been
166+
// created by our code flow code. Thus, ensure we have a session with the user in it
167+
if (isStroomUIServlet(servletName)) {
168+
SessionUtil.getOrCreateSession(request, session -> {
169+
LOGGER.info("Creating session {} for user {}, fullPath: {}, servlet: {}",
170+
session.getId(), userIdentity, fullPath, servletName);
171+
UserIdentitySessionUtil.set(session, userIdentity);
172+
});
173+
}
174+
166175
// Now handle the request as this user
167176
securityContext.asUser(userIdentity, () ->
168177
process(request, response, chain));
169-
170-
} else if (shouldBypassAuthentication(request, fullPath, servletPath, servletName)) {
171-
LOGGER.debug("Running as proc user for unauthenticated servletName: {}, " +
172-
"fullPath: {}, servletPath: {}", servletName, fullPath, servletPath);
173-
// Some paths don't need authentication. If that is the case then proceed as proc user.
174-
securityContext.asProcessingUser(() ->
175-
process(request, response, chain));
176-
177-
// } else if (isApiRequest(servletPath)) {
178-
} else if (Objects.equals(ResourcePaths.STROOM_SERVLET_NAME, servletName)) {
178+
} else if (isStroomUIServlet(servletName)) {
179179
doOpenIdFlow(request, response, fullPath);
180180
} else {
181181
// If we couldn't log in with a token or couldn't get a token then error as this is an API call
@@ -187,6 +187,10 @@ private void filter(final HttpServletRequest request,
187187
}
188188
}
189189

190+
private boolean isStroomUIServlet(final String servletName) {
191+
return Objects.equals(ResourcePaths.STROOM_SERVLET_NAME, servletName);
192+
}
193+
190194
private void doOpenIdFlow(final HttpServletRequest request,
191195
final HttpServletResponse response,
192196
final String fullPath) throws IOException {
@@ -245,8 +249,9 @@ private void doOpenIdFlow(final HttpServletRequest request,
245249
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
246250
private void logUserIdentityToDebug(final Optional<UserIdentity> optUserIdentity,
247251
final String fullPath,
252+
final String servletName,
248253
final String msg) {
249-
LOGGER.debug("User identity ({}): {} path: {}",
254+
LOGGER.debug(() -> LogUtil.message("User identity ({}): {}, fullPath: {}, servletName: {}",
250255
msg,
251256
optUserIdentity.map(
252257
identity -> {
@@ -258,7 +263,8 @@ private void logUserIdentityToDebug(final Optional<UserIdentity> optUserIdentity
258263
identity.getClass().getSimpleName());
259264
})
260265
.orElse("<empty>"),
261-
fullPath);
266+
fullPath,
267+
servletName));
262268
}
263269

264270
private String getPostAuthRedirectUri(final HttpServletRequest request) {
@@ -298,17 +304,6 @@ private boolean shouldBypassAuthentication(final HttpServletRequest servletReque
298304
} else {
299305
shouldBypass = authenticationBypassChecker.isUnauthenticated(servletName, servletPath, fullPath);
300306
}
301-
302-
if (LOGGER.isDebugEnabled()) {
303-
if (shouldBypass) {
304-
LOGGER.debug("Bypassing authentication for servletName: {}, fullPath: {}, servletPath: {}",
305-
NullSafe.get(
306-
servletRequest.getHttpServletMapping(),
307-
HttpServletMapping::getServletName),
308-
fullPath,
309-
servletPath);
310-
}
311-
}
312307
return shouldBypass;
313308
}
314309

stroom-util/src/main/java/stroom/util/servlet/SessionUtil.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ public static boolean requestHasSessionCookie(final HttpServletRequest request)
3434
return hasCookie;
3535
}
3636

37+
public static boolean hasSession(final HttpServletRequest request) {
38+
return request.getSession(false) != null;
39+
}
40+
41+
3742
/**
3843
* Gets the existing {@link HttpSession} or returns null if no session is present.
3944
* Does NOT create a session.
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
* Issue **#5132** : Fix missing session when AWS ALB does the code flow.
2+
3+
4+
```sh
5+
# ********************************************************************************
6+
# Issue title: /stroom/sessionList is empty on 7.10.2
7+
# Issue link: https://github.com/gchq/stroom/issues/5132
8+
# ********************************************************************************
9+
10+
# ONLY the top line will be included as a change entry in the CHANGELOG.
11+
# The entry should be in GitHub flavour markdown and should be written on a SINGLE
12+
# line with no hard breaks. You can have multiple change files for a single GitHub issue.
13+
# The entry should be written in the imperative mood, i.e. 'Fix nasty bug' rather than
14+
# 'Fixed nasty bug'.
15+
#
16+
# Examples of acceptable entries are:
17+
#
18+
#
19+
# * Issue **123** : Fix bug with an associated GitHub issue in this repository
20+
#
21+
# * Issue **namespace/other-repo#456** : Fix bug with an associated GitHub issue in another repository
22+
#
23+
# * Fix bug with no associated GitHub issue.
24+
25+
26+
# --------------------------------------------------------------------------------
27+
# The following is random text to make this file unique for git's change detection
28+
# 0OFaWWSfTLSoczakC5MFfSFrLeOaQcVELlisvohpCkViqq1RGA324pldl67yaM4UuyIg9R58n46LFBZ2
29+
# hJ9CTv1WQPyobhGNuSyJCUPGEVK8qY9bwCLAwuE29oHZSYWNQuvw4ygUH3wv9fH1lSPZxEKc9EAqRxn8
30+
# 7pYE8v7E1yfsmPCLHsmcatdYLbXjJX4M0I30y1J78zAkBJ3lAUk7Z2iQW90fMsB2daaduMfZmCanwXt8
31+
# CdW139qLO4O93DQeOPaJ4qTcUeQ7jUn10oRRF9cf5esL5PbGHzyC65hkQCbRhitXHAcZHYQUbSloa0Bo
32+
# maK0NZE0ryaj0mfQOUrGJOIOSdtEgzNY0AuMT679qA1LkqXZmKNikvsYsW7vywhxnDyQTW6tdsy9LTIi
33+
# 7rhHUNI9fTZw0ufKraas71hiW5xQja1zhXAchLbOs57KR1vFy8VC0WfHLhripwuK6vNGIGv18piP5GM9
34+
# mXBO7LZghpBl38s6UhOhRFCLkWbCWVrnyxjU96TkJ1j6yXWkC4a7lMyJaa0QNElYgjGnzv74EeeLHIIR
35+
# KFZUtIN9phOrihWl7eJIlBWDXba6kxorB4zQmpzchrigUXTmr1RV38AlEtZ0CiPvSMBM9wU4ACK9LPWk
36+
# PlQ65g2uROweRte35KFZBnI7AS2tED50lFncfrPUfY9csLbRxRo9gc3D3RwiZobaIWEh1EYdmPkp8NCi
37+
# vYyogaG13Om3M52FC8l09wdd4RoOwG3qBM1HnaC1h6xsjetJ1xuyF4OoEF5EV2nYYvyu7WAWvgTI4pN5
38+
# WW08RRyyvWl439FTafptIxNi5ZnnkPgC6xdrJzKzrAZMKYfieQ3RlYVX7iZh6LsVm9Bgh0uazT81oR4e
39+
# M3BHdM6bYjU18gnXNFnZK9b1k3UDCjznGLwWL1uN6AfyPBpygyjb4dG6sFEmChmC0aLUAs5eeAUaFZrn
40+
# vBY1QOUh1UvxePeOPyqKb6ZIFYLIMGGaOVtKopunOg2RdZmqzH7QBLTSyUxK9oVO7e1GDDNp2AuYujVS
41+
# lF1G3T0TEr4pdTLQJgugqFShuwUkVRVQgsCl0sXOuJAoBbb9yEfvvohTWJEUUatHYxmpBSuFg9jWW9mj
42+
# DaaRGcBuTEYMluf4knHNAlJ34isYVjQsOcpra2p5fFPgfbPgV7QZ1AHxUWjI0qQAip2ZNyEoiYfb6Uk5
43+
# bhvaQuAIr3vgLB5FQo3QKU2QygiOMNane2HbLyZTr79QPolhzhHL8XJXE8brTGEBUpfW6HPneNSoRTWs
44+
# Ey2BFQ6iyshzx10WVMlyg1U3FcD1u9umTEIMX1eWsQdpRcAaZgjsmHgbIMkENsNBMCKTyRjn6Q0VkUjb
45+
# KLkdhGT8e3qsVCNNoOD3SnQ8A88BqEq0ixUIcbKewmxLw0Qt96hLjlUFEjVUJ0VkAMlsxuwc0Dxhh3RA
46+
# WkfzAAAhY7Xqu9LK9DY4yt6jBHfZ2tLSLcOdt3tEpSEXFzkjzHzDBCVOq4q1HtqoWamEdknhOgAlLM8h
47+
# ARmZp7RGh6HMMO7YF9zmvxehFRJwVmITGBhluCEKhOz1gMVwVdtvqwqTnSkc53dE0K6WEUMIR0nKj9BB
48+
# W8NtmXclIKDfGm1hcLgegT0Uojyas41zwpprGsNGnNJMpyLfiprBbfnLcUiBHvG8EercFHXPGTHlougd
49+
# 9yitiIXKWCZF93hquGlAsQR987gCRcqu229B4rNlEYoaZiVLfN3sWOJPcVW9rd0H7BsDyomt87dtR4bW
50+
# UQH0Wkfd8TvgLVuKGIcBsI7Ao1bjJX30bOsyUu47Ah029z2AG6cGdey5etsEiExIMeYIn0SHxXxIA7yj
51+
# KIQbFN5j5lR0G053T7QmcE4hINWLe3koixgVIyagWfOg6NThPBLgQeMlepjpXPOEIXwfQlSJUQe2EDQc
52+
# Eyl5maRtMnqs4RldkKGXofGnIosbPkSQvEwAkAGhtd4hBw7XCtVVQqDW0kM8fpIOMiStjavcRvpgtsgH
53+
# X6dcjBAoBylFGxDP4DVg0HWjjnm4CZSv50kIgBXjg0wTsz8ogoDDs08Rw6X9qFqmXHlwmtw1Nur9Ifkt
54+
# PhijQUwmbFRj7clxYQKIKuzXogQabtstKGsnSksEGw5s4ucIj1ADR2GjWllC6NDvykCc8GdJB92J7vgp
55+
# OgG95M5cq7P9nHY0VIYkEMtJyHnB1QTbCtoPRb28vr3JK1tYkCUQnX9dSvMtZHYaF0R4kNB3e7NCx3fn
56+
# KbkLXo3OCEhKFHGCTqwRiWbRFnTY3aRYaPl3TVRd1twqcFmiavXa2TxPhsEWpirYp9FuFQBevcP6rSvp
57+
# 6zUovoOfBjbu46UJfMb1FN2IOkinWv50PRcxZ2SoJemecXiYLxN8OoaJUTnHdoXFsyhDOmsrjsBkdqbj
58+
# --------------------------------------------------------------------------------
59+
60+
```

0 commit comments

Comments
 (0)