Skip to content

Commit

Permalink
Mask sensitive information in logs (#551)
Browse files Browse the repository at this point in the history 
* Masked username in scimUserManager

* Mask username in SCIMUserOperationListener

* Minor fix

* Removed unused import

* Addressed comments
  • Loading branch information
@ashanthamara
ashanthamara authored May 13, 2024
1 parent 611bdac commit adb835c
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 29 deletions.
69 changes: 41 additions & 28 deletions ...cim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,8 @@ public User createUser(User user, Map<String, Boolean> requiredAttributes)
}

if (isExistingUser) {
String error = "User with the name: " + user.getUserName() + " already exists in the system.";
String error = "User with the name: " + maskIfRequired(user.getUserName()) +
" already exists in the system.";
throw new ConflictException(error);
}

Expand Down Expand Up @@ -347,8 +348,8 @@ public User createUser(User user, Map<String, Boolean> requiredAttributes)
// Set the schemas of the SCIM user.
user.setSchemas(this);
} catch (UserStoreClientException e) {
String errorMessage = String.format("Error in adding the user: " + user.getUserName() + ". %s",
e.getMessage());
String errorMessage = String.format("Error in adding the user: " + maskIfRequired(user.getUserName()) +
". %s", e.getMessage());
if (log.isDebugEnabled()) {
log.debug(errorMessage, e);
}
Expand All @@ -361,8 +362,8 @@ public User createUser(User user, Map<String, Boolean> requiredAttributes)
// Therefore checking for possible client exception.
Throwable ex = ExceptionUtils.getRootCause(e);
if (ex instanceof UserStoreClientException) {
String errorMessage = String.format("Error in adding the user: " + user.getUserName() + ". %s",
ex.getMessage());
String errorMessage = String.format("Error in adding the user: " + maskIfRequired(user.getUserName())
+ ". %s", ex.getMessage());
if (log.isDebugEnabled()) {
log.debug(errorMessage, ex);
}
Expand Down Expand Up @@ -533,17 +534,18 @@ public void deleteUser(String userId) throws NotFoundException, CharonException,
} else if (userStoreDomainFromSP != null &&
!(userStoreDomainFromSP
.equalsIgnoreCase(coreUser.getUserStoreDomain()))) {
throw new CharonException("User :" + coreUser.getUsername() + "is not belong to user store " +
userStoreDomainFromSP + "Hence user updating fail");
throw new CharonException("User : " + maskIfRequired(coreUser.getUsername()) + " is not belong to " +
"user store " + userStoreDomainFromSP + ". Hence user updating fail.");
} else {
// We assume (since id is unique per user) only one user exists for a given id.
userName = coreUser.getUsername();
String userStoreDomainName = coreUser.getUserStoreDomain();

// Check if SCIM is enabled for the user store.
if (!isSCIMEnabled(userStoreDomainName)) {
throw new CharonException("Cannot delete user: " + userName + " through SCIM from user store: " +
userStoreDomainName + ". SCIM is not enabled for user store: " + userStoreDomainName);
throw new CharonException("Cannot delete user: " + maskIfRequired(userName) + " through SCIM from" +
" user store: " + userStoreDomainName + ". SCIM is not enabled for user store: " +
userStoreDomainName);
}
carbonUM.deleteUserWithID(coreUser.getUserID());
if (log.isDebugEnabled()) {
Expand Down Expand Up @@ -1017,8 +1019,8 @@ public User updateUser(User user, Map<String, Boolean> requiredAttributes) throw
User oldUser = this.getUser(user.getId(), ResourceManagerUtil.getAllAttributeURIs(schema));
if (userStoreDomainFromSP != null && !userStoreDomainFromSP
.equalsIgnoreCase(IdentityUtil.extractDomainFromName(oldUser.getUserName()))) {
throw new CharonException("User :" + oldUser.getUserName() + "is not belong to user store " +
userStoreDomainFromSP + "Hence user updating fail");
throw new CharonException("User : " + maskIfRequired(oldUser.getUserName()) + " is not belong to " +
"user store " + userStoreDomainFromSP + ". Hence user updating fail.");
}
if (getUserStoreDomainFromSP() != null &&
!UserCoreConstants.PRIMARY_DEFAULT_DOMAIN_NAME.equalsIgnoreCase(getUserStoreDomainFromSP())) {
Expand Down Expand Up @@ -1132,8 +1134,7 @@ public User updateUser(User user, Map<String, Boolean> requiredAttributes) throw
}
throw new BadRequestException(errorMessage, ResponseCodeConstants.INVALID_VALUE);
} catch (UserStoreException e) {
String errMsg = "Error while updating attributes of user: " + (LoggerUtils.isLogMaskingEnable ?
LoggerUtils.getMaskedContent(user.getUserName()) : user.getUserName());
String errMsg = "Error while updating attributes of user: " + maskIfRequired(user.getUserName());
// Sometimes client exceptions are wrapped in the super class.
// Therefore checking for possible client exception.
Throwable ex = ExceptionUtils.getRootCause(e);
Expand Down Expand Up @@ -1207,9 +1208,8 @@ public User updateUser(User user, Map<String, Boolean> requiredAttributes,
if (userStoreDomainFromSP != null) {
if (!userStoreDomainFromSP
.equalsIgnoreCase(IdentityUtil.extractDomainFromName(oldUser.getUserName()))) {
String errorMessage =
String.format("User : %s does not belong to userstore %s. Hence user updating failed",
oldUser.getUserName(), userStoreDomainFromSP);
String errorMessage = String.format("User : %s does not belong to userstore %s. Hence user " +
"updating failed.", maskIfRequired(oldUser.getUserName()), userStoreDomainFromSP);
throw new CharonException(errorMessage);
}
if (!UserCoreConstants.PRIMARY_DEFAULT_DOMAIN_NAME.equalsIgnoreCase(userStoreDomainFromSP)) {
Expand Down Expand Up @@ -1313,7 +1313,8 @@ public User updateUser(User user, Map<String, Boolean> requiredAttributes,
return getUser(user.getId(), requiredAttributes);
} catch (UserStoreException e) {
handleErrorsOnUserNameAndPasswordPolicy(e);
throw resolveError(e, "Error while updating attributes of user: " + user.getUserName());
throw resolveError(e, "Error while updating attributes of user: " +
maskIfRequired(user.getUserName()));
} catch (BadRequestException e) {
/*
This is needed as most BadRequests are thrown to charon as
Expand All @@ -1322,9 +1323,11 @@ public User updateUser(User user, Map<String, Boolean> requiredAttributes,
the end party.
*/
reThrowMutabilityBadRequests(e);
throw new CharonException("Error occurred while trying to update the user: " + user.getUserName(), e);
throw new CharonException("Error occurred while trying to update the user: " +
maskIfRequired(user.getUserName()), e);
} catch (CharonException e) {
throw new CharonException("Error occurred while trying to update the user: " + user.getUserName(), e);
throw new CharonException("Error occurred while trying to update the user: " +
maskIfRequired(user.getUserName()), e);
}
}

Expand Down Expand Up @@ -1522,10 +1525,8 @@ private String resolveDomainName(String domainName, ExpressionNode node) throws
// Extract the domain name if the domain name is embedded in the filter attribute value.
domainName = resolveDomainNameInAttributeValue(domainName, node);
} catch (BadRequestException e) {
String errorMessage = String
.format("Domain parameter: %s in request does not match with the domain name in the attribute "
+ "value: %s ", domainName, (LoggerUtils.isLogMaskingEnable ?
LoggerUtils.getMaskedContent(node.getValue()) : node.getValue()));
String errorMessage = String.format("Domain parameter: %s in request does not match with the domain " +
"name in the attribute value: %s", domainName, maskIfRequired(node.getValue()));
throw new CharonException(errorMessage, e);
}
// Get domain name according to Filter Enhancements properties as in identity.xml
Expand Down Expand Up @@ -4183,7 +4184,7 @@ private Set<User> getSCIMUsers(Set<org.wso2.carbon.user.core.common.User> users,
attributes = SCIMCommonUtils.convertLocalToSCIMDialect(userClaimValues, scimToLocalClaimsMap);
} catch (UserStoreException e) {
throw resolveError(e, "Error in converting local claims to SCIM dialect for user: "
+ user.getUsername());
+ maskIfRequired(user.getUsername()));
}

try {
Expand Down Expand Up @@ -4294,10 +4295,11 @@ private Set<User> getSCIMUsers(Set<org.wso2.carbon.user.core.common.User> users,
}

} catch (UserStoreException e) {
throw resolveError(e, "Error in getting user information for user: " + user.getUsername());
} catch (CharonException | NotFoundException | IdentitySCIMException |
BadRequestException e) {
throw new CharonException("Error in getting user information for user: " + user.getUsername(), e);
throw resolveError(e, "Error in getting user information for user: " +
maskIfRequired(user.getUsername()));
} catch (CharonException | NotFoundException | IdentitySCIMException | BadRequestException e) {
throw new CharonException("Error in getting user information for user: " +
maskIfRequired(user.getUsername()), e);
}

if (scimUser != null) {
Expand Down Expand Up @@ -6414,4 +6416,15 @@ private void publishEvent(User user, String eventName, boolean isAdminUpdate)
throw new BadRequestException("Error occurred publishing event", ResponseCodeConstants.INVALID_VALUE);
}
}

/**
* Mask the given value if it is required.
*
* @param value Value to be masked.
* @return Masked/unmasked value.
*/
private String maskIfRequired(String value) {

return LoggerUtils.isLogMaskingEnable ? LoggerUtils.getMaskedContent(value) : value;
}
}
4 changes: 3 additions & 1 deletion ...c/main/java/org/wso2/carbon/identity/scim2/common/listener/SCIMUserOperationListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.wso2.carbon.identity.application.authentication.framework.store.UserSessionStore;
import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkConstants;
import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkUtils;
import org.wso2.carbon.identity.central.log.mgt.utils.LoggerUtils;
import org.wso2.carbon.identity.claim.metadata.mgt.exception.ClaimMetadataException;
import org.wso2.carbon.identity.claim.metadata.mgt.model.LocalClaim;
import org.wso2.carbon.identity.core.AbstractIdentityUserOperationEventListener;
Expand Down Expand Up @@ -434,7 +435,8 @@ private void validateClaimUpdate(String username) throws UserStoreException {
try {
isExistingJITProvisionedUser = UserSessionStore.getInstance().isExistingUser(username);
} catch (UserSessionException e) {
throw new UserStoreException("Error while checking the federated user existence for the user: " + username);
throw new UserStoreException("Error while checking the federated user existence for the user: " +
(LoggerUtils.isLogMaskingEnable ? LoggerUtils.getMaskedContent(username) : username));
}

// If federated user is already provisioned, block that user's synced attribute editing.
Expand Down

0 comments on commit adb835c

Please sign in to comment.