Skip to content

Commit

Permalink
Avoid logging email, even masked
Browse files Browse the repository at this point in the history
Fix code scanning alert: Exposure of private information

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
  • Loading branch information
1 parent e4b54e8 commit 1ba0cdb
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 38 deletions.
34 changes: 16 additions & 18 deletions src/WebApp/Pages/Account/ExternalLogin.cshtml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using MyApp.AppServices.Staff.Dto;
using MyApp.Domain.Identity;
using MyApp.WebApp.Models;
using MyApp.WebApp.Platform.Logging;
using MyApp.WebApp.Platform.PageModelHelpers;
using MyApp.WebApp.Platform.Settings;

Expand Down Expand Up @@ -94,19 +93,19 @@ public async Task<IActionResult> OnGetCallbackAsync(string? returnUrl = null, st

if (!userEmail.IsValidEmailDomain())
{
logger.LogWarning("User {UserName} with invalid email domain attempted signin", userEmail.MaskEmail());
logger.LogWarning("User with invalid email domain {Domain} attempted signin", userEmail.Split('@')[1]);

Check warning

Code scanning / CodeQL

Exposure of private information Medium

Private data returned by
call to method GetEmail
is written to an external location.
Private data returned by
access to local variable userEmail
is written to an external location.
Private data returned by
access to local variable userEmail
is written to an external location.
Private data returned by
access to local variable userEmail
is written to an external location.
return RedirectToPage("./Unavailable");
}

logger.LogInformation("User {UserName} in tenant {TenantID} successfully authenticated", userEmail.MaskEmail(),
userTenant);
logger.LogInformation("User with object ID {ObjectId} in tenant {TenantID} successfully authenticated",
externalLoginInfo.Principal.GetObjectId(), userTenant);

// Determine if a user account already exists with the Object ID.
// If not, then determine if a user account already exists with the given username.
var user = AppSettings.DevSettings.UseInMemoryData
? await userManager.FindByNameAsync(userEmail)
: await userManager.Users.SingleOrDefaultAsync(u =>
u.ObjectIdentifier == externalLoginInfo.Principal.GetObjectId()) ??
: await userManager.Users.SingleOrDefaultAsync(au =>
au.ObjectIdentifier == externalLoginInfo.Principal.GetObjectId()) ??
await userManager.FindByNameAsync(userEmail);

// If the user does not have a local account yet, then create one and sign in.
Expand All @@ -116,7 +115,7 @@ public async Task<IActionResult> OnGetCallbackAsync(string? returnUrl = null, st
// If user has been marked as inactive, don't sign in.
if (!user.Active)
{
logger.LogWarning("Inactive user {Email} attempted signin", userEmail.MaskEmail());
logger.LogWarning("Inactive user with object ID {ObjectId} attempted signin", user.ObjectIdentifier);
return RedirectToPage("./Unavailable");
}

Expand Down Expand Up @@ -165,25 +164,24 @@ private async Task<IActionResult> CreateUserAndSignInAsync(ExternalLoginInfo inf
var createUserResult = await userManager.CreateAsync(user);
if (!createUserResult.Succeeded)
{
logger.LogWarning("Failed to create new user {UserName}", user.Email.MaskEmail());
logger.LogWarning("Failed to create new user with object ID {ObjectId}", user.ObjectIdentifier);
return await FailedLoginAsync(createUserResult, user);
}

logger.LogInformation("Created new user {Email} with object ID {ObjectId}",
user.Email.MaskEmail(), user.ObjectIdentifier);
logger.LogInformation("Created new user with object ID {ObjectId}", user.ObjectIdentifier);

// Add new user to application Roles if seeded in app settings or local admin user setting is enabled.
var seedAdminUsers = configuration.GetSection("SeedAdminUsers").Get<string[]>();
if (AppSettings.DevSettings.LocalUserIsStaff)
{
logger.LogInformation("Seeding staff role for new user {Email}", user.Email.MaskEmail());
logger.LogInformation("Seeding staff role for new user with object ID {ObjectId}", user.ObjectIdentifier);
await userManager.AddToRoleAsync(user, RoleName.Staff);
}

if (AppSettings.DevSettings.LocalUserIsAdmin ||
(seedAdminUsers != null && seedAdminUsers.Contains(user.Email, StringComparer.InvariantCultureIgnoreCase)))
{
logger.LogInformation("Seeding all roles for new user {Email}", user.Email.MaskEmail());
logger.LogInformation("Seeding all roles for new user with object ID {ObjectId}", user.ObjectIdentifier);
foreach (var role in AppRole.AllRoles) await userManager.AddToRoleAsync(user, role.Key);
}

Expand All @@ -196,8 +194,8 @@ private async Task<IActionResult> CreateUserAndSignInAsync(ExternalLoginInfo inf
// Update local store with from external provider.
private async Task<IActionResult> RefreshUserInfoAndSignInAsync(ApplicationUser user, ExternalLoginInfo info)
{
logger.LogInformation("Existing user {Email} logged in with {LoginProvider} provider",
user.Email.MaskEmail(), info.LoginProvider);
logger.LogInformation("Existing user with object ID {ObjectId} logged in with {LoginProvider} provider",
user.ObjectIdentifier, info.LoginProvider);

var previousValues = new ApplicationUser
{
Expand Down Expand Up @@ -232,17 +230,17 @@ private async Task<IActionResult> AddLoginProviderAndSignInAsync(

if (!addLoginResult.Succeeded)
{
logger.LogWarning("Failed to add login provider {LoginProvider} for user {Email}",
info.LoginProvider, user.Email.MaskEmail());
logger.LogWarning("Failed to add login provider {LoginProvider} for user with object ID {ObjectId}",
info.LoginProvider, user.ObjectIdentifier);
return await FailedLoginAsync(addLoginResult, user);
}

user.ObjectIdentifier ??= info.Principal.GetObjectId();
user.MostRecentLogin = DateTimeOffset.Now;
await userManager.UpdateAsync(user);

logger.LogInformation("Login provider {LoginProvider} added for user {Email} with object ID {ObjectId}",
info.LoginProvider, user.Email.MaskEmail(), user.ObjectIdentifier);
logger.LogInformation("Login provider {LoginProvider} added for user with object ID {ObjectId}",
info.LoginProvider, user.ObjectIdentifier);

// Include the access token in the properties.
var props = new AuthenticationProperties();
Expand Down
20 changes: 0 additions & 20 deletions src/WebApp/Platform/Logging/Redaction.cs

This file was deleted.

0 comments on commit 1ba0cdb

Please sign in to comment.