Skip to content

Audit entries rework #19345

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

Merged
merged 8 commits into from
Jun 6, 2025
Merged
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -11,13 +11,13 @@ internal static class AuditLogBuilderExtensions
internal static IUmbracoBuilder AddAuditLogs(this IUmbracoBuilder builder)
{
builder.Services.AddTransient<IAuditLogPresentationFactory, AuditLogPresentationFactory>();
builder.AddNotificationHandler<UserLoginSuccessNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationHandler<UserLogoutSuccessNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationHandler<UserLoginFailedNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationHandler<UserForgotPasswordRequestedNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationHandler<UserForgotPasswordChangedNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationHandler<UserPasswordChangedNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationHandler<UserPasswordResetNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationAsyncHandler<UserLoginSuccessNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationAsyncHandler<UserLogoutSuccessNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationAsyncHandler<UserLoginFailedNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationAsyncHandler<UserForgotPasswordRequestedNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationAsyncHandler<UserForgotPasswordChangedNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationAsyncHandler<UserPasswordChangedNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationAsyncHandler<UserPasswordResetNotification, BackOfficeUserManagerAuditer>();

return builder;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
using System.Globalization;

Check notice on line 1 in src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v17/dev)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 52.38% to 41.38%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.

Check notice on line 1 in src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v17/dev)

✅ No longer an issue: String Heavy Function Arguments

The ratio of strings in function arguments is no longer above the threshold
using System.Text;
using Microsoft.Extensions.DependencyInjection;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Models.Membership;
using Umbraco.Cms.Core.Notifications;
@@ -12,131 +16,139 @@
/// Binds to notifications to write audit logs for the <see cref="BackOfficeUserManager" />
/// </summary>
internal sealed class BackOfficeUserManagerAuditer :
INotificationHandler<UserLoginSuccessNotification>,
INotificationHandler<UserLogoutSuccessNotification>,
INotificationHandler<UserLoginFailedNotification>,
INotificationHandler<UserForgotPasswordRequestedNotification>,
INotificationHandler<UserForgotPasswordChangedNotification>,
INotificationHandler<UserPasswordChangedNotification>,
INotificationHandler<UserPasswordResetNotification>
INotificationAsyncHandler<UserLoginSuccessNotification>,
INotificationAsyncHandler<UserLogoutSuccessNotification>,
INotificationAsyncHandler<UserLoginFailedNotification>,
INotificationAsyncHandler<UserForgotPasswordRequestedNotification>,
INotificationAsyncHandler<UserForgotPasswordChangedNotification>,
INotificationAsyncHandler<UserPasswordChangedNotification>,
INotificationAsyncHandler<UserPasswordResetNotification>
{
private readonly IAuditService _auditService;
private readonly IAuditEntryService _auditEntryService;
private readonly IUserService _userService;

public BackOfficeUserManagerAuditer(IAuditService auditService, IUserService userService)
/// <summary>
/// Initializes a new instance of the <see cref="BackOfficeUserManagerAuditer"/> class.
/// </summary>
/// <param name="auditEntryService">The audit entry service.</param>
/// <param name="userService">The user service.</param>
public BackOfficeUserManagerAuditer(
IAuditEntryService auditEntryService,
IUserService userService)
{
_auditService = auditService;
_auditEntryService = auditEntryService;
_userService = userService;
}

public void Handle(UserForgotPasswordChangedNotification notification) =>
/// <inheritdoc />
public Task HandleAsync(UserForgotPasswordChangedNotification notification, CancellationToken cancellationToken) =>
WriteAudit(
notification.PerformingUserId,
notification.AffectedUserId,
notification.IpAddress,
"umbraco/user/password/forgot/change",
"password forgot/change");

public void Handle(UserForgotPasswordRequestedNotification notification) =>
/// <inheritdoc />
public Task HandleAsync(UserForgotPasswordRequestedNotification notification, CancellationToken cancellationToken) =>
WriteAudit(
notification.PerformingUserId,
notification.AffectedUserId,
notification.IpAddress,
"umbraco/user/password/forgot/request",
"password forgot/request");

public void Handle(UserLoginFailedNotification notification) =>
/// <inheritdoc />
public Task HandleAsync(UserLoginFailedNotification notification, CancellationToken cancellationToken) =>
WriteAudit(
notification.PerformingUserId,
null,
notification.IpAddress,
"umbraco/user/sign-in/failed",
"login failed");

public void Handle(UserLoginSuccessNotification notification)
/// <inheritdoc />
public Task HandleAsync(UserLoginSuccessNotification notification, CancellationToken cancellationToken)
=> WriteAudit(
notification.PerformingUserId,
notification.AffectedUserId,
notification.IpAddress,
"umbraco/user/sign-in/login",
"login success");

public void Handle(UserLogoutSuccessNotification notification)
/// <inheritdoc />
public Task HandleAsync(UserLogoutSuccessNotification notification, CancellationToken cancellationToken)
=> WriteAudit(
notification.PerformingUserId,
notification.AffectedUserId,
notification.IpAddress,
"umbraco/user/sign-in/logout",
"logout success");

public void Handle(UserPasswordChangedNotification notification) =>
/// <inheritdoc />
public Task HandleAsync(UserPasswordChangedNotification notification, CancellationToken cancellationToken) =>
WriteAudit(
notification.PerformingUserId,
notification.AffectedUserId,
notification.IpAddress,
"umbraco/user/password/change",
"password change");

public void Handle(UserPasswordResetNotification notification) =>
/// <inheritdoc />
public Task HandleAsync(UserPasswordResetNotification notification, CancellationToken cancellationToken) =>
WriteAudit(
notification.PerformingUserId,
notification.AffectedUserId,
notification.IpAddress,
"umbraco/user/password/reset",
"password reset");

private static string FormatEmail(IMembershipUser? user) =>
user is null ? string.Empty : user.Email.IsNullOrWhiteSpace() ? string.Empty : $"<{user.Email}>";

private void WriteAudit(
private async Task WriteAudit(
string performingId,
string? affectedId,
string ipAddress,
string eventType,
string eventDetails)
{
int? performingIdAsInt = ParseUserId(performingId);
int? affectedIdAsInt = ParseUserId(affectedId);
var performingIdAsInt = ParseUserId(performingId);
var affectedIdAsInt = ParseUserId(affectedId);

WriteAudit(performingIdAsInt, affectedIdAsInt, ipAddress, eventType, eventDetails);
await WriteAudit(performingIdAsInt, affectedIdAsInt, ipAddress, eventType, eventDetails);
}

private static int? ParseUserId(string? id)
=> int.TryParse(id, NumberStyles.Integer, CultureInfo.InvariantCulture, out var isAsInt) ? isAsInt : null;

private void WriteAudit(
private async Task WriteAudit(
int? performingId,
int? affectedId,
string ipAddress,
string eventType,
string eventDetails)
{
var performingDetails = "User UNKNOWN:0";
if (performingId.HasValue)
{
IUser? performingUser = _userService.GetUserById(performingId.Value);
performingDetails = performingUser is null
? $"User UNKNOWN:{performingId.Value}"
: $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}";
}

var affectedDetails = "User UNKNOWN:0";
if (affectedId.HasValue)
{
IUser? affectedUser = _userService.GetUserById(affectedId.Value);
affectedDetails = affectedUser is null
? $"User UNKNOWN:{affectedId.Value}"
: $"User \"{affectedUser.Name}\" {FormatEmail(affectedUser)}";
}
IUser? performingUser = performingId is not null ? _userService.GetUserById(performingId.Value) : null;
IUser? affectedUser = affectedId is not null ? _userService.GetUserById(affectedId.Value) : null;

_auditService.Write(
performingId ?? 0,
performingDetails,
await _auditEntryService.WriteAsync(
performingUser?.Key,
FormatDetails(performingId, performingUser),
ipAddress,
DateTime.UtcNow,
affectedId ?? 0,
affectedDetails,
affectedUser?.Key,
FormatDetails(affectedId, affectedUser),
eventType,
eventDetails);
}

private static string FormatDetails(int? id, IUser? user)
{
if (user == null)
{
return $"User UNKNOWN:{id ?? 0}";
}

return user.Email.IsNullOrWhiteSpace()
? $"User \"{user.Name}\""
: $"User \"{user.Name}\" <{user.Email}>";
}
}
Loading