feat(jans-auth-server): sanitized log in AuthenticationService #12958 by yuriyz · Pull Request #12959 · JanssenProject/jans (original) (raw)

422-423: Potential incomplete sanitization: keyValue might be sensitive.

The keyValue parameter in this debug log likely contains a username and should be sanitized for consistency with the PR's security objectives.

🔒 Suggested sanitization

   log.debug("Attempting to find userDN by primary key: '{}' and key value: '{}', credentials: '{}'", primaryKey,

           keyValue, System.identityHashCode(credentials));

   log.debug("Attempting to find userDN by primary key: '{}' and key value: '{}', credentials: '{}'", primaryKey,

           sanitizeUsernameForLog(keyValue), System.identityHashCode(credentials));

572-573: Potential incomplete sanitization: attributeValue might contain sensitive data.

The attributeValue in getUserByAttribute could be a username when the attributeName is a username field (like "uid"). Consider sanitizing for completeness.

🔒 Suggested sanitization

   log.debug("Getting user information from LDAP: attributeName = '{}', attributeValue = '{}'", attributeName,

```
           attributeValue);
```

   log.debug("Getting user information from LDAP: attributeName = '{}', attributeValue = '{}'", attributeName,

           sanitizeUsernameForLog(attributeValue));

605-605: Inconsistent sanitization: userId logged without sanitization.

The user.getUserId() is logged in this warning message without sanitization, while similar userId logging elsewhere in the file has been sanitized.

🔒 Suggested sanitization

   log.warn("User '{}' is disabled", user.getUserId());

   log.warn("User '{}' is disabled", sanitizeUsernameForLog(user.getUserId()));

641-641: Inconsistent sanitization: userId logged without sanitization.

The user.getUserId() is logged in this error message without sanitization, while similar userId logging elsewhere has been sanitized.

🔒 Suggested sanitization

       log.error("Failed to update jansLastLogonTime of user '{}'", user.getUserId());

       log.error("Failed to update jansLastLogonTime of user '{}'", sanitizeUsernameForLog(user.getUserId()));

653-653: Inconsistent sanitization: sessionAuthUser (username) logged without sanitization.

The sessionAuthUser variable contains a username from session attributes and should be sanitized for consistency.

🔒 Suggested sanitization

   log.trace("configureSessionUser sessionId: '{}', sessionId.auth_user: '{}'", sessionId, sessionAuthUser);

   log.trace("configureSessionUser sessionId: '{}', sessionId.auth_user: '{}'", sessionId, sanitizeUsernameForLog(sessionAuthUser));

669-669: Inconsistent sanitization: userId logged without sanitization.

The user.getUserId() is logged without sanitization, while similar logging has been sanitized elsewhere in this file.

🔒 Suggested sanitization

   log.debug("ConfigureEventUser: username: '{}', credentials: '{}'", user.getUserId(),

           System.identityHashCode(credentials));

   log.debug("ConfigureEventUser: username: '{}', credentials: '{}'", sanitizeUsernameForLog(user.getUserId()),

           System.identityHashCode(credentials));

740-741: Consider sanitizing clientInum for consistency.

The clientInum variable comes from credentials.getUsername() (line 739) and is logged without sanitization. While this appears to be for client authentication rather than user authentication, consider sanitizing it for consistency with the security objectives of this PR.

🔒 Suggested sanitization

   log.debug("ConfigureSessionClient: username: '{}', credentials: '{}'", clientInum,

           System.identityHashCode(credentials));

   log.debug("ConfigureSessionClient: username: '{}', credentials: '{}'", sanitizeUsernameForLog(clientInum),

           System.identityHashCode(credentials));

1-905: Multiple instances of unsanitized username logging require sanitization.

The code contains several log statements that expose usernames and user identifiers without proper sanitization:

Line 316: keyValue logged directly in log.debug() without sanitization
Line 459: userInum logged directly without sanitization
Line 524: attributeValue logged directly without sanitization (may contain username)
Line 531: customAttributes logged directly without sanitization (may contain sensitive data)
Line 590: user object logged directly without sanitization
Line 595: sessionUser object logged directly without sanitization
Line 602: user object logged directly without sanitization
Line 782: user.getUserId() logged directly without sanitization

Wrap these values with sanitizeUsernameForLog() to prevent usernames from appearing in logs, consistent with existing sanitization at lines 120, 155, 222, 234, 376, 396, 427, 482, and 767.