Skip to content

Commit a855d5c

Browse files
LEDfanLEDfan
committed
Fix #29445: do not log raw sessionids
1 parent 7fc7064 commit a855d5c

10 files changed

Lines changed: 67 additions & 53 deletions

File tree

‎src/main/java/eu/openanalytics/containerproxy/event/AuthFailedEvent.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,14 @@
2525
public class AuthFailedEvent extends ApplicationEvent {
2626

2727
private final String userId;
28-
private final String sessionId;
2928

30-
public AuthFailedEvent(Object source, String userId, String sessionId) {
29+
public AuthFailedEvent(Object source, String userId) {
3130
super(source);
3231
this.userId = userId;
33-
this.sessionId = sessionId;
34-
}
35-
36-
public String getSessionId() {
37-
return sessionId;
3832
}
3933

4034
public String getUserId() {
4135
return userId;
4236
}
4337

4438
}
45-
46-

‎src/main/java/eu/openanalytics/containerproxy/event/UserLoginEvent.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,15 @@
2525
public class UserLoginEvent extends ApplicationEvent {
2626

2727
private final String userId;
28-
private final String sessionId;
2928

30-
public UserLoginEvent(Object source, String userId, String sessionId) {
29+
public UserLoginEvent(Object source, String userId) {
3130
super(source);
3231
this.userId = userId;
33-
this.sessionId = sessionId;
3432
}
3533

36-
public String getSessionId() {
37-
return sessionId;
38-
}
3934

4035
public String getUserId() {
4136
return userId;
4237
}
4338

4439
}
45-
46-

‎src/main/java/eu/openanalytics/containerproxy/event/UserLogoutEvent.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,27 +25,20 @@
2525
public class UserLogoutEvent extends ApplicationEvent {
2626

2727
private final String userId;
28-
private final String sessionId;
2928
private final Boolean wasExpired;
3029

3130
/**
3231
*
3332
* @param source
3433
* @param userId
35-
* @param sessionId
3634
* @param wasExpired whether the user is logged automatically because the session has expired
3735
*/
38-
public UserLogoutEvent(Object source, String userId, String sessionId, Boolean wasExpired) {
36+
public UserLogoutEvent(Object source, String userId, Boolean wasExpired) {
3937
super(source);
4038
this.userId = userId;
41-
this.sessionId = sessionId;
4239
this.wasExpired = wasExpired;
4340
}
4441

45-
public String getSessionId() {
46-
return sessionId;
47-
}
48-
4942
public String getUserId() {
5043
return userId;
5144
}
@@ -54,5 +47,3 @@ public Boolean getWasExpired() {
5447
return wasExpired;
5548
}
5649
}
57-
58-

‎src/main/java/eu/openanalytics/containerproxy/service/AccessControlEvaluationService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public boolean allowedByUsers(Authentication auth, AccessControl accessControl)
9898
return false;
9999
}
100100
for (String user : accessControl.getUsers()) {
101-
if (auth.getName().equals(user)) {
101+
if (UserService.getUserId(auth).equals(user)) {
102102
return true;
103103
}
104104
}

‎src/main/java/eu/openanalytics/containerproxy/service/ProxyService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ public Command startProxy(Authentication user, ProxySpec spec, List<RuntimeValue
257257
Proxy.ProxyBuilder proxyBuilder = Proxy.builder();
258258
proxyBuilder.id(proxyId);
259259
proxyBuilder.status(ProxyStatus.New);
260-
proxyBuilder.userId(userService.getUserId(user));
260+
proxyBuilder.userId(UserService.getUserId(user));
261261
proxyBuilder.specId(spec.getId());
262262
proxyBuilder.createdTimestamp(System.currentTimeMillis());
263263

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

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import eu.openanalytics.containerproxy.event.UserLogoutEvent;
2828
import eu.openanalytics.containerproxy.model.runtime.Proxy;
2929
import eu.openanalytics.containerproxy.model.spec.ProxySpec;
30+
import eu.openanalytics.containerproxy.util.Sha256;
3031
import org.apache.logging.log4j.LogManager;
3132
import org.apache.logging.log4j.Logger;
3233
import org.springframework.context.ApplicationEventPublisher;
@@ -40,6 +41,7 @@
4041
import org.springframework.security.core.GrantedAuthority;
4142
import org.springframework.security.core.context.SecurityContext;
4243
import org.springframework.security.core.context.SecurityContextHolder;
44+
import org.springframework.security.web.authentication.WebAuthenticationDetails;
4345
import org.springframework.security.web.session.HttpSessionCreatedEvent;
4446
import org.springframework.security.web.session.HttpSessionDestroyedEvent;
4547
import org.springframework.stereotype.Service;
@@ -154,11 +156,11 @@ public boolean isMember(Authentication auth, String groupName) {
154156
return false;
155157
}
156158

157-
public String getUserId(Authentication auth) {
159+
public static String getUserId(Authentication auth) {
158160
if (auth == null) return null;
159161
if (auth instanceof AnonymousAuthenticationToken) {
160-
// Anonymous authentication: use the session id instead of the user name.
161-
return RequestContextHolder.currentRequestAttributes().getSessionId();
162+
// Anonymous authentication: use the session id instead of the username.
163+
return Sha256.hash(((WebAuthenticationDetails) auth.getDetails()).getSessionId());
162164
}
163165
return auth.getName();
164166
}
@@ -167,13 +169,12 @@ public String getUserId(Authentication auth) {
167169
public void onAbstractAuthenticationFailureEvent(AbstractAuthenticationFailureEvent event) {
168170
Authentication source = event.getAuthentication();
169171
Exception e = event.getException();
170-
log.info(String.format("Authentication failure [user: %s] [error: %s]", source.getName(), e.getMessage()));
172+
log.info(String.format("Authentication failure [user: %s] [error: %s]", getUserId(source), e.getMessage()));
171173
String userId = getUserId(source);
172174

173175
applicationEventPublisher.publishEvent(new AuthFailedEvent(
174176
this,
175-
userId,
176-
RequestContextHolder.currentRequestAttributes().getSessionId()));
177+
userId));
177178
}
178179

179180
public void logout(Authentication auth) {
@@ -186,26 +187,23 @@ public void logout(Authentication auth) {
186187
HttpSession session = ((ServletRequestAttributes) RequestContextHolder.currentRequestAttributes()).getRequest().getSession();
187188
session.setAttribute(ATTRIBUTE_USER_INITIATED_LOGOUT, "true"); // mark that the user initiated the logout
188189

189-
String sessionId = RequestContextHolder.currentRequestAttributes().getSessionId();
190190
applicationEventPublisher.publishEvent(new UserLogoutEvent(
191191
this,
192192
userId,
193-
sessionId,
194193
false));
195194
}
196195

197196
@EventListener
198197
public void onAuthenticationSuccessEvent(AuthenticationSuccessEvent event) {
199198
Authentication auth = event.getAuthentication();
200-
String userName = auth.getName();
199+
String userName = getUserId(auth);
201200

202201
log.info(String.format("User logged in [user: %s]", userName));
203202

204203
String userId = getUserId(auth);
205204
applicationEventPublisher.publishEvent(new UserLoginEvent(
206205
this,
207-
userId,
208-
RequestContextHolder.currentRequestAttributes().getSessionId()));
206+
userId));
209207
}
210208

211209
@EventListener
@@ -222,21 +220,20 @@ public void onHttpSessionDestroyedEvent(HttpSessionDestroyedEvent event) {
222220
SecurityContext securityContext = event.getSecurityContexts().get(0);
223221
if (securityContext == null) return;
224222

225-
String userId = securityContext.getAuthentication().getName();
223+
String userId = getUserId(securityContext.getAuthentication());
226224

227225
log.info(String.format("User logged out [user: %s]", userId));
228226
applicationEventPublisher.publishEvent(new UserLogoutEvent(
229227
this,
230228
userId,
231-
event.getSession().getId(),
232229
true
233230
));
234231
} else if (authBackend.getName().equals("none")) {
235-
log.info(String.format("Anonymous user logged out [user: %s]", event.getSession().getId()));
232+
String userId = Sha256.hash(event.getSession().getId());
233+
log.info(String.format("Anonymous user logged out [user: %s]", userId));
236234
applicationEventPublisher.publishEvent(new UserLogoutEvent(
237235
this,
238-
event.getSession().getId(),
239-
event.getSession().getId(),
236+
userId,
240237
true
241238
));
242239
}
@@ -246,11 +243,11 @@ public void onHttpSessionDestroyedEvent(HttpSessionDestroyedEvent event) {
246243
@EventListener
247244
public void onHttpSessionCreated(HttpSessionCreatedEvent event) {
248245
if (authBackend.getName().equals("none")) {
249-
log.info(String.format("Anonymous user logged in [user: %s]", event.getSession().getId()));
246+
String userId = Sha256.hash(event.getSession().getId());
247+
log.info(String.format("Anonymous user logged in [user: %s]", userId));
250248
applicationEventPublisher.publishEvent(new UserLoginEvent(
251249
this,
252-
event.getSession().getId(),
253-
event.getSession().getId()));
250+
userId));
254251
}
255252
}
256253

‎src/main/java/eu/openanalytics/containerproxy/service/session/AbstractSessionService.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import eu.openanalytics.containerproxy.auth.IAuthenticationBackend;
2424
import eu.openanalytics.containerproxy.auth.impl.NoAuthenticationBackend;
25+
import eu.openanalytics.containerproxy.util.Sha256;
2526
import org.springframework.context.annotation.Lazy;
2627
import org.springframework.security.core.Authentication;
2728

@@ -45,7 +46,7 @@ protected String extractAuthName(Authentication authentication, String sessionId
4546
}
4647

4748
if (authBackend.getName().equals(NoAuthenticationBackend.NAME)) {
48-
return sessionId;
49+
return Sha256.hash(sessionId);
4950
}
5051

5152
return null;

‎src/main/java/eu/openanalytics/containerproxy/spec/expression/SpecExpressionContext.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ public static SpecExpressionContext create(SpecExpressionContextBuilder builder,
135135
}
136136
if (o instanceof Authentication) {
137137
builder.groups = Arrays.asList(UserService.getGroups((Authentication) o));
138-
builder.userId = ((Authentication) o).getName();
138+
builder.userId = UserService.getUserId(((Authentication) o));
139139
}
140140
}
141141
return builder.build();

‎src/main/java/eu/openanalytics/containerproxy/stat/impl/Micrometer.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,13 @@ public void run() {
118118

119119
@EventListener
120120
public void onUserLogoutEvent(UserLogoutEvent event) {
121-
logger.debug("UserLogoutEvent [user: {}, sessionId: {}, expired: {}]", event.getUserId(), event.getSessionId(), event.getWasExpired());
121+
logger.debug("UserLogoutEvent [user: {}, expired: {}]", event.getUserId(), event.getWasExpired());
122122
userLogouts.increment();
123123
}
124124

125125
@EventListener
126126
public void onUserLoginEvent(UserLoginEvent event) {
127-
logger.debug("UserLoginEvent [user: {}, sessionId: {}]", event.getUserId(), event.getSessionId());
127+
logger.debug("UserLoginEvent [user: {}]", event.getUserId());
128128
userLogins.increment();
129129
}
130130

@@ -178,7 +178,7 @@ public void onProxyStartFailedEvent(ProxyStartFailedEvent event) {
178178

179179
@EventListener
180180
public void onAuthFailedEvent(AuthFailedEvent event) {
181-
logger.debug("AuthFailedEvent [user: {}, sessionId: {}]", event.getUserId(), event.getSessionId());
181+
logger.debug("AuthFailedEvent [user: {}]", event.getUserId());
182182
authFailedCounter.increment();
183183
}
184184

‎src/main/java/eu/openanalytics/containerproxy/util/Sha256.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
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.util;
22+
23+
import java.math.BigInteger;
24+
import java.security.MessageDigest;
25+
import java.security.NoSuchAlgorithmException;
26+
27+
public class Sha256 {
28+
29+
public static String hash(String value) {
30+
try {
31+
MessageDigest digest = MessageDigest.getInstance("SHA-256");
32+
digest.reset();
33+
digest.update(value.getBytes());
34+
return String.format("%040x", new BigInteger(1, digest.digest()));
35+
} catch (NoSuchAlgorithmException ex) {
36+
throw new RuntimeException(ex);
37+
}
38+
}
39+
40+
}

0 commit comments

Comments
 (0)