Skip to content

Commit

Permalink
Improve user authentication procedure (#125)
Browse files Browse the repository at this point in the history
* Update the user email if changed in the external login provider
* Search for user by Object ID before email address
* Add auditing dates to `ApplicationUser`
* Track most recent login date
* Mask private data when logging
  • Loading branch information
dougwaldron authored Nov 5, 2024
1 parent adc4c39 commit 627dfd8
Show file tree
Hide file tree
Showing 12 changed files with 104 additions and 144 deletions.
1 change: 1 addition & 0 deletions src/AppServices/AppServices.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<PackageReference Include="FluentValidation.AspNetCore" />
<PackageReference Include="GaEpd.FileService" />
<PackageReference Include="JetBrains.Annotations" />
<PackageReference Include="Microsoft.Identity.Web" />
<PackageReference Include="SonarAnalyzer.CSharp">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
Expand Down
10 changes: 8 additions & 2 deletions src/AppServices/Permissions/Helpers/PrincipalExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,14 @@ namespace MyApp.AppServices.Permissions.Helpers;

public static class PrincipalExtensions
{
public static string? GetUserIdValue(this ClaimsPrincipal principal) =>
principal.FindFirstValue(ClaimTypes.NameIdentifier);
public static string? GetEmail(this ClaimsPrincipal principal) =>
principal.FindFirstValue(ClaimTypes.Email);

public static string GetGivenName(this ClaimsPrincipal principal) =>
principal.FindFirstValue(ClaimTypes.GivenName) ?? string.Empty;

public static string GetFamilyName(this ClaimsPrincipal principal) =>
principal.FindFirstValue(ClaimTypes.Surname) ?? string.Empty;

public static bool HasRealClaim(this ClaimsPrincipal principal, string type, [NotNullWhen(true)] string? value) =>
value is not null && principal.HasClaim(type, value);
Expand Down
103 changes: 0 additions & 103 deletions src/AppServices/Staff/ClaimConstants.cs

This file was deleted.

1 change: 1 addition & 0 deletions src/AppServices/Staff/Dto/StaffViewDto.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public record StaffViewDto : INamedEntity
public string? PhoneNumber { get; init; }
public OfficeViewDto? Office { get; init; }
public bool Active { get; init; }
public string? ObjectIdentifier { get; init; }

// Display properties
[JsonIgnore]
Expand Down
5 changes: 3 additions & 2 deletions src/AppServices/Staff/StaffService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
using GaEpd.AppLibrary.Pagination;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Identity;
using Microsoft.Identity.Web;
using MyApp.AppServices.Permissions;
using MyApp.AppServices.Permissions.Helpers;
using MyApp.AppServices.Staff.Dto;
using MyApp.AppServices.UserServices;
using MyApp.Domain.Entities.Offices;
using MyApp.Domain.Identity;
using System.Security.Claims;

namespace MyApp.AppServices.Staff;

Expand Down Expand Up @@ -125,7 +125,7 @@ async Task<IdentityResult> UpdateUserRoleAsync(ApplicationUser u, string r, bool
public async Task<IdentityResult> UpdateAsync(string id, StaffUpdateDto resource)
{
var principal = userService.GetCurrentPrincipal()!;
if (id != principal.FindFirstValue(ClaimConstants.NameIdentifierId) &&
if (id != principal.GetNameIdentifierId() &&
!await authorization.Succeeded(principal, Policies.UserAdministrator).ConfigureAwait(false))
{
throw new InsufficientPermissionsException(nameof(Policies.UserAdministrator));
Expand All @@ -139,6 +139,7 @@ public async Task<IdentityResult> UpdateAsync(string id, StaffUpdateDto resource
? null
: await officeRepository.GetAsync(resource.OfficeId.Value).ConfigureAwait(false);
user.Active = resource.Active;
user.ProfileUpdatedAt = DateTimeOffset.UtcNow;

return await userManager.UpdateAsync(user).ConfigureAwait(false);
}
Expand Down
6 changes: 6 additions & 0 deletions src/Domain/Identity/ApplicationUser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ public class ApplicationUser : IdentityUser, IEntity<string>
[StringLength(36)]
public string? ObjectIdentifier { get; set; }

// Auditing properties
public DateTimeOffset? AccountCreatedAt { get; init; }
public DateTimeOffset? AccountUpdatedAt { get; set; }
public DateTimeOffset? ProfileUpdatedAt { get; set; }
public DateTimeOffset? MostRecentLogin { get; set; }

// Display properties
public string SortableFullName =>
string.Join(", ", new[] { FamilyName, GivenName }.Where(s => !string.IsNullOrEmpty(s)));
Expand Down
95 changes: 62 additions & 33 deletions src/WebApp/Pages/Account/ExternalLogin.cshtml.cs
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.OpenIdConnect;
using Microsoft.AspNetCore.Identity;
using Microsoft.EntityFrameworkCore;
using Microsoft.Identity.Web;
using MyApp.AppServices.Permissions.Helpers;
using MyApp.AppServices.Staff;
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;
using System.Security.Claims;

namespace MyApp.WebApp.Pages.Account;

Expand Down Expand Up @@ -82,18 +85,27 @@ public async Task<IActionResult> OnGetCallbackAsync(string? returnUrl = null, st
if (externalLoginInfo?.Principal is null)
return RedirectToLoginPageWithError("Error loading work account information.");

var preferredUserName = externalLoginInfo.Principal.FindFirstValue(ClaimConstants.PreferredUserName);
if (preferredUserName is null)
var userTenant = externalLoginInfo.Principal.GetTenantId();
var userEmail = externalLoginInfo.Principal.GetEmail();
if (userEmail is null || userTenant is null)
return RedirectToLoginPageWithError("Error loading detailed work account information.");

if (!preferredUserName.IsValidEmailDomain())
if (!userEmail.IsValidEmailDomain())
{
logger.LogWarning("User {UserName} with invalid email domain attempted signin", preferredUserName);
logger.LogWarning("User {UserName} with invalid email domain attempted signin", userEmail.MaskEmail());
return RedirectToPage("./Unavailable");
}

// Determine if a user account already exists.
var user = await userManager.FindByNameAsync(preferredUserName);
logger.LogInformation("User {UserName} in tenant {TenantID} successfully authenticated", userEmail.MaskEmail(),
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.FindByNameAsync(userEmail);

// If the user does not have a local account yet, then create one and sign in.
if (user is null)
Expand All @@ -102,7 +114,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 {UserName} attempted signin", preferredUserName);
logger.LogWarning("Inactive user {Email} attempted signin", userEmail.MaskEmail());
return RedirectToPage("./Unavailable");
}

Expand Down Expand Up @@ -138,37 +150,38 @@ private async Task<IActionResult> CreateUserAndSignInAsync(ExternalLoginInfo inf
{
var user = new ApplicationUser
{
UserName = info.Principal.FindFirstValue(ClaimConstants.PreferredUserName),
Email = info.Principal.FindFirstValue(ClaimTypes.Email) ??
info.Principal.FindFirstValue(ClaimConstants.PreferredUserName),
GivenName = info.Principal.FindFirstValue(ClaimTypes.GivenName) ?? "",
FamilyName = info.Principal.FindFirstValue(ClaimTypes.Surname) ?? "",
ObjectIdentifier = info.Principal.FindFirstValue(ClaimConstants.ObjectId),
UserName = info.Principal.GetDisplayName(),
Email = info.Principal.GetEmail(),
GivenName = info.Principal.GetGivenName(),
FamilyName = info.Principal.GetFamilyName(),
ObjectIdentifier = info.Principal.GetObjectId(),
AccountCreatedAt = DateTimeOffset.Now,
MostRecentLogin = DateTimeOffset.Now,
};

// Create the user in the backing store.
var createUserResult = await userManager.CreateAsync(user);
if (!createUserResult.Succeeded)
{
logger.LogWarning("Failed to create new user {UserName}", user.UserName);
logger.LogWarning("Failed to create new user {UserName}", user.Email.MaskEmail());
return await FailedLoginAsync(createUserResult, user);
}

logger.LogInformation("Created new user {UserName} with object ID {ObjectId}", user.UserName,
logger.LogInformation("Created new user {Email} with object ID {ObjectId}", user.Email.MaskEmail(),
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 {UserName}", user.UserName);
logger.LogInformation("Seeding staff role for new user {Email}", user.Email.MaskEmail());
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 {UserName}", user.UserName);
logger.LogInformation("Seeding all roles for new user {Email}", user.Email.MaskEmail());
foreach (var role in AppRole.AllRoles) await userManager.AddToRoleAsync(user, role.Key);
}

Expand All @@ -181,13 +194,31 @@ 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 {UserName} logged in with {LoginProvider} provider",
user.UserName, info.LoginProvider);
user.Email = info.Principal.FindFirstValue(ClaimTypes.Email) ??
info.Principal.FindFirstValue(ClaimConstants.PreferredUserName);
user.GivenName = info.Principal.FindFirstValue(ClaimTypes.GivenName) ?? user.GivenName;
user.FamilyName = info.Principal.FindFirstValue(ClaimTypes.Surname) ?? user.FamilyName;
logger.LogInformation("Existing user {Email} logged in with {LoginProvider} provider",
user.Email.MaskEmail(), info.LoginProvider);

var previousValues = new ApplicationUser
{
UserName = user.UserName,
Email = user.Email,
GivenName = user.GivenName,
FamilyName = user.FamilyName,
};

user.UserName = info.Principal.GetDisplayName();
user.Email = info.Principal.GetEmail();
user.GivenName = info.Principal.GetGivenName();
user.FamilyName = info.Principal.GetFamilyName();
user.MostRecentLogin = DateTimeOffset.Now;

if (user.UserName != previousValues.UserName || user.Email != previousValues.Email ||
user.GivenName != previousValues.GivenName || user.FamilyName != previousValues.FamilyName)
{
user.AccountUpdatedAt = DateTimeOffset.Now;
}

await userManager.UpdateAsync(user);

await signInManager.RefreshSignInAsync(user);
return LocalRedirectOrHome();
}
Expand All @@ -200,19 +231,17 @@ private async Task<IActionResult> AddLoginProviderAndSignInAsync(

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

if (user.ObjectIdentifier == null)
{
user.ObjectIdentifier = info.Principal.FindFirstValue(ClaimConstants.ObjectId);
await userManager.UpdateAsync(user);
}
user.ObjectIdentifier ??= info.Principal.GetObjectId();
user.MostRecentLogin = DateTimeOffset.Now;
await userManager.UpdateAsync(user);

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

// Include the access token in the properties.
var props = new AuthenticationProperties();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
using MyApp.WebApp.Platform.Settings;
using System.Collections;

namespace MyApp.WebApp.Platform.ErrorLogging;
namespace MyApp.WebApp.Platform.Logging;

public class ErrorLogger(IFileService fileService, IServiceProvider serviceProvider) : IErrorLogger
{
Expand Down
20 changes: 20 additions & 0 deletions src/WebApp/Platform/Logging/Redaction.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using System.Text.RegularExpressions;

namespace MyApp.WebApp.Platform.Logging;

public static partial class Redaction
{
public static string MaskEmail(this string? email)
{
if (string.IsNullOrEmpty(email)) return string.Empty;

var atIndex = email.IndexOf('@');
if (atIndex <= 1) return email;

var maskedEmail = MyRegex().Replace(email[..atIndex], "*");
return string.Concat(maskedEmail, email.AsSpan(atIndex));
}

[GeneratedRegex(".(?=.{2})")]
private static partial Regex MyRegex();
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using System.Text;

namespace MyApp.WebApp.Platform.ErrorLogging;
namespace MyApp.WebApp.Platform.Logging;

public static class ShortId
{
Expand Down
Loading

0 comments on commit 627dfd8

Please sign in to comment.