-
Notifications
You must be signed in to change notification settings - Fork 645
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.
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
Remove usernames and emails from AI traces. #5293
Conversation
@@ -756,7 +755,7 @@ private User FindByUserNameOrEmail(string userNameOrEmail) | |||
else | |||
{ | |||
// If multiple matches, leave it null to signal no unique email address | |||
_trace.Warning("Multiple user accounts with email address: " + userNameOrEmail + " found: " + String.Join(", ", allMatches.Select(u => u.Username))); | |||
_trace.Warning("Multiple user accounts with a single email address were found. Count: " + String.Join(", ", allMatches.Select(u => u.Username))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing String.Join(", ", allMatches.Select(u => u.Username))
should be allMatches.Count()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, good catch. Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chenriksson - First comment - thank you - fixed.
Second comment - plain Client IP is not ok - needs to be obfuscated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified the traces for cookie compliance and do not seem to be IPs in the messages.
SecurePushSubscription may have Client IP is ok in AI logs, right? |
Remove usernames and emails from AI traces.