Skip to content

Commit 47b8929

Browse files
jeremiahjstaceyjeremiahjstacey
committed
Splitting User/Client Info & Impl Propagation
Completing the split of the user/client info from the appender package. Includes updates to the LogPrefixAppender to accept the configurations as two distinct boolean values. The constructor updates of the LogPrefixAppender have been propagated back to the implemented *LogFactory implementations. Related test updates.
1 parent 617f373 commit 47b8929

File tree

11 files changed

+96
-73
lines changed

11 files changed

+96
-73
lines changed

‎src/main/java/org/owasp/esapi/logging/appender/ClientInfoSupplier.java‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@
2828
* information.
2929
*/
3030
public class ClientInfoSupplier implements Supplier<String> {
31-
/** Default UserName string if the Authenticated user is null.*/
32-
private static final String DEFAULT_USERNAME = "#ANONYMOUS#";
3331
/** Default Last Host string if the Authenticated user is null.*/
3432
private static final String DEFAULT_LAST_HOST = "#UNKNOWN_HOST#";
3533
/** Session Attribute containing the ESAPI Session id. */
@@ -44,16 +42,16 @@ public class ClientInfoSupplier implements Supplier<String> {
4442
private static final int ESAPI_SESSION_RAND_MAX = 1000000;
4543

4644
/** Format for supplier output. */
47-
private static final String USER_INFO_FORMAT = "%s:%s@%s"; // USER_NAME, SID, USER_HOST_ADDRESS
45+
private static final String USER_INFO_FORMAT = "%s@%s"; // SID, USER_HOST_ADDRESS
4846

4947
/** Whether to log the user info from this instance. */
50-
private boolean logUserInfo = true;
48+
private boolean logClientInfo = true;
5149

5250
@Override
5351
public String get() {
54-
String userInfo = "";
52+
String clientInfo = "";
5553

56-
if (logUserInfo) {
54+
if (logClientInfo) {
5755
HttpServletRequest request = ESAPI.currentRequest();
5856
// create a random session number for the user to represent the user's
5957
// 'session', if it doesn't exist already
@@ -73,21 +71,21 @@ public String get() {
7371
// log user information - username:session@ipaddr
7472
User user = ESAPI.authenticator().getCurrentUser();
7573
if (user == null) {
76-
userInfo = String.format(USER_INFO_FORMAT, DEFAULT_USERNAME, sid, DEFAULT_LAST_HOST);
74+
clientInfo = String.format(USER_INFO_FORMAT, sid, DEFAULT_LAST_HOST);
7775
} else {
78-
userInfo = String.format(USER_INFO_FORMAT, user.getAccountName(), sid, user.getLastHostAddress());
76+
clientInfo = String.format(USER_INFO_FORMAT, sid, user.getLastHostAddress());
7977
}
8078
}
81-
return userInfo;
79+
return clientInfo;
8280
}
8381

8482
/**
8583
* Specify whether the instance should record the client info.
8684
*
8785
* @param log {@code true} to record
8886
*/
89-
public void setLogUserInfo(boolean log) {
90-
this.logUserInfo = log;
87+
public void setLogClientInfo(boolean log) {
88+
this.logClientInfo = log;
9189
}
9290

9391
}

‎src/main/java/org/owasp/esapi/logging/appender/LogPrefixAppender.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
*/
2424
public class LogPrefixAppender implements LogAppender {
2525
/** Output format used to assemble return values. */
26-
private static final String RESULT_FORMAT = "[%s %s -> %s] %s";// EVENT_TYPE, CLIENT_INFO, SERVER_INFO, messageBody
26+
private static final String RESULT_FORMAT = "[%s %s:%s -> %s] %s";// EVENT_TYPE, CLIENT_INFO, SERVER_INFO, messageBody
2727

28+
/** Whether or not to record user information. */
29+
private final boolean logUserInfo;
2830
/** Whether or not to record client information. */
2931
private final boolean logClientInfo;
3032
/** Whether or not to record server ip information. */
@@ -37,12 +39,14 @@ public class LogPrefixAppender implements LogAppender {
3739
/**
3840
* Ctr.
3941
*
42+
* @param logUserInfo Whether or not to record user information
4043
* @param logClientInfo Whether or not to record client information
4144
* @param logServerIp Whether or not to record server ip information
4245
* @param logApplicationName Whether or not to record application name
4346
* @param appName Application Name to record.
4447
*/
45-
public LogPrefixAppender(boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName) {
48+
public LogPrefixAppender(boolean logUserInfo, boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName) {
49+
this.logUserInfo = logUserInfo;
4650
this.logClientInfo = logClientInfo;
4751
this.logServerIp = logServerIp;
4852
this.logApplicationName = logApplicationName;
@@ -53,17 +57,21 @@ public LogPrefixAppender(boolean logClientInfo, boolean logServerIp, boolean log
5357
public String appendTo(String logName, EventType eventType, String message) {
5458
EventTypeLogSupplier eventTypeSupplier = new EventTypeLogSupplier(eventType);
5559

60+
UserInfoSupplier userInfoSupplier = new UserInfoSupplier();
61+
userInfoSupplier.setLogUserInfo(logUserInfo);
62+
5663
ClientInfoSupplier clientInfoSupplier = new ClientInfoSupplier();
57-
clientInfoSupplier.setLogUserInfo(logClientInfo);
58-
59-
ServerInfoSupplier serverInfoSupplier = new ServerInfoSupplier(logName);
64+
clientInfoSupplier.setLogClientInfo(logClientInfo);
65+
66+
ServerInfoSupplier serverInfoSupplier = new ServerInfoSupplier(logName);
6067
serverInfoSupplier.setLogServerIp(logServerIp);
6168
serverInfoSupplier.setLogApplicationName(logApplicationName, appName);
6269

6370
String eventTypeMsg = eventTypeSupplier.get();
71+
String userInfoMsg = userInfoSupplier.get();
6472
String clientInfoMsg = clientInfoSupplier.get();
6573
String serverInfoMsg = serverInfoSupplier.get();
6674

67-
return String.format(RESULT_FORMAT, eventTypeMsg, clientInfoMsg, serverInfoMsg, message);
75+
return String.format(RESULT_FORMAT, eventTypeMsg, userInfoMsg, clientInfoMsg, serverInfoMsg, message);
6876
}
6977
}

‎src/main/java/org/owasp/esapi/logging/java/JavaLogFactory.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,13 @@ public class JavaLogFactory implements LogFactory {
5353
boolean encodeLog = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_ENCODING_REQUIRED);
5454
JAVA_LOG_SCRUBBER = createLogScrubber(encodeLog);
5555

56-
boolean logClientInfo = true;
56+
57+
boolean logUserInfo = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_USER_INFO);
58+
boolean logClientInfo = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_APP_INFO);
5759
boolean logApplicationName = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_APPLICATION_NAME);
5860
String appName = ESAPI.securityConfiguration().getStringProp(DefaultSecurityConfiguration.APPLICATION_NAME);
5961
boolean logServerIp = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_SERVER_IP);
60-
JAVA_LOG_APPENDER = createLogAppender(logClientInfo, logServerIp, logApplicationName, appName);
62+
JAVA_LOG_APPENDER = createLogAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName);
6163

6264
Map<Integer, JavaLogLevelHandler> levelLookup = new HashMap<>();
6365
levelLookup.put(Logger.ALL, JavaLogLevelHandlers.ALL);
@@ -116,8 +118,8 @@ public class JavaLogFactory implements LogFactory {
116118
*
117119
* @return LogAppender instance.
118120
*/
119-
/*package*/ static LogAppender createLogAppender(boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName) {
120-
return new LogPrefixAppender(logClientInfo, logServerIp, logApplicationName, appName);
121+
/*package*/ static LogAppender createLogAppender(boolean logUserInfo, boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName) {
122+
return new LogPrefixAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName);
121123
}
122124

123125

‎src/main/java/org/owasp/esapi/logging/log4j/Log4JLogFactory.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,13 @@ public class Log4JLogFactory implements LogFactory {
5151
boolean encodeLog = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_ENCODING_REQUIRED);
5252
Log4J_LOG_SCRUBBER = createLogScrubber(encodeLog);
5353

54-
boolean logClientInfo = true;
54+
55+
boolean logUserInfo = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_USER_INFO);
56+
boolean logClientInfo = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_APP_INFO);
5557
boolean logApplicationName = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_APPLICATION_NAME);
5658
String appName = ESAPI.securityConfiguration().getStringProp(DefaultSecurityConfiguration.APPLICATION_NAME);
5759
boolean logServerIp = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_SERVER_IP);
58-
Log4J_LOG_APPENDER = createLogAppender(logClientInfo, logServerIp, logApplicationName, appName);
60+
Log4J_LOG_APPENDER = createLogAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName);
5961

6062
Map<Integer, Log4JLogLevelHandler> levelLookup = new HashMap<>();
6163
levelLookup.put(Logger.ALL, Log4JLogLevelHandlers.TRACE);
@@ -96,8 +98,8 @@ public class Log4JLogFactory implements LogFactory {
9698
*
9799
* @return LogAppender instance.
98100
*/
99-
/*package*/ static LogAppender createLogAppender(boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName) {
100-
return new LogPrefixAppender(logClientInfo, logServerIp, logApplicationName, appName);
101+
/*package*/ static LogAppender createLogAppender(boolean logUserInfo, boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName) {
102+
return new LogPrefixAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName);
101103
}
102104

103105

‎src/main/java/org/owasp/esapi/logging/log4j/Log4JLoggerFactory.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,12 @@ public class Log4JLoggerFactory implements LoggerFactory {
4040
boolean encodeLog = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_ENCODING_REQUIRED);
4141
LOG4J_LOG_SCRUBBER = Log4JLogFactory.createLogScrubber(encodeLog);
4242

43-
boolean logClientInfo = true;
43+
boolean logUserInfo = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_USER_INFO);
44+
boolean logClientInfo = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_APP_INFO);
4445
boolean logApplicationName = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_APPLICATION_NAME);
4546
String appName = ESAPI.securityConfiguration().getStringProp(DefaultSecurityConfiguration.APPLICATION_NAME);
4647
boolean logServerIp = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_SERVER_IP);
47-
LOG4J_LOG_APPENDER = Log4JLogFactory.createLogAppender(logClientInfo, logServerIp, logApplicationName, appName);
48+
LOG4J_LOG_APPENDER = Log4JLogFactory.createLogAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName);
4849
}
4950

5051
/**

‎src/main/java/org/owasp/esapi/logging/slf4j/Slf4JLogFactory.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,13 @@ public class Slf4JLogFactory implements LogFactory {
5757
boolean encodeLog = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_ENCODING_REQUIRED);
5858
SLF4J_LOG_SCRUBBER = createLogScrubber(encodeLog);
5959

60-
boolean logClientInfo = true;
60+
61+
boolean logUserInfo = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_USER_INFO);
62+
boolean logClientInfo = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_APP_INFO);
6163
boolean logApplicationName = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_APPLICATION_NAME);
6264
String appName = ESAPI.securityConfiguration().getStringProp(DefaultSecurityConfiguration.APPLICATION_NAME);
6365
boolean logServerIp = ESAPI.securityConfiguration().getBooleanProp(DefaultSecurityConfiguration.LOG_SERVER_IP);
64-
SLF4J_LOG_APPENDER = createLogAppender(logClientInfo, logServerIp, logApplicationName, appName);
66+
SLF4J_LOG_APPENDER = createLogAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName);
6567

6668
Map<Integer, Slf4JLogLevelHandler> levelLookup = new HashMap<>();
6769
levelLookup.put(Logger.ALL, Slf4JLogLevelHandlers.TRACE);
@@ -102,8 +104,8 @@ public class Slf4JLogFactory implements LogFactory {
102104
*
103105
* @return LogAppender instance.
104106
*/
105-
/*package*/ static LogAppender createLogAppender(boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName) {
106-
return new LogPrefixAppender(logClientInfo, logServerIp, logApplicationName, appName);
107+
/*package*/ static LogAppender createLogAppender(boolean logUserInfo, boolean logClientInfo, boolean logServerIp, boolean logApplicationName, String appName) {
108+
return new LogPrefixAppender(logUserInfo, logClientInfo, logServerIp, logApplicationName, appName);
107109
}
108110

109111

‎src/test/java/org/owasp/esapi/logging/appender/ClientInfoSupplierTest.java‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ public void before() throws Exception {
6060
//Session value generation
6161
when(mockRand.getRandomInteger(ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt())).thenReturn(55555);
6262

63-
when(mockUser.getAccountName()).thenReturn(testName.getMethodName() + "-USER");
6463
when(mockUser.getLastHostAddress()).thenReturn(testName.getMethodName() + "-HOST_ADDR");
6564

6665

@@ -70,15 +69,14 @@ public void before() throws Exception {
7069
@Test
7170
public void testHappyPath() throws Exception {
7271
ClientInfoSupplier cis = new ClientInfoSupplier();
73-
cis.setLogUserInfo(true);
72+
cis.setLogClientInfo(true);
7473
String result = cis.get();
7574

76-
assertEquals(testName.getMethodName() + "-USER:"+ testName.getMethodName() + "-SESSION@"+testName.getMethodName() + "-HOST_ADDR", result);
75+
assertEquals(testName.getMethodName() + "-SESSION@"+testName.getMethodName() + "-HOST_ADDR", result);
7776

7877
verify(mockAuth,times(1)).getCurrentUser();
7978
verify(mockRequest,times(1)).getSession(false);
8079
verify(mockSession,times(1)).getAttribute(ESAPI_SESSION_ATTR);
81-
verify(mockUser,times(1)).getAccountName();
8280
verify(mockUser,times(1)).getLastHostAddress();
8381

8482
verifyNoMoreInteractions(mockAuth, mockRand, mockRequest, mockSession, mockUser);
@@ -87,7 +85,7 @@ public void testHappyPath() throws Exception {
8785
@Test
8886
public void testLogUserOff() {
8987
ClientInfoSupplier cis = new ClientInfoSupplier();
90-
cis.setLogUserInfo(false);
88+
cis.setLogClientInfo(false);
9189
String result = cis.get();
9290

9391
assertTrue(result.isEmpty());
@@ -99,10 +97,10 @@ public void testLogUserOff() {
9997
public void testLogUserNull() {
10098
when(mockAuth.getCurrentUser()).thenReturn(null);
10199
ClientInfoSupplier cis = new ClientInfoSupplier();
102-
cis.setLogUserInfo(true);
100+
cis.setLogClientInfo(true);
103101
String result = cis.get();
104102

105-
assertEquals("#ANONYMOUS#:"+testName.getMethodName()+ "-SESSION@#UNKNOWN_HOST#", result);
103+
assertEquals(testName.getMethodName()+ "-SESSION@#UNKNOWN_HOST#", result);
106104

107105
verify(mockAuth,times(1)).getCurrentUser();
108106
verify(mockRequest,times(1)).getSession(false);
@@ -115,14 +113,13 @@ public void testLogUserNull() {
115113
public void testNullRequest() throws Exception {
116114
when(ESAPI.class, "currentRequest").thenReturn(null);
117115
ClientInfoSupplier cis = new ClientInfoSupplier();
118-
cis.setLogUserInfo(true);
116+
cis.setLogClientInfo(true);
119117
String result = cis.get();
120118

121119
//sid is empty when request is null
122-
assertEquals(testName.getMethodName() + "-USER:@"+testName.getMethodName() + "-HOST_ADDR", result);
120+
assertEquals("@"+testName.getMethodName() + "-HOST_ADDR", result);
123121

124122
verify(mockAuth,times(1)).getCurrentUser();
125-
verify(mockUser,times(1)).getAccountName();
126123
verify(mockUser,times(1)).getLastHostAddress();
127124

128125
verifyNoMoreInteractions(mockAuth, mockRand, mockRequest, mockSession, mockUser);
@@ -132,16 +129,15 @@ public void testNullRequest() throws Exception {
132129
public void testNullSession() throws Exception {
133130
when(mockRequest.getSession(false)).thenReturn(null);
134131
ClientInfoSupplier cis = new ClientInfoSupplier();
135-
cis.setLogUserInfo(true);
132+
cis.setLogClientInfo(true);
136133
String result = cis.get();
137134

138135
//sid is empty when session is null
139-
assertEquals(testName.getMethodName() + "-USER:@"+testName.getMethodName() + "-HOST_ADDR", result);
136+
assertEquals("@"+testName.getMethodName() + "-HOST_ADDR", result);
140137

141138

142139
verify(mockAuth,times(1)).getCurrentUser();
143140
verify(mockRequest,times(1)).getSession(false);
144-
verify(mockUser,times(1)).getAccountName();
145141
verify(mockUser,times(1)).getLastHostAddress();
146142

147143

@@ -154,18 +150,17 @@ public void testNullSession() throws Exception {
154150
public void testNullEsapiSession() throws Exception {
155151
when(mockSession.getAttribute(ESAPI_SESSION_ATTR)).thenReturn(null);
156152
ClientInfoSupplier cis = new ClientInfoSupplier();
157-
cis.setLogUserInfo(true);
153+
cis.setLogClientInfo(true);
158154
String result = cis.get();
159155

160156
//sid is empty when session is null
161-
assertEquals(testName.getMethodName() + "-USER:55555@"+testName.getMethodName() + "-HOST_ADDR", result);
157+
assertEquals("55555@"+testName.getMethodName() + "-HOST_ADDR", result);
162158

163159
verify(mockAuth,times(1)).getCurrentUser();
164160
verify(mockRequest,times(1)).getSession(false);
165161
verify(mockSession,times(1)).getAttribute(ESAPI_SESSION_ATTR);
166162
verify(mockSession, times(1)).setAttribute(ESAPI_SESSION_ATTR, (""+55555));
167163
verify(mockRand, times(1)).getRandomInteger(ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt());
168-
verify(mockUser,times(1)).getAccountName();
169164
verify(mockUser,times(1)).getLastHostAddress();
170165

171166
verifyNoMoreInteractions(mockAuth, mockRand, mockRequest, mockSession, mockUser);

0 commit comments

Comments
 (0)