Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mask sensitive information in logs #551

Merged
merged 5 commits into from
May 13, 2024
Merged

Mask sensitive information in logs #551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
309d806
Masked username in scimUserManager
ashanthamara May 8, 2024
3fb8db7
Mask username in SCIMUserOperationListener
ashanthamara May 8, 2024
848728b
Minor fix
ashanthamara May 13, 2024
d7f04fb
Removed unused import
ashanthamara May 13, 2024
e6f0491
Addressed comments
ashanthamara May 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 41 additions & 28 deletions ...cim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java
View file
Open in desktop
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
Open in desktop
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
Loading