Skip to content

Commit 87cee6e

Browse files
committed
Fix authentication none with new session services
- ensure cookie is created for sessions - fix code that relied on the name of the cookie (which doesn't work in the case of using Spring Redis for sessions) - fix active/logged in user metric when using authentication none
1 parent 08a69f4 commit 87cee6e

6 files changed

Lines changed: 74 additions & 114 deletions

File tree

src/main/java/eu/openanalytics/containerproxy/security/WebSecurityConfig.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
3939
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
4040
import org.springframework.security.config.annotation.web.configurers.ExpressionUrlAuthorizationConfigurer;
41+
import org.springframework.security.config.http.SessionCreationPolicy;
4142
import org.springframework.security.web.access.AccessDeniedHandler;
4243
import org.springframework.security.web.access.AccessDeniedHandlerImpl;
4344
import org.springframework.security.web.authentication.www.BasicAuthenticationFilter;
@@ -181,7 +182,8 @@ public void handle(HttpServletRequest request, HttpServletResponse response, Acc
181182
auth.configureHttpSecurity(http, anyRequestConfigurer);
182183
}
183184

184-
185+
// create session cookie even if there is no Authentication in order to support the None authentication backend
186+
http.sessionManagement().sessionCreationPolicy(SessionCreationPolicy.ALWAYS);
185187
}
186188

187189
@Bean

src/main/java/eu/openanalytics/containerproxy/service/UserService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ private String getUserId(Authentication auth) {
165165
if (auth == null) return null;
166166
if (auth instanceof AnonymousAuthenticationToken) {
167167
// Anonymous authentication: use the session id instead of the user name.
168-
return SessionHelper.getCurrentSessionId(true);
168+
return RequestContextHolder.currentRequestAttributes().getSessionId();
169169
}
170170
return auth.getName();
171171
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/**
2+
* ContainerProxy
3+
*
4+
* Copyright (C) 2016-2021 Open Analytics
5+
*
6+
* ===========================================================================
7+
*
8+
* This program is free software: you can redistribute it and/or modify
9+
* it under the terms of the Apache License as published by
10+
* The Apache Software Foundation, either version 2 of the License, or
11+
* (at your option) any later version.
12+
*
13+
* This program is distributed in the hope that it will be useful,
14+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
15+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16+
* Apache License for more details.
17+
*
18+
* You should have received a copy of the Apache License
19+
* along with this program. If not, see <http://www.apache.org/licenses/>
20+
*/
21+
package eu.openanalytics.containerproxy.service.session;
22+
23+
import eu.openanalytics.containerproxy.auth.IAuthenticationBackend;
24+
import eu.openanalytics.containerproxy.auth.impl.NoAuthenticationBackend;
25+
import org.springframework.context.annotation.Lazy;
26+
import org.springframework.security.core.Authentication;
27+
28+
import javax.inject.Inject;
29+
30+
abstract public class AbstractSessionService implements ISessionService {
31+
32+
@Inject
33+
@Lazy
34+
// Note: lazy needed to work around early initialization conflict
35+
// Only used to check whether we are using Authentication none
36+
private IAuthenticationBackend authBackend;
37+
38+
protected String extractAuthName(Authentication authentication, String sessionId) {
39+
if (authentication != null && !authentication.isAuthenticated()) {
40+
return null;
41+
}
42+
43+
if (authentication != null) {
44+
return authentication.getName();
45+
}
46+
47+
if (authBackend.getName().equals(NoAuthenticationBackend.NAME)) {
48+
return sessionId;
49+
}
50+
51+
return null;
52+
}
53+
54+
}

src/main/java/eu/openanalytics/containerproxy/service/session/redis/RedisSessionService.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import eu.openanalytics.containerproxy.RedisSessionConfig;
2424
import eu.openanalytics.containerproxy.service.hearbeat.HeartbeatService;
25+
import eu.openanalytics.containerproxy.service.session.AbstractSessionService;
2526
import eu.openanalytics.containerproxy.service.session.ISessionService;
2627
import io.undertow.server.HttpServerExchange;
2728
import io.undertow.servlet.handlers.ServletRequestContext;
@@ -52,7 +53,7 @@
5253

5354
@Component
5455
@ConditionalOnProperty(name = "spring.session.store-type", havingValue = "redis")
55-
public class RedisSessionService implements ISessionService {
56+
public class RedisSessionService extends AbstractSessionService {
5657

5758
private static final Pattern SESSION_ID_PATTERN = Pattern.compile("^.*sessions:([a-z0-9-]*)$");
5859
private static final int CACHE_UPDATE_INTERVAL = 20 * 1000; // update cache every minutes
@@ -134,13 +135,13 @@ private void updateCachedUsersLoggedInCount() {
134135
Session session = redisIndexedSessionRepository.findById(sessionId);
135136
if (session == null) continue;
136137

137-
Authentication authentication = extractAuthenticationIfAuthenticated(session);
138-
if (authentication == null) continue;
138+
String authenticationName = extractAuthName(extractAuthenticationIfAuthenticated(session), sessionId);
139+
if (authenticationName == null) continue;
139140

140-
authenticatedUsers.add(authentication.getName());
141+
authenticatedUsers.add(authenticationName);
141142

142143
if (session.getLastAccessedTime().isAfter(minimumInstantTime)) {
143-
activeUsers.add(authentication.getName());
144+
activeUsers.add(authenticationName);
144145
}
145146
}
146147

src/main/java/eu/openanalytics/containerproxy/service/session/undertow/UndertowSessionService.java

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
*/
2121
package eu.openanalytics.containerproxy.service.session.undertow;
2222

23-
import eu.openanalytics.containerproxy.service.session.ISessionService;
23+
import eu.openanalytics.containerproxy.service.session.AbstractSessionService;
2424
import io.undertow.server.HttpServerExchange;
2525
import io.undertow.server.session.InMemorySessionManager;
2626
import io.undertow.server.session.Session;
@@ -43,7 +43,7 @@
4343

4444
@Component
4545
@ConditionalOnProperty(name = "spring.session.store-type", havingValue = "none")
46-
public class UndertowSessionService implements ISessionService {
46+
public class UndertowSessionService extends AbstractSessionService {
4747

4848
private static final int CACHE_UPDATE_INTERVAL = 20 * 1000; // update cache every minutes
4949

@@ -119,17 +119,17 @@ private void updateCachedUsersLoggedInCount() {
119119
Set<String> authenticatedUsers = new HashSet<>();
120120
Set<String> activeUsers = new HashSet<>();
121121

122-
for (String sessionId: instance.getAllSessions()) {
122+
for (String sessionId : instance.getAllSessions()) {
123123
Session session = instance.getSession(sessionId);
124124
if (session == null) continue;
125125

126-
Authentication authentication = extractAuthenticationIfAuthenticated(session);
127-
if (authentication == null) continue;
126+
String authenticationName = extractAuthName(extractAuthenticationIfAuthenticated(session), sessionId);
127+
if (authenticationName == null) continue;
128128

129-
authenticatedUsers.add(authentication.getName());
129+
authenticatedUsers.add(authenticationName);
130130

131131
if (Instant.ofEpochMilli(session.getLastAccessedTime()).isAfter(minimumInstantTime)) {
132-
activeUsers.add(authentication.getName());
132+
activeUsers.add(authenticationName);
133133
}
134134
}
135135

@@ -142,23 +142,18 @@ private void updateCachedUsersLoggedInCount() {
142142

143143
/**
144144
* Extracts the {@link Authentication} object from the given Session, if and only if the user is authenticated.
145+
*
145146
* @param session the session
146147
* @return Authentication|null
147148
*/
148149
private Authentication extractAuthenticationIfAuthenticated(Session session) {
149150
Object object = session.getAttribute("SPRING_SECURITY_CONTEXT");
150151

151152
if (object instanceof SecurityContext) {
152-
SecurityContext securityContext = (SecurityContext) object;
153-
154-
if (securityContext.getAuthentication().isAuthenticated()) {
155-
156-
return securityContext.getAuthentication();
157-
}
153+
return ((SecurityContext) object).getAuthentication();
158154
}
159155

160156
return null;
161157
}
162158

163-
164-
}
159+
}

src/main/java/eu/openanalytics/containerproxy/util/SessionHelper.java

Lines changed: 0 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -20,42 +20,10 @@
2020
*/
2121
package eu.openanalytics.containerproxy.util;
2222

23-
import java.util.Optional;
24-
25-
import javax.servlet.http.HttpSession;
26-
2723
import org.springframework.core.env.Environment;
28-
import org.springframework.security.authentication.AnonymousAuthenticationToken;
29-
import org.springframework.security.core.AuthenticatedPrincipal;
30-
import org.springframework.security.core.context.SecurityContext;
31-
32-
import io.undertow.server.HttpServerExchange;
33-
import io.undertow.server.handlers.Cookie;
34-
import io.undertow.servlet.handlers.ServletRequestContext;
35-
import io.undertow.util.HeaderValues;
3624

3725
public class SessionHelper {
3826

39-
/**
40-
* Looks up the session of the current servlet exchange, and return its ID.
41-
*
42-
* @param createIfMissing True to create a session if no session is currently active.
43-
* @return The current session ID, or null if no session is active.
44-
*/
45-
public static String getCurrentSessionId(boolean createIfMissing) {
46-
ServletRequestContext context = ServletRequestContext.current();
47-
if (context == null) return null;
48-
49-
HttpSession session = context.getSession();
50-
if (session != null) return session.getId();
51-
52-
Cookie jSessionIdCookie = context.getExchange().getRequestCookies().get("JSESSIONID");
53-
if (jSessionIdCookie != null) return jSessionIdCookie.getValue();
54-
55-
if (createIfMissing) return context.getCurrentServletContext().getSession(context.getExchange(), true).getId();
56-
else return null;
57-
}
58-
5927
/**
6028
* Get the context path that has been configured for this instance.
6129
*
@@ -72,65 +40,5 @@ public static String getContextPath(Environment environment, boolean endWithSlas
7240

7341
return contextPath;
7442
}
75-
76-
/**
77-
* Obtain information about the 'owner' of the current HTTP exchange.
78-
* This method will try to identify the owner, even if:
79-
*
80-
* <ul>
81-
* <li>There is no servlet context</li>
82-
* <li>There is no authentication backend (users are anonymous)</li>
83-
* </ul>
84-
*
85-
* @param exchange The current HTTP exchange.
86-
* @return An object containing information about the current user.
87-
*/
88-
public static SessionOwnerInfo createOwnerInfo(HttpServerExchange exchange) {
89-
SessionOwnerInfo info = new SessionOwnerInfo();
90-
91-
// Ideally, use the HTTP session information.
92-
info.principal = Optional.ofNullable(ServletRequestContext.current())
93-
.map(ctx -> ctx.getSession())
94-
.map(session -> (SecurityContext) session.getAttribute("SPRING_SECURITY_CONTEXT"))
95-
.map(ctx -> ctx.getAuthentication())
96-
.filter(auth -> !AnonymousAuthenticationToken.class.isInstance(auth))
97-
.map(auth -> auth.getPrincipal())
98-
.orElse(null);
99-
100-
// Fallback: use the Authorization header, if present.
101-
HeaderValues authHeader = exchange.getRequestHeaders().get("Authorization");
102-
if (authHeader != null) info.authHeader = authHeader.getFirst();
10343

104-
// Fallback: use the JSESSIONID cookie, if present.
105-
Cookie jSessionIdCookie = exchange.getRequestCookies().get("JSESSIONID");
106-
if (jSessionIdCookie != null) info.jSessionId = jSessionIdCookie.getValue();
107-
108-
// Final fallback: generate a JSESSIONID for this exchange.
109-
// Supports anonymous requests (i.e. authentication: 'none')
110-
if (info.principal == null && info.authHeader == null && info.jSessionId == null) {
111-
info.jSessionId = getCurrentSessionId(true);
112-
}
113-
114-
return info;
115-
}
116-
117-
public static class SessionOwnerInfo {
118-
119-
public Object principal;
120-
public String authHeader;
121-
public String jSessionId;
122-
123-
public boolean isSame(SessionOwnerInfo other) {
124-
if (principal != null && other.principal != null) {
125-
//TODO Probably, this check will have to be made dependent on the type of principal (LDAP vs OIDC etc)
126-
if (principal instanceof AuthenticatedPrincipal && other.principal instanceof AuthenticatedPrincipal) {
127-
return ((AuthenticatedPrincipal) principal).getName().equals(((AuthenticatedPrincipal) other.principal).getName());
128-
}
129-
return principal.equals(other.principal);
130-
}
131-
if (authHeader != null && other.authHeader != null) return authHeader.equals(other.authHeader);
132-
if (jSessionId != null && other.jSessionId != null) return jSessionId.equals(other.jSessionId);
133-
return false;
134-
}
135-
}
13644
}

0 commit comments

Comments
 (0)