From 263e6f017fe83fb7012a8f992e7bd2f04a860ac2 Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Fri, 16 May 2025 15:17:34 +0200 Subject: [PATCH 01/12] Introduce new AuditEntryService - Moved logic related to the IAuditEntryRepository from the AuditService to the new service - Introduced new Async methods - Using ids (for easier transition from the previous Write method) - Using keys - Moved and updated integration tests related to the audit entries to a new test class `AuditEntryServiceTests` - Added unit tests class `AuditEntryServiceTests` and added a few unit tests - Added migration to add columns for `performingUserKey` and `affectedUserKey` and convert existing user ids - Adjusted usages of the old AuditService.Write method to use the new one (mostly notification handlers) --- .../AuditLogBuilderExtensions.cs | 14 +- .../Security/BackOfficeUserManagerAuditer.cs | 67 ++-- src/Umbraco.Core/Constants-Security.cs | 5 + .../Handlers/AuditNotificationsHandler.cs | 289 +++++++++++------- src/Umbraco.Core/Models/AuditEntry.cs | 16 + src/Umbraco.Core/Models/IAuditEntry.cs | 21 ++ .../Services/AuditEntryService.cs | 248 +++++++++++++++ src/Umbraco.Core/Services/AuditService.cs | 158 +++------- .../Services/IAuditEntryService.cs | 69 +++++ src/Umbraco.Core/Services/IAuditService.cs | 1 + .../Services/IUserIdKeyResolver.cs | 14 + .../AuditEntryOperationStatus.cs | 10 + .../UmbracoBuilder.CoreServices.cs | 18 +- .../UmbracoBuilder.Services.cs | 1 + .../Migrations/Upgrade/UmbracoPlan.cs | 3 + .../V_17_0_0/AddGuidsToAuditEntries.cs | 52 ++++ .../Persistence/Dtos/AuditEntryDto.cs | 8 + .../Factories/AuditEntryFactory.cs | 4 + .../Persistence/Mappers/AuditEntryMapper.cs | 2 + .../Implement/AuditEntryRepository.cs | 13 +- .../Services/Implement/UserIdKeyResolver.cs | 39 ++- .../Builders/AuditEntryBuilder.cs | 21 +- .../Services/AuditEntryServiceTests.cs | 94 ++++++ .../Services/AuditServiceTests.cs | 62 ---- .../Services/AuditEntryServiceTests.cs | 148 +++++++++ 25 files changed, 1043 insertions(+), 334 deletions(-) create mode 100644 src/Umbraco.Core/Services/AuditEntryService.cs create mode 100644 src/Umbraco.Core/Services/IAuditEntryService.cs create mode 100644 src/Umbraco.Core/Services/OperationStatus/AuditEntryOperationStatus.cs create mode 100644 src/Umbraco.Infrastructure/Migrations/Upgrade/V_17_0_0/AddGuidsToAuditEntries.cs create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditEntryServiceTests.cs create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditEntryServiceTests.cs diff --git a/src/Umbraco.Cms.Api.Management/DependencyInjection/AuditLogBuilderExtensions.cs b/src/Umbraco.Cms.Api.Management/DependencyInjection/AuditLogBuilderExtensions.cs index edca89e56bd7..0badbc9f30a3 100644 --- a/src/Umbraco.Cms.Api.Management/DependencyInjection/AuditLogBuilderExtensions.cs +++ b/src/Umbraco.Cms.Api.Management/DependencyInjection/AuditLogBuilderExtensions.cs @@ -11,13 +11,13 @@ internal static class AuditLogBuilderExtensions internal static IUmbracoBuilder AddAuditLogs(this IUmbracoBuilder builder) { builder.Services.AddTransient(); - builder.AddNotificationHandler(); - builder.AddNotificationHandler(); - builder.AddNotificationHandler(); - builder.AddNotificationHandler(); - builder.AddNotificationHandler(); - builder.AddNotificationHandler(); - builder.AddNotificationHandler(); + builder.AddNotificationAsyncHandler(); + builder.AddNotificationAsyncHandler(); + builder.AddNotificationAsyncHandler(); + builder.AddNotificationAsyncHandler(); + builder.AddNotificationAsyncHandler(); + builder.AddNotificationAsyncHandler(); + builder.AddNotificationAsyncHandler(); return builder; } diff --git a/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs b/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs index 561e67bd384f..772e879826a2 100644 --- a/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs +++ b/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs @@ -1,4 +1,7 @@ using System.Globalization; +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,24 +15,32 @@ namespace Umbraco.Cms.Api.Management.Security; /// Binds to notifications to write audit logs for the /// internal sealed class BackOfficeUserManagerAuditer : - INotificationHandler, - INotificationHandler, - INotificationHandler, - INotificationHandler, - INotificationHandler, - INotificationHandler, - INotificationHandler + INotificationAsyncHandler, + INotificationAsyncHandler, + INotificationAsyncHandler, + INotificationAsyncHandler, + INotificationAsyncHandler, + INotificationAsyncHandler, + INotificationAsyncHandler { - private readonly IAuditService _auditService; + private readonly IAuditEntryService _auditEntryService; private readonly IUserService _userService; - public BackOfficeUserManagerAuditer(IAuditService auditService, IUserService userService) + /// + /// Initializes a new instance of the class. + /// + /// The audit entry service. + /// The user service. + public BackOfficeUserManagerAuditer( + IAuditEntryService auditEntryService, + IUserService userService) { - _auditService = auditService; + _auditEntryService = auditEntryService; _userService = userService; } - public void Handle(UserForgotPasswordChangedNotification notification) => + /// + public Task HandleAsync(UserForgotPasswordChangedNotification notification, CancellationToken cancellationToken) => WriteAudit( notification.PerformingUserId, notification.AffectedUserId, @@ -37,7 +48,8 @@ public void Handle(UserForgotPasswordChangedNotification notification) => "umbraco/user/password/forgot/change", "password forgot/change"); - public void Handle(UserForgotPasswordRequestedNotification notification) => + /// + public Task HandleAsync(UserForgotPasswordRequestedNotification notification, CancellationToken cancellationToken) => WriteAudit( notification.PerformingUserId, notification.AffectedUserId, @@ -45,7 +57,8 @@ public void Handle(UserForgotPasswordRequestedNotification notification) => "umbraco/user/password/forgot/request", "password forgot/request"); - public void Handle(UserLoginFailedNotification notification) => + /// + public Task HandleAsync(UserLoginFailedNotification notification, CancellationToken cancellationToken) => WriteAudit( notification.PerformingUserId, null, @@ -53,7 +66,8 @@ public void Handle(UserLoginFailedNotification notification) => "umbraco/user/sign-in/failed", "login failed"); - public void Handle(UserLoginSuccessNotification notification) + /// + public Task HandleAsync(UserLoginSuccessNotification notification, CancellationToken cancellationToken) => WriteAudit( notification.PerformingUserId, notification.AffectedUserId, @@ -61,7 +75,8 @@ public void Handle(UserLoginSuccessNotification notification) "umbraco/user/sign-in/login", "login success"); - public void Handle(UserLogoutSuccessNotification notification) + /// + public Task HandleAsync(UserLogoutSuccessNotification notification, CancellationToken cancellationToken) => WriteAudit( notification.PerformingUserId, notification.AffectedUserId, @@ -69,7 +84,8 @@ public void Handle(UserLogoutSuccessNotification notification) "umbraco/user/sign-in/logout", "logout success"); - public void Handle(UserPasswordChangedNotification notification) => + /// + public Task HandleAsync(UserPasswordChangedNotification notification, CancellationToken cancellationToken) => WriteAudit( notification.PerformingUserId, notification.AffectedUserId, @@ -77,7 +93,8 @@ public void Handle(UserPasswordChangedNotification notification) => "umbraco/user/password/change", "password change"); - public void Handle(UserPasswordResetNotification notification) => + /// + public Task HandleAsync(UserPasswordResetNotification notification, CancellationToken cancellationToken) => WriteAudit( notification.PerformingUserId, notification.AffectedUserId, @@ -88,7 +105,7 @@ public void Handle(UserPasswordResetNotification notification) => 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, @@ -98,43 +115,47 @@ private void WriteAudit( int? performingIdAsInt = ParseUserId(performingId); int? 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) { + Guid? performingKey = null; var performingDetails = "User UNKNOWN:0"; if (performingId.HasValue) { IUser? performingUser = _userService.GetUserById(performingId.Value); + performingKey = performingUser?.Key; performingDetails = performingUser is null ? $"User UNKNOWN:{performingId.Value}" : $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}"; } + Guid? affectedKey = null; var affectedDetails = "User UNKNOWN:0"; if (affectedId.HasValue) { IUser? affectedUser = _userService.GetUserById(affectedId.Value); + affectedKey = affectedUser?.Key; affectedDetails = affectedUser is null ? $"User UNKNOWN:{affectedId.Value}" : $"User \"{affectedUser.Name}\" {FormatEmail(affectedUser)}"; } - _auditService.Write( - performingId ?? 0, + await _auditEntryService.WriteAsync( + performingKey ?? Constants.Security.UnknownUserKey, performingDetails, ipAddress, DateTime.UtcNow, - affectedId ?? 0, + affectedKey ?? Constants.Security.UnknownUserKey, affectedDetails, eventType, eventDetails); diff --git a/src/Umbraco.Core/Constants-Security.cs b/src/Umbraco.Core/Constants-Security.cs index eb77642f1bb9..dba58fcb5704 100644 --- a/src/Umbraco.Core/Constants-Security.cs +++ b/src/Umbraco.Core/Constants-Security.cs @@ -27,6 +27,11 @@ public static class Security /// public const int UnknownUserId = 0; + /// + /// The key for the 'unknown' user. + /// + public static readonly Guid UnknownUserKey = Guid.Empty; + /// /// The name of the 'unknown' user. /// diff --git a/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs b/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs index 0edd21754069..97e60fef655a 100644 --- a/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs +++ b/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs @@ -1,6 +1,9 @@ +using System.Runtime.CompilerServices; using System.Text; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Entities; @@ -15,86 +18,134 @@ namespace Umbraco.Cms.Core.Handlers; public sealed class AuditNotificationsHandler : INotificationHandler, + INotificationAsyncHandler, INotificationHandler, + INotificationAsyncHandler, INotificationHandler, + INotificationAsyncHandler, INotificationHandler, + INotificationAsyncHandler, INotificationHandler, + INotificationAsyncHandler, INotificationHandler, + INotificationAsyncHandler, INotificationHandler, + INotificationAsyncHandler, INotificationHandler, - INotificationHandler + INotificationAsyncHandler, + INotificationHandler, + INotificationAsyncHandler { - private readonly IAuditService _auditService; + private readonly IAuditEntryService _auditEntryService; private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; private readonly IEntityService _entityService; - private readonly GlobalSettings _globalSettings; private readonly IIpResolver _ipResolver; private readonly IMemberService _memberService; private readonly IUserGroupService _userGroupService; private readonly IUserService _userService; public AuditNotificationsHandler( - IAuditService auditService, + IAuditEntryService auditEntryService, IUserService userService, IEntityService entityService, IIpResolver ipResolver, - IOptionsMonitor globalSettings, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IMemberService memberService, IUserGroupService userGroupService) { - _auditService = auditService; + _auditEntryService = auditEntryService; _userService = userService; _entityService = entityService; _ipResolver = ipResolver; _backOfficeSecurityAccessor = backOfficeSecurityAccessor; _memberService = memberService; _userGroupService = userGroupService; - _globalSettings = globalSettings.CurrentValue; } - private IUser CurrentPerformingUser + [Obsolete("Use the non-obsolete constructor instead. Scheduled for removal in V19.")] + public AuditNotificationsHandler( + IAuditService auditService, + IUserService userService, + IEntityService entityService, + IIpResolver ipResolver, + IOptionsMonitor globalSettings, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, + IMemberService memberService, + IUserGroupService userGroupService) + : this( + StaticServiceProvider.Instance.GetRequiredService(), + userService, + entityService, + ipResolver, + backOfficeSecurityAccessor, + memberService, + userGroupService) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled for removal in V19.")] + public AuditNotificationsHandler( + IAuditEntryService auditEntryService, + IAuditService auditService, + IUserService userService, + IEntityService entityService, + IIpResolver ipResolver, + IOptionsMonitor globalSettings, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, + IMemberService memberService, + IUserGroupService userGroupService) + : this( + auditEntryService, + userService, + entityService, + ipResolver, + backOfficeSecurityAccessor, + memberService, + userGroupService) + { + } + + private IUser? CurrentPerformingUser { get { IUser? identity = _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser; IUser? user = identity == null ? null : _userService.GetAsync(identity.Key).GetAwaiter().GetResult(); - return user ?? UnknownUser(_globalSettings); + return user; } } private string PerformingIp => _ipResolver.GetCurrentRequestIpAddress(); - public static IUser UnknownUser(GlobalSettings globalSettings) => new User(globalSettings) - { - Id = Constants.Security.UnknownUserId, - Name = Constants.Security.UnknownUserName, - Email = string.Empty, - }; - - public void Handle(AssignedMemberRolesNotification notification) + /// + public async Task HandleAsync(AssignedMemberRolesNotification notification, CancellationToken cancellationToken) { - IUser performingUser = CurrentPerformingUser; + IUser? performingUser = CurrentPerformingUser; var roles = string.Join(", ", notification.Roles); var members = _memberService.GetAllMembers(notification.MemberIds).ToDictionary(x => x.Id, x => x); foreach (var id in notification.MemberIds) { members.TryGetValue(id, out IMember? member); - _auditService.Write( - performingUser.Id, - $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", - PerformingIp, - DateTime.UtcNow, - -1, + + await Audit( + performingUser, + null, $"Member {id} \"{member?.Name ?? "(unknown)"}\" {FormatEmail(member)}", "umbraco/member/roles/assigned", $"roles modified, assigned {roles}"); } } - public void Handle(AssignedUserGroupPermissionsNotification notification) + [Obsolete("Use HandleAsync() instead. Scheduled for removal in V19.")] + public void Handle(AssignedMemberRolesNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); + + /// + public async Task HandleAsync( + AssignedUserGroupPermissionsNotification notification, + CancellationToken cancellationToken) { - IUser performingUser = CurrentPerformingUser; + IUser? performingUser = CurrentPerformingUser; IEnumerable perms = notification.EntityPermissions; foreach (EntityPermission perm in perms) { @@ -102,113 +153,124 @@ public void Handle(AssignedUserGroupPermissionsNotification notification) var assigned = string.Join(", ", perm.AssignedPermissions); IEntitySlim? entity = _entityService.Get(perm.EntityId); - _auditService.Write( - performingUser.Id, - $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", - PerformingIp, - DateTime.UtcNow, - -1, + await Audit( + performingUser, + null, $"User Group {group?.Id} \"{group?.Name}\" ({group?.Alias})", "umbraco/user-group/permissions-change", $"assigning {(string.IsNullOrWhiteSpace(assigned) ? "(nothing)" : assigned)} on id:{perm.EntityId} \"{entity?.Name}\""); } } - public void Handle(ExportedMemberNotification notification) + [Obsolete("Use HandleAsync() instead. Scheduled for removal in V19.")] + public void Handle(AssignedUserGroupPermissionsNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); + + /// + public async Task HandleAsync(ExportedMemberNotification notification, CancellationToken cancellationToken) { - IUser performingUser = CurrentPerformingUser; + IUser? performingUser = CurrentPerformingUser; IMember member = notification.Member; - _auditService.Write( - performingUser.Id, - $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", - PerformingIp, - DateTime.UtcNow, - -1, + await Audit( + performingUser, + null, $"Member {member.Id} \"{member.Name}\" {FormatEmail(member)}", "umbraco/member/exported", "exported member data"); } - public void Handle(MemberDeletedNotification notification) + [Obsolete("Use HandleAsync() instead. Scheduled for removal in V19.")] + public void Handle(ExportedMemberNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); + + public async Task HandleAsync(MemberDeletedNotification notification, CancellationToken cancellationToken) { - IUser performingUser = CurrentPerformingUser; + IUser? performingUser = CurrentPerformingUser; IEnumerable members = notification.DeletedEntities; foreach (IMember member in members) { - _auditService.Write( - performingUser.Id, - $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", - PerformingIp, - DateTime.UtcNow, - -1, + await Audit( + performingUser, + null, $"Member {member.Id} \"{member.Name}\" {FormatEmail(member)}", "umbraco/member/delete", $"delete member id:{member.Id} \"{member.Name}\" {FormatEmail(member)}"); } } - public void Handle(MemberSavedNotification notification) + [Obsolete("Use HandleAsync() instead. Scheduled for removal in V19.")] + public void Handle(MemberDeletedNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); + + /// + public async Task HandleAsync(MemberSavedNotification notification, CancellationToken cancellationToken) { - IUser performingUser = CurrentPerformingUser; + IUser? performingUser = CurrentPerformingUser; IEnumerable members = notification.SavedEntities; foreach (IMember member in members) { var dp = string.Join(", ", ((Member)member).GetWereDirtyProperties()); - _auditService.Write( - performingUser.Id, - $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", - PerformingIp, - DateTime.UtcNow, - -1, + await Audit( + performingUser, + null, $"Member {member.Id} \"{member.Name}\" {FormatEmail(member)}", "umbraco/member/save", $"updating {(string.IsNullOrWhiteSpace(dp) ? "(nothing)" : dp)}"); } } - public void Handle(RemovedMemberRolesNotification notification) + [Obsolete("Use HandleAsync() instead. Scheduled for removal in V19.")] + public void Handle(MemberSavedNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); + + /// + public async Task HandleAsync(RemovedMemberRolesNotification notification, CancellationToken cancellationToken) { - IUser performingUser = CurrentPerformingUser; + IUser? performingUser = CurrentPerformingUser; var roles = string.Join(", ", notification.Roles); var members = _memberService.GetAllMembers(notification.MemberIds).ToDictionary(x => x.Id, x => x); foreach (var id in notification.MemberIds) { members.TryGetValue(id, out IMember? member); - _auditService.Write( - performingUser.Id, - $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", - PerformingIp, - DateTime.UtcNow, - -1, + + await Audit( + performingUser, + null, $"Member {id} \"{member?.Name ?? "(unknown)"}\" {FormatEmail(member)}", "umbraco/member/roles/removed", $"roles modified, removed {roles}"); } } - public void Handle(UserDeletedNotification notification) + [Obsolete("Use HandleAsync() instead. Scheduled for removal in V19.")] + public void Handle(RemovedMemberRolesNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); + + /// + public async Task HandleAsync(UserDeletedNotification notification, CancellationToken cancellationToken) { - IUser performingUser = CurrentPerformingUser; + IUser? performingUser = CurrentPerformingUser; IEnumerable affectedUsers = notification.DeletedEntities; foreach (IUser affectedUser in affectedUsers) { - _auditService.Write( - performingUser.Id, - $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", - PerformingIp, - DateTime.UtcNow, - affectedUser.Id, - $"User \"{affectedUser.Name}\" {FormatEmail(affectedUser)}", + await Audit( + performingUser, + affectedUser, + null, "umbraco/user/delete", "delete user"); } } - public void Handle(UserGroupWithUsersSavedNotification notification) + [Obsolete("Use HandleAsync() instead. Scheduled for removal in V19.")] + public void Handle(UserDeletedNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); + + public async Task HandleAsync(UserGroupWithUsersSavedNotification notification, CancellationToken cancellationToken) { - IUser performingUser = CurrentPerformingUser; + IUser? performingUser = CurrentPerformingUser; foreach (UserGroupWithUsers groupWithUser in notification.SavedEntities) { IUserGroup group = groupWithUser.UserGroup; @@ -238,12 +300,9 @@ public void Handle(UserGroupWithUsersSavedNotification notification) sb.Append($"default perms: {perms}"); } - _auditService.Write( - performingUser.Id, - $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", - PerformingIp, - DateTime.UtcNow, - -1, + await Audit( + performingUser, + null, $"User Group {group.Id} \"{group.Name}\" ({group.Alias})", "umbraco/user-group/save", $"{sb}"); @@ -251,35 +310,34 @@ public void Handle(UserGroupWithUsersSavedNotification notification) // now audit the users that have changed foreach (IUser user in groupWithUser.RemovedUsers) { - _auditService.Write( - performingUser.Id, - $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", - PerformingIp, - DateTime.UtcNow, - user.Id, - $"User \"{user.Name}\" {FormatEmail(user)}", + await Audit( + performingUser, + user, + null, "umbraco/user-group/save", $"Removed user \"{user.Name}\" {FormatEmail(user)} from group {group.Id} \"{group.Name}\" ({group.Alias})"); } foreach (IUser user in groupWithUser.AddedUsers) { - _auditService.Write( - performingUser.Id, - $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", - PerformingIp, - DateTime.UtcNow, - user.Id, - $"User \"{user.Name}\" {FormatEmail(user)}", + await Audit( + performingUser, + user, + null, "umbraco/user-group/save", $"Added user \"{user.Name}\" {FormatEmail(user)} to group {group.Id} \"{group.Name}\" ({group.Alias})"); } } } - public void Handle(UserSavedNotification notification) + [Obsolete("Use HandleAsync() instead. Scheduled for removal in V19.")] + public void Handle(UserGroupWithUsersSavedNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); + + /// + public async Task HandleAsync(UserSavedNotification notification, CancellationToken cancellationToken) { - IUser performingUser = CurrentPerformingUser; + IUser? performingUser = CurrentPerformingUser; IEnumerable affectedUsers = notification.SavedEntities; foreach (IUser affectedUser in affectedUsers) { @@ -289,20 +347,41 @@ public void Handle(UserSavedNotification notification) var dp = string.Join(", ", ((User)affectedUser).GetWereDirtyProperties()); - _auditService.Write( - performingUser.Id, - $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", - PerformingIp, - DateTime.UtcNow, - affectedUser.Id, - $"User \"{affectedUser.Name}\" {FormatEmail(affectedUser)}", + await Audit( + performingUser, + affectedUser, + null, "umbraco/user/save", $"updating {(string.IsNullOrWhiteSpace(dp) ? "(nothing)" : dp)}{(groups == null ? string.Empty : "; groups assigned: " + groups)}"); } } + public void Handle(UserSavedNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); + + private async Task Audit( + IUser? performingUser, + IUser? affectedUser, + string? affectedDetails, + string eventType, + string eventDetails) + { + affectedDetails ??= affectedUser is null ? string.Empty : $"User \"{affectedUser.Name}\" {FormatEmail(affectedUser)}"; + await _auditEntryService.WriteAsync( + performingUser?.Key ?? Constants.Security.UnknownUserKey, + $"User \"{performingUser?.Name ?? Constants.Security.UnknownUserName}\" {FormatEmail(performingUser)}", + PerformingIp, + DateTime.UtcNow, + affectedUser?.Key ?? Constants.Security.SuperUserKey, + affectedDetails, + eventType, + eventDetails); + } + private string FormatEmail(IMember? member) => member == null ? string.Empty : member.Email.IsNullOrWhiteSpace() ? string.Empty : $"<{member.Email}>"; - private string FormatEmail(IUser user) => user == null ? string.Empty : user.Email.IsNullOrWhiteSpace() ? string.Empty : $"<{user.Email}>"; + private string FormatEmail(IUser? user) => user is not null && !user.Email.IsNullOrWhiteSpace() + ? $"<{user.Email}>" + : string.Empty; } diff --git a/src/Umbraco.Core/Models/AuditEntry.cs b/src/Umbraco.Core/Models/AuditEntry.cs index 9d1b4dfcef04..2d621e93f428 100644 --- a/src/Umbraco.Core/Models/AuditEntry.cs +++ b/src/Umbraco.Core/Models/AuditEntry.cs @@ -12,11 +12,13 @@ public class AuditEntry : EntityBase, IAuditEntry { private string? _affectedDetails; private int _affectedUserId; + private Guid? _affectedUserKey; private string? _eventDetails; private string? _eventType; private string? _performingDetails; private string? _performingIp; private int _performingUserId; + private Guid? _performingUserKey; /// public int PerformingUserId @@ -25,6 +27,13 @@ public int PerformingUserId set => SetPropertyValueAndDetectChanges(value, ref _performingUserId, nameof(PerformingUserId)); } + /// + public Guid? PerformingUserKey + { + get => _performingUserKey; + set => SetPropertyValueAndDetectChanges(value, ref _performingUserKey, nameof(PerformingUserKey)); + } + /// public string? PerformingDetails { @@ -53,6 +62,13 @@ public int AffectedUserId set => SetPropertyValueAndDetectChanges(value, ref _affectedUserId, nameof(AffectedUserId)); } + /// + public Guid? AffectedUserKey + { + get => _affectedUserKey; + set => SetPropertyValueAndDetectChanges(value, ref _affectedUserKey, nameof(AffectedUserKey)); + } + /// public string? AffectedDetails { diff --git a/src/Umbraco.Core/Models/IAuditEntry.cs b/src/Umbraco.Core/Models/IAuditEntry.cs index 3a1b412ce0da..e01f979fbfd6 100644 --- a/src/Umbraco.Core/Models/IAuditEntry.cs +++ b/src/Umbraco.Core/Models/IAuditEntry.cs @@ -23,6 +23,16 @@ public interface IAuditEntry : IEntity, IRememberBeingDirty /// int PerformingUserId { get; set; } + /// + /// Gets or sets the key of the user triggering the audited event. + /// + // TODO (V19): Remove the default implementations from this interface. + Guid? PerformingUserKey + { + get => null; + set { } + } + /// /// Gets or sets free-form details about the user triggering the audited event. /// @@ -44,6 +54,17 @@ public interface IAuditEntry : IEntity, IRememberBeingDirty /// Not used when no single user is affected by the event. int AffectedUserId { get; set; } + /// + /// Gets or sets the key of the user affected by the audited event. + /// + /// Not used when no single user is affected by the event. + // TODO (V19): Remove the default implementations from this interface. + Guid? AffectedUserKey + { + get => null; + set { } + } + /// /// Gets or sets free-form details about the entity affected by the audited event. /// diff --git a/src/Umbraco.Core/Services/AuditEntryService.cs b/src/Umbraco.Core/Services/AuditEntryService.cs new file mode 100644 index 000000000000..f3fc2f8b8065 --- /dev/null +++ b/src/Umbraco.Core/Services/AuditEntryService.cs @@ -0,0 +1,248 @@ +// +// Copyright (c) PlaceholderCompany. All rights reserved. +// + +using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Persistence.Repositories; +using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services.OperationStatus; + +namespace Umbraco.Cms.Core.Services; + +/// +public class AuditEntryService : RepositoryService, IAuditEntryService +{ + private readonly IAuditEntryRepository _auditEntryRepository; + private readonly IUserIdKeyResolver _userIdKeyResolver; + private readonly Lock _isAvailableLock = new(); + private bool _isAvailable; + + /// + /// Initializes a new instance of the class. + /// + public AuditEntryService( + IAuditEntryRepository auditEntryRepository, + IUserIdKeyResolver userIdKeyResolver, + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory) + : base(provider, loggerFactory, eventMessagesFactory) + { + _auditEntryRepository = auditEntryRepository; + _userIdKeyResolver = userIdKeyResolver; + } + + /// + public async Task> WriteAsync( + Guid performingUserKey, + string performingDetails, + string performingIp, + DateTime eventDateUtc, + Guid affectedUserKey, + string? affectedDetails, + string eventType, + string eventDetails) + { + var performingUserId = await GetUserId(performingUserKey); + var affectedUserId = await GetUserId(affectedUserKey); + return WriteInner( + performingUserId, + performingUserKey, + performingDetails, + performingIp, + eventDateUtc, + affectedUserId, + affectedUserKey, + affectedDetails, + eventType, + eventDetails); + + async Task GetUserId(Guid userKey) + { + return userKey switch + { + _ when userKey == Constants.Security.UnknownUserKey => Constants.Security.UnknownUserId, + _ when await _userIdKeyResolver.TryGetAsync(userKey) is { Success: true } attempt => attempt.Result, + _ => Constants.Security.UnknownUserId, + }; + } + } + + /// + [Obsolete("Use the overload that takes user keys. Scheduled for removal in Umbraco 19.")] + public async Task> WriteAsync( + int performingUserId, + string perfomingDetails, + string performingIp, + DateTime eventDateUtc, + int affectedUserId, + string? affectedDetails, + string eventType, + string eventDetails) => + await WriteInner( + performingUserId, + perfomingDetails, + performingIp, + eventDateUtc, + affectedUserId, + affectedDetails, + eventType, + eventDetails); + + // This method is used by the AuditService while the AuditService.Write() method is not removed. + internal async Task> WriteInner( + int performingUserId, + string performingDetails, + string performingIp, + DateTime eventDateUtc, + int affectedUserId, + string? affectedDetails, + string eventType, + string eventDetails) + { + Guid? performingUserKey = await GetUserKey(performingUserId); + Guid? affectedUserKey = await GetUserKey(affectedUserId); + + return WriteInner( + performingUserId, + performingUserKey, + performingDetails, + performingIp, + eventDateUtc, + affectedUserId, + affectedUserKey, + affectedDetails, + eventType, + eventDetails); + + async Task GetUserKey(int userId) + { + return userId switch + { + Constants.Security.UnknownUserId => Constants.Security.UnknownUserKey, + _ when await _userIdKeyResolver.TryGetAsync(userId) is { Success: true } attempt => attempt.Result, + _ => Constants.Security.UnknownUserKey, + }; + } + } + + private Attempt WriteInner( + int performingUserId, + Guid? performingUserKey, + string performingDetails, + string performingIp, + DateTime eventDateUtc, + int affectedUserId, + Guid? affectedUserKey, + string? affectedDetails, + string eventType, + string eventDetails) + { + ArgumentOutOfRangeException.ThrowIfLessThan(performingUserId, Constants.Security.SuperUserId); + + if (string.IsNullOrWhiteSpace(performingDetails)) + { + throw new ArgumentException("Value cannot be null or whitespace.", nameof(performingDetails)); + } + + if (string.IsNullOrWhiteSpace(eventType)) + { + throw new ArgumentException("Value cannot be null or whitespace.", nameof(eventType)); + } + + if (string.IsNullOrWhiteSpace(eventDetails)) + { + throw new ArgumentException("Value cannot be null or whitespace.", nameof(eventDetails)); + } + + // we need to truncate the data else we'll get SQL errors + affectedDetails = + affectedDetails?[..Math.Min(affectedDetails.Length, Constants.Audit.DetailsLength)]; + eventDetails = eventDetails[..Math.Min(eventDetails.Length, Constants.Audit.DetailsLength)]; + + // validate the eventType - must contain a forward slash, no spaces, no special chars + var eventTypeParts = eventType.ToCharArray(); + if (eventTypeParts.Contains('/') == false || + eventTypeParts.All(c => char.IsLetterOrDigit(c) || c == '/' || c == '-') == false) + { + throw new ArgumentException( + nameof(eventType) + " must contain only alphanumeric characters, hyphens and at least one '/' defining a category"); + } + + if (eventType.Length > Constants.Audit.EventTypeLength) + { + throw new ArgumentException($"Must be max {Constants.Audit.EventTypeLength} chars.", nameof(eventType)); + } + + if (performingIp is { Length: > Constants.Audit.IpLength }) + { + throw new ArgumentException($"Must be max {Constants.Audit.EventTypeLength} chars.", nameof(performingIp)); + } + + var entry = new AuditEntry + { + PerformingUserId = performingUserId, + PerformingUserKey = performingUserKey, + PerformingDetails = performingDetails, + PerformingIp = performingIp, + EventDateUtc = eventDateUtc, + AffectedUserId = affectedUserId, + AffectedUserKey = affectedUserKey, + AffectedDetails = affectedDetails, + EventType = eventType, + EventDetails = eventDetails, + }; + + if (!IsAvailable()) + { + return Attempt.FailWithStatus(AuditEntryOperationStatus.RepositoryNotAvailable, (IAuditEntry)entry); + } + + using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + { + _auditEntryRepository.Save(entry); + scope.Complete(); + } + + return Attempt.SucceedWithStatus(AuditEntryOperationStatus.Success, (IAuditEntry)entry); + } + + // TODO: Currently used in testing only, not part of the interface, need to add queryable methods to the interface instead + internal IEnumerable GetAll() + { + if (!IsAvailable()) + { + return []; + } + + using (ScopeProvider.CreateCoreScope(autoComplete: true)) + { + return _auditEntryRepository.GetMany(); + } + } + + private bool IsAvailable() + { + if (_isAvailable) + { + return true; + } + + lock (_isAvailableLock) + { + if (_isAvailable) + { + return true; + } + + using (ScopeProvider.CreateCoreScope(autoComplete: true)) + { + _isAvailable = _auditEntryRepository.IsAvailable(); + } + } + + return _isAvailable; + } +} diff --git a/src/Umbraco.Core/Services/AuditService.cs b/src/Umbraco.Core/Services/AuditService.cs index 6af3d1567587..7eaa1c4a303e 100644 --- a/src/Umbraco.Core/Services/AuditService.cs +++ b/src/Umbraco.Core/Services/AuditService.cs @@ -1,4 +1,6 @@ +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; @@ -10,27 +12,41 @@ namespace Umbraco.Cms.Core.Services.Implement; public sealed class AuditService : RepositoryService, IAuditService { - private readonly IAuditEntryRepository _auditEntryRepository; private readonly IUserService _userService; private readonly IAuditRepository _auditRepository; private readonly IEntityService _entityService; - private readonly Lazy _isAvailable; public AuditService( ICoreScopeProvider provider, ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IAuditRepository auditRepository, - IAuditEntryRepository auditEntryRepository, IUserService userService, IEntityService entityService) : base(provider, loggerFactory, eventMessagesFactory) { _auditRepository = auditRepository; - _auditEntryRepository = auditEntryRepository; _userService = userService; _entityService = entityService; - _isAvailable = new Lazy(DetermineIsAvailable); + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled for removal in Umbraco 19.")] + public AuditService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IAuditRepository auditRepository, + IAuditEntryRepository auditEntryRepository, + IUserService userService, + IEntityService entityService) + : this( + provider, + loggerFactory, + eventMessagesFactory, + auditRepository, + userService, + entityService) + { } public void Add(AuditType type, int userId, int objectId, string? entityType, string comment, string? parameters = null) @@ -264,115 +280,29 @@ public async Task> GetPagedItemsByUserAsync( } /// - public IAuditEntry Write(int performingUserId, string perfomingDetails, string performingIp, DateTime eventDateUtc, int affectedUserId, string? affectedDetails, string eventType, string eventDetails) + [Obsolete("Use AuditEntryService.WriteAsync() instead. Scheduled for removal in Umbraco 18.")] + public IAuditEntry Write( + int performingUserId, + string perfomingDetails, + string performingIp, + DateTime eventDateUtc, + int affectedUserId, + string? affectedDetails, + string eventType, + string eventDetails) { - if (performingUserId < 0 && performingUserId != Constants.Security.SuperUserId) - { - throw new ArgumentOutOfRangeException(nameof(performingUserId)); - } - - if (string.IsNullOrWhiteSpace(perfomingDetails)) - { - throw new ArgumentException("Value cannot be null or whitespace.", nameof(perfomingDetails)); - } - - if (string.IsNullOrWhiteSpace(eventType)) - { - throw new ArgumentException("Value cannot be null or whitespace.", nameof(eventType)); - } - - if (string.IsNullOrWhiteSpace(eventDetails)) - { - throw new ArgumentException("Value cannot be null or whitespace.", nameof(eventDetails)); - } - - // we need to truncate the data else we'll get SQL errors - affectedDetails = - affectedDetails?[..Math.Min(affectedDetails.Length, Constants.Audit.DetailsLength)]; - eventDetails = eventDetails[..Math.Min(eventDetails.Length, Constants.Audit.DetailsLength)]; - - // validate the eventType - must contain a forward slash, no spaces, no special chars - var eventTypeParts = eventType.ToCharArray(); - if (eventTypeParts.Contains('/') == false || - eventTypeParts.All(c => char.IsLetterOrDigit(c) || c == '/' || c == '-') == false) - { - throw new ArgumentException(nameof(eventType) + - " must contain only alphanumeric characters, hyphens and at least one '/' defining a category"); - } - - if (eventType.Length > Constants.Audit.EventTypeLength) - { - throw new ArgumentException($"Must be max {Constants.Audit.EventTypeLength} chars.", nameof(eventType)); - } - - if (performingIp != null && performingIp.Length > Constants.Audit.IpLength) - { - throw new ArgumentException($"Must be max {Constants.Audit.EventTypeLength} chars.", nameof(performingIp)); - } - - var entry = new AuditEntry - { - PerformingUserId = performingUserId, - PerformingDetails = perfomingDetails, - PerformingIp = performingIp, - EventDateUtc = eventDateUtc, - AffectedUserId = affectedUserId, - AffectedDetails = affectedDetails, - EventType = eventType, - EventDetails = eventDetails, - }; - - if (_isAvailable.Value == false) - { - return entry; - } - - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) - { - _auditEntryRepository.Save(entry); - scope.Complete(); - } - - return entry; - } - - // TODO: Currently used in testing only, not part of the interface, need to add queryable methods to the interface instead - internal IEnumerable? GetAll() - { - if (_isAvailable.Value == false) - { - return Enumerable.Empty(); - } - - using (ScopeProvider.CreateCoreScope(autoComplete: true)) - { - return _auditEntryRepository.GetMany(); - } - } - - // TODO: Currently used in testing only, not part of the interface, need to add queryable methods to the interface instead - internal IEnumerable GetPage(long pageIndex, int pageCount, out long records) - { - if (_isAvailable.Value == false) - { - records = 0; - return Enumerable.Empty(); - } - - using (ScopeProvider.CreateCoreScope(autoComplete: true)) - { - return _auditEntryRepository.GetPage(pageIndex, pageCount, out records); - } - } - - /// - /// Determines whether the repository is available. - /// - private bool DetermineIsAvailable() - { - using (ScopeProvider.CreateCoreScope(autoComplete: true)) - { - return _auditEntryRepository.IsAvailable(); - } + // Use the static service provider to resolve the audit entry service, as this is only needed for this obsolete method. + var auditEntryService = + (AuditEntryService)StaticServiceProvider.Instance.GetRequiredService(); + + return auditEntryService.WriteInner( + performingUserId, + perfomingDetails, + performingIp, + eventDateUtc, + affectedUserId, + affectedDetails, + eventType, + eventDetails).GetAwaiter().GetResult().Result; } } diff --git a/src/Umbraco.Core/Services/IAuditEntryService.cs b/src/Umbraco.Core/Services/IAuditEntryService.cs new file mode 100644 index 000000000000..ffdf7e596f13 --- /dev/null +++ b/src/Umbraco.Core/Services/IAuditEntryService.cs @@ -0,0 +1,69 @@ +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Services.OperationStatus; + +namespace Umbraco.Cms.Core.Services; + +/// +/// Represents a service for handling audit entries. +/// +public interface IAuditEntryService : IService +{ + /// + /// Writes an audit entry for an audited event. + /// + /// The key of the user triggering the audited event. + /// Free-form details about the user triggering the audited event. + /// The IP address or the request triggering the audited event. + /// The date and time of the audited event. + /// The identifier of the user affected by the audited event. + /// Free-form details about the entity affected by the audited event. + /// + /// The type of the audited event - must contain only alphanumeric chars and hyphens with forward slashes separating + /// categories. + /// + /// The eventType will generally be formatted like: {application}/{entity-type}/{category}/{sub-category} + /// Example: umbraco/user/sign-in/failed + /// + /// + /// Free-form details about the audited event. + /// The created audit entry. + public Task> WriteAsync( + Guid performingUserKey, + string performingDetails, + string performingIp, + DateTime eventDateUtc, + Guid affectedUserKey, + string? affectedDetails, + string eventType, + string eventDetails); + + /// + /// Writes an audit entry for an audited event. + /// + /// The identifier of the user triggering the audited event. + /// Free-form details about the user triggering the audited event. + /// The IP address or the request triggering the audited event. + /// The date and time of the audited event. + /// The identifier of the user affected by the audited event. + /// Free-form details about the entity affected by the audited event. + /// + /// The type of the audited event - must contain only alphanumeric chars and hyphens with forward slashes separating + /// categories. + /// + /// The eventType will generally be formatted like: {application}/{entity-type}/{category}/{sub-category} + /// Example: umbraco/user/sign-in/failed + /// + /// + /// Free-form details about the audited event. + /// The created audit entry. + [Obsolete("Use the overload that takes user keys. Scheduled for removal in Umbraco 19.")] + public Task> WriteAsync( + int performingUserId, + string performingDetails, + string performingIp, + DateTime eventDateUtc, + int affectedUserId, + string? affectedDetails, + string eventType, + string eventDetails); +} diff --git a/src/Umbraco.Core/Services/IAuditService.cs b/src/Umbraco.Core/Services/IAuditService.cs index 7481a5e613f4..0ca72e401527 100644 --- a/src/Umbraco.Core/Services/IAuditService.cs +++ b/src/Umbraco.Core/Services/IAuditService.cs @@ -144,6 +144,7 @@ Task> GetPagedItemsByUserAsync( /// /// /// Free-form details about the audited event. + [Obsolete("Use AuditEntryService.WriteAsync() instead. Scheduled for removal in Umbraco 18.")] IAuditEntry Write( int performingUserId, string perfomingDetails, diff --git a/src/Umbraco.Core/Services/IUserIdKeyResolver.cs b/src/Umbraco.Core/Services/IUserIdKeyResolver.cs index b308f16c01aa..aec8e38db3fb 100644 --- a/src/Umbraco.Core/Services/IUserIdKeyResolver.cs +++ b/src/Umbraco.Core/Services/IUserIdKeyResolver.cs @@ -10,6 +10,13 @@ public interface IUserIdKeyResolver /// Thrown when no user was found with the specified key. public Task GetAsync(Guid key); + /// + /// Tries to resolve a user key to a user id without fetching the entire user. + /// + /// The key of the user. + /// An attempt with the id of the user. + public Task> TryGetAsync(Guid key) => throw new NotImplementedException(); + /// /// Tries to resolve a user id to a user key without fetching the entire user. /// @@ -17,4 +24,11 @@ public interface IUserIdKeyResolver /// The key of the user. /// Thrown when no user was found with the specified id. public Task GetAsync(int id); + + /// + /// Tries to resolve a user id to a user key without fetching the entire user. + /// + /// The id of the user. + /// An attempt with the key of the user. + public Task> TryGetAsync(int id) => throw new NotImplementedException(); } diff --git a/src/Umbraco.Core/Services/OperationStatus/AuditEntryOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/AuditEntryOperationStatus.cs new file mode 100644 index 000000000000..795c14b473c6 --- /dev/null +++ b/src/Umbraco.Core/Services/OperationStatus/AuditEntryOperationStatus.cs @@ -0,0 +1,10 @@ +namespace Umbraco.Cms.Core.Services.OperationStatus; + +/// +/// Represents the status of an audit entry operation. +/// +public enum AuditEntryOperationStatus +{ + Success, + RepositoryNotAvailable, +} diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index aee4fdac67a1..0d7ebc0d5056 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -403,15 +403,15 @@ public static IUmbracoBuilder AddCoreNotifications(this IUmbracoBuilder builder) // add notification handlers for auditing builder - .AddNotificationHandler() - .AddNotificationHandler() - .AddNotificationHandler() - .AddNotificationHandler() - .AddNotificationHandler() - .AddNotificationHandler() - .AddNotificationHandler() - .AddNotificationHandler() - .AddNotificationHandler(); + .AddNotificationAsyncHandler() + .AddNotificationAsyncHandler() + .AddNotificationAsyncHandler() + .AddNotificationAsyncHandler() + .AddNotificationAsyncHandler() + .AddNotificationAsyncHandler() + .AddNotificationAsyncHandler() + .AddNotificationAsyncHandler() + .AddNotificationAsyncHandler(); // Handlers for publish warnings builder.AddNotificationHandler(); diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Services.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Services.cs index 4d78be637bb0..4a87fc2bc8cf 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Services.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Services.cs @@ -44,6 +44,7 @@ internal static IUmbracoBuilder AddServices(this IUmbracoBuilder builder) builder.Services.AddUnique(); builder.Services.AddUnique(); + builder.Services.AddUnique(); builder.Services.AddUnique(); builder.Services.AddUnique(); builder.Services.AddUnique(); diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs index 21cfe558a688..f4d278e6555e 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs @@ -119,5 +119,8 @@ protected virtual void DefinePlan() // To 16.0.0 To("{C6681435-584F-4BC8-BB8D-BC853966AF0B}"); To("{D1568C33-A697-455F-8D16-48060CB954A1}"); + + // To 17.0.0 + To("{17D5F6CA-CEB8-462A-AF86-4B9C3BF91CF1}"); } } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_17_0_0/AddGuidsToAuditEntries.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_17_0_0/AddGuidsToAuditEntries.cs new file mode 100644 index 000000000000..e9cf6bdfd4c7 --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_17_0_0/AddGuidsToAuditEntries.cs @@ -0,0 +1,52 @@ +using NPoco; +using Umbraco.Cms.Core; +using Umbraco.Cms.Infrastructure.Persistence.Dtos; +using Umbraco.Cms.Infrastructure.Scoping; + +namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_17_0_0; + +public class AddGuidsToAuditEntries : UnscopedMigrationBase +{ + private const string NewPerformingUserKeyColumnName = "performingUserKey"; + private const string NewAffectedUserKeyColumnName = "affectedUserKey"; + private readonly IScopeProvider _scopeProvider; + + public AddGuidsToAuditEntries(IMigrationContext context, IScopeProvider scopeProvider) + : base(context) + { + _scopeProvider = scopeProvider; + } + + protected override void Migrate() + { + // If the new column already exists, we'll do nothing. + if (ColumnExists(Constants.DatabaseSchema.Tables.AuditEntry, NewPerformingUserKeyColumnName)) + { + Context.Complete(); + return; + } + + using IScope scope = _scopeProvider.CreateScope(); + using IDisposable notificationSuppression = scope.Notifications.Suppress(); + ScopeDatabase(scope); + + var columns = SqlSyntax.GetColumnsInSchema(Context.Database).ToList(); + AddColumnIfNotExists(columns, NewPerformingUserKeyColumnName); + AddColumnIfNotExists(columns, NewAffectedUserKeyColumnName); + + Database.Execute( + new Sql( + "UPDATE umbracoAudit " + + "SET performingUserKey = (" + + $"SELECT umbracoUser.{Database.SqlContext.SqlSyntax.GetQuotedColumnName("key")} FROM umbracoUser WHERE umbracoUser.id= umbracoAudit.performingUserId);")); + + Database.Execute( + new Sql( + "UPDATE umbracoAudit " + + "SET affectedUserKey = (" + + $"SELECT umbracoUser.{Database.SqlContext.SqlSyntax.GetQuotedColumnName("key")} FROM umbracoUser WHERE umbracoUser.id= umbracoAudit.affectedUserId);")); + + scope.Complete(); + Context.Complete(); + } +} diff --git a/src/Umbraco.Infrastructure/Persistence/Dtos/AuditEntryDto.cs b/src/Umbraco.Infrastructure/Persistence/Dtos/AuditEntryDto.cs index c94a680a1f01..774bc7b3fc6a 100644 --- a/src/Umbraco.Infrastructure/Persistence/Dtos/AuditEntryDto.cs +++ b/src/Umbraco.Infrastructure/Persistence/Dtos/AuditEntryDto.cs @@ -20,6 +20,10 @@ internal class AuditEntryDto [Column("performingUserId")] public int PerformingUserId { get; set; } + [Column("performingUserKey")] + [NullSetting(NullSetting = NullSettings.Null)] + public Guid? PerformingUserKey { get; set; } + [Column("performingDetails")] [NullSetting(NullSetting = NullSettings.Null)] [Length(Constants.Audit.DetailsLength)] @@ -37,6 +41,10 @@ internal class AuditEntryDto [Column("affectedUserId")] public int AffectedUserId { get; set; } + [Column("affectedUserKey")] + [NullSetting(NullSetting = NullSettings.Null)] + public Guid? AffectedUserKey { get; set; } + [Column("affectedDetails")] [NullSetting(NullSetting = NullSettings.Null)] [Length(Constants.Audit.DetailsLength)] diff --git a/src/Umbraco.Infrastructure/Persistence/Factories/AuditEntryFactory.cs b/src/Umbraco.Infrastructure/Persistence/Factories/AuditEntryFactory.cs index 297cea90252e..2d2d15211c5d 100644 --- a/src/Umbraco.Infrastructure/Persistence/Factories/AuditEntryFactory.cs +++ b/src/Umbraco.Infrastructure/Persistence/Factories/AuditEntryFactory.cs @@ -14,10 +14,12 @@ public static IAuditEntry BuildEntity(AuditEntryDto dto) { Id = dto.Id, PerformingUserId = dto.PerformingUserId, + PerformingUserKey = dto.PerformingUserKey, PerformingDetails = dto.PerformingDetails, PerformingIp = dto.PerformingIp, EventDateUtc = dto.EventDateUtc, AffectedUserId = dto.AffectedUserId, + AffectedUserKey = dto.AffectedUserKey, AffectedDetails = dto.AffectedDetails, EventType = dto.EventType, EventDetails = dto.EventDetails, @@ -34,10 +36,12 @@ public static AuditEntryDto BuildDto(IAuditEntry entity) => { Id = entity.Id, PerformingUserId = entity.PerformingUserId, + PerformingUserKey = entity.PerformingUserKey, PerformingDetails = entity.PerformingDetails, PerformingIp = entity.PerformingIp, EventDateUtc = entity.EventDateUtc, AffectedUserId = entity.AffectedUserId, + AffectedUserKey = entity.AffectedUserKey, AffectedDetails = entity.AffectedDetails, EventType = entity.EventType, EventDetails = entity.EventDetails, diff --git a/src/Umbraco.Infrastructure/Persistence/Mappers/AuditEntryMapper.cs b/src/Umbraco.Infrastructure/Persistence/Mappers/AuditEntryMapper.cs index 6f27cf830f0a..04fc3377cd3e 100644 --- a/src/Umbraco.Infrastructure/Persistence/Mappers/AuditEntryMapper.cs +++ b/src/Umbraco.Infrastructure/Persistence/Mappers/AuditEntryMapper.cs @@ -19,10 +19,12 @@ protected override void DefineMaps() { DefineMap(nameof(AuditEntry.Id), nameof(AuditEntryDto.Id)); DefineMap(nameof(AuditEntry.PerformingUserId), nameof(AuditEntryDto.PerformingUserId)); + DefineMap(nameof(AuditEntry.PerformingUserKey), nameof(AuditEntryDto.PerformingUserKey)); DefineMap(nameof(AuditEntry.PerformingDetails), nameof(AuditEntryDto.PerformingDetails)); DefineMap(nameof(AuditEntry.PerformingIp), nameof(AuditEntryDto.PerformingIp)); DefineMap(nameof(AuditEntry.EventDateUtc), nameof(AuditEntryDto.EventDateUtc)); DefineMap(nameof(AuditEntry.AffectedUserId), nameof(AuditEntryDto.AffectedUserId)); + DefineMap(nameof(AuditEntry.AffectedUserKey), nameof(AuditEntryDto.AffectedUserKey)); DefineMap(nameof(AuditEntry.AffectedDetails), nameof(AuditEntryDto.AffectedDetails)); DefineMap(nameof(AuditEntry.EventType), nameof(AuditEntryDto.EventType)); DefineMap(nameof(AuditEntry.EventDetails), nameof(AuditEntryDto.EventDetails)); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs index eab408823ee2..bdd637b2c9d0 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs @@ -10,6 +10,7 @@ using Umbraco.Cms.Infrastructure.Persistence.Querying; using Umbraco.Cms.Infrastructure.Scoping; using Umbraco.Extensions; +using ColumnInfo = Umbraco.Cms.Infrastructure.Persistence.SqlSyntax.ColumnInfo; namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement; @@ -43,7 +44,17 @@ public IEnumerable GetPage(long pageIndex, int pageCount, out long public bool IsAvailable() { var tables = SqlSyntax.GetTablesInSchema(Database).ToArray(); - return tables.InvariantContains(Constants.DatabaseSchema.Tables.AuditEntry); + if (!tables.InvariantContains(Constants.DatabaseSchema.Tables.AuditEntry)) + { + return false; + } + + // This check is needed to determine if the migration 'V_17_0_0.AddGuidsToAuditEntries' has run. + // Otherwise, an exception will be thrown when trying to write audits when in "maintenance" mode. + // This check can be removed once that migration is removed. + ColumnInfo[] columns = SqlSyntax.GetColumnsInSchema(Database).Distinct().ToArray(); + return columns.Any(x => x.TableName.InvariantEquals(Constants.DatabaseSchema.Tables.AuditEntry) + && x.ColumnName.InvariantEquals("performingUserKey")); } /// diff --git a/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs b/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs index e8ebbd6df569..d11c4a09af9b 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/UserIdKeyResolver.cs @@ -1,5 +1,6 @@ using System.Collections.Concurrent; using NPoco; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Infrastructure.Persistence.Dtos; @@ -22,10 +23,14 @@ internal sealed class UserIdKeyResolver : IUserIdKeyResolver /// public async Task GetAsync(Guid key) + => await TryGetAsync(key) is { Success: true } attempt ? attempt.Result : throw new InvalidOperationException("No user found with the specified key"); + + /// + public async Task> TryGetAsync(Guid key) { if (_keyToId.TryGetValue(key, out int id)) { - return id; + return Attempt.Succeed(id); } // We don't have it in the cache, so we'll need to look it up in the database @@ -36,7 +41,7 @@ public async Task GetAsync(Guid key) if (_keyToId.TryGetValue(key, out int recheckedId)) { // It was added while we were waiting, so we'll just return it - return recheckedId; + return Attempt.Succeed(recheckedId); } // Still not here, so actually fetch it now @@ -48,12 +53,15 @@ public async Task GetAsync(Guid key) .From() .Where(x => x.Key == key); - int fetchedId = (await scope.Database.ExecuteScalarAsync(query)) - ?? throw new InvalidOperationException("No user found with the specified key"); + int? fetchedId = await scope.Database.ExecuteScalarAsync(query); + if (fetchedId is null) + { + return Attempt.Fail(); + } - _keyToId[key] = fetchedId; - return fetchedId; + _keyToId[key] = fetchedId.Value; + return Attempt.Succeed(fetchedId.Value); } finally { @@ -63,10 +71,14 @@ public async Task GetAsync(Guid key) /// public async Task GetAsync(int id) + => await TryGetAsync(id) is { Success: true } attempt ? attempt.Result : throw new InvalidOperationException("No user found with the specified id"); + + /// + public async Task> TryGetAsync(int id) { if (_idToKey.TryGetValue(id, out Guid key)) { - return key; + return Attempt.Succeed(key); } await _idToKeyLock.WaitAsync(); @@ -74,7 +86,7 @@ public async Task GetAsync(int id) { if (_idToKey.TryGetValue(id, out Guid recheckedKey)) { - return recheckedKey; + return Attempt.Succeed(recheckedKey); } using IScope scope = _scopeProvider.CreateScope(autoComplete: true); @@ -85,12 +97,15 @@ public async Task GetAsync(int id) .From() .Where(x => x.Id == id); - Guid fetchedKey = scope.Database.ExecuteScalar(query) - ?? throw new InvalidOperationException("No user found with the specified id"); + Guid? fetchedKey = scope.Database.ExecuteScalar(query); + if (fetchedKey is null) + { + return Attempt.Fail(); + } - _idToKey[id] = fetchedKey; + _idToKey[id] = fetchedKey.Value; - return fetchedKey; + return Attempt.Succeed(fetchedKey.Value); } finally { diff --git a/tests/Umbraco.Tests.Common/Builders/AuditEntryBuilder.cs b/tests/Umbraco.Tests.Common/Builders/AuditEntryBuilder.cs index 15d98ca21214..3daca9b518a3 100644 --- a/tests/Umbraco.Tests.Common/Builders/AuditEntryBuilder.cs +++ b/tests/Umbraco.Tests.Common/Builders/AuditEntryBuilder.cs @@ -2,6 +2,7 @@ // See LICENSE for more details. using System; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Tests.Common.Builders.Interfaces; @@ -25,6 +26,7 @@ public class AuditEntryBuilder { private string _affectedDetails; private int? _affectedUserId; + private Guid? _affectedUserKey; private DateTime? _createDate; private DateTime? _deleteDate; private DateTime? _eventDateUtc; @@ -34,6 +36,7 @@ public class AuditEntryBuilder private Guid? _key; private string _performingDetails; private string _performingIp; + private Guid? _performingUserKey; private int? _performingUserId; private DateTime? _updateDate; @@ -84,6 +87,12 @@ public AuditEntryBuilder WithAffectedUserId(int affectedUserId) return this; } + public AuditEntryBuilder WithAffectedUserKey(Guid? affectedUserKey) + { + _affectedUserKey = affectedUserKey; + return this; + } + public AuditEntryBuilder WithEventDetails(string eventDetails) { _eventDetails = eventDetails; @@ -120,6 +129,12 @@ public AuditEntryBuilder WithPerformingUserId(int performingUserId) return this; } + public AuditEntryBuilder WithPerformingUserKey(Guid? performingUserKey) + { + _performingUserKey = performingUserKey; + return this; + } + public override IAuditEntry Build() { var id = _id ?? 0; @@ -129,12 +144,14 @@ public override IAuditEntry Build() var deleteDate = _deleteDate; var affectedDetails = _affectedDetails ?? Guid.NewGuid().ToString(); var affectedUserId = _affectedUserId ?? -1; + var affectedUserKey = _affectedUserKey ?? Constants.Security.SuperUserKey; var eventDetails = _eventDetails ?? Guid.NewGuid().ToString(); var eventType = _eventType ?? "umbraco/user"; var performingDetails = _performingDetails ?? Guid.NewGuid().ToString(); var performingIp = _performingIp ?? "127.0.0.1"; var eventDateUtc = _eventDateUtc ?? DateTime.UtcNow; var performingUserId = _performingUserId ?? -1; + var performingUserKey = _performingUserKey ?? Constants.Security.SuperUserKey; return new AuditEntry { @@ -145,12 +162,14 @@ public override IAuditEntry Build() DeleteDate = deleteDate, AffectedDetails = affectedDetails, AffectedUserId = affectedUserId, + AffectedUserKey = affectedUserKey, EventDetails = eventDetails, EventType = eventType, PerformingDetails = performingDetails, PerformingIp = performingIp, EventDateUtc = eventDateUtc, - PerformingUserId = performingUserId + PerformingUserId = performingUserId, + PerformingUserKey = performingUserKey, }; } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditEntryServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditEntryServiceTests.cs new file mode 100644 index 000000000000..2837acdf82aa --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditEntryServiceTests.cs @@ -0,0 +1,94 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Microsoft.Extensions.DependencyInjection; +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; + +[TestFixture] +[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] +internal sealed class AuditEntryServiceTests : UmbracoIntegrationTest +{ + [Test] + public async Task Write_and_GetAll() + { + var sut = (AuditEntryService)Services.GetRequiredService(); + var expected = new AuditEntryBuilder() + .Build(); + + var result = await sut.WriteAsync( + expected.PerformingUserKey.Value, + expected.PerformingDetails, + expected.PerformingIp, + expected.EventDateUtc, + expected.AffectedUserKey.Value, + expected.AffectedDetails, + expected.EventType, + expected.EventDetails); + Assert.IsTrue(result.Success); + + var actual = result.Result; + + var entries = sut.GetAll().ToArray(); + + Assert.Multiple(() => + { + Assert.AreEqual(expected.PerformingUserId, actual.PerformingUserId); + Assert.AreEqual(expected.PerformingUserKey, actual.PerformingUserKey); + Assert.AreEqual(expected.PerformingDetails, actual.PerformingDetails); + Assert.AreEqual(expected.EventDateUtc, actual.EventDateUtc); + Assert.AreEqual(expected.AffectedUserId, actual.AffectedUserId); + Assert.AreEqual(expected.AffectedUserKey, actual.AffectedUserKey); + Assert.AreEqual(expected.AffectedDetails, actual.AffectedDetails); + Assert.AreEqual(expected.EventType, actual.EventType); + Assert.AreEqual(expected.EventDetails, actual.EventDetails); + Assert.IsNotNull(entries); + Assert.AreEqual(1, entries.Length); + Assert.AreEqual(expected.PerformingUserKey, entries[0].PerformingUserKey); + }); + } + + [Test] + public async Task Write_and_GetAll_With_Keys() + { + var sut = (AuditEntryService)Services.GetRequiredService(); + var eventDateUtc = DateTime.UtcNow; + var result = await sut.WriteAsync( + Constants.Security.SuperUserKey, + "performingDetails", + "performingIp", + eventDateUtc, + Constants.Security.UnknownUserKey, + "affectedDetails", + "umbraco/test", + "eventDetails"); + Assert.IsTrue(result.Success); + + var actual = result.Result; + + var entries = sut.GetAll().ToArray(); + + Assert.Multiple(() => + { + Assert.AreEqual(Constants.Security.SuperUserId, actual.PerformingUserId); + Assert.AreEqual(Constants.Security.SuperUserKey, actual.PerformingUserKey); + Assert.AreEqual("performingDetails", actual.PerformingDetails); + Assert.AreEqual("performingIp", actual.PerformingIp); + Assert.AreEqual(eventDateUtc, actual.EventDateUtc); + Assert.AreEqual(Constants.Security.UnknownUserId, actual.AffectedUserId); + Assert.AreEqual(Constants.Security.UnknownUserKey, actual.AffectedUserKey); + Assert.AreEqual("affectedDetails", actual.AffectedDetails); + Assert.AreEqual("umbraco/test", actual.EventType); + Assert.AreEqual("eventDetails", actual.EventDetails); + Assert.IsNotNull(entries); + Assert.AreEqual(1, entries.Length); + Assert.AreEqual(Constants.Security.SuperUserId, entries[0].PerformingUserId); + }); + } +} diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs index 21f663a43987..248fa38a056e 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs @@ -17,35 +17,6 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] internal sealed class AuditServiceTests : UmbracoIntegrationTest { - [Test] - public void GetPage() - { - var sut = (AuditService)GetRequiredService(); - var expected = new AuditEntryBuilder().Build(); - - for (var i = 0; i < 10; i++) - { - sut.Write( - expected.PerformingUserId + i, - expected.PerformingDetails, - expected.PerformingIp, - expected.EventDateUtc.AddMinutes(i), - expected.AffectedUserId + i, - expected.AffectedDetails, - expected.EventType, - expected.EventDetails); - } - - var entries = sut.GetPage(2, 2, out var count).ToArray(); - - Assert.Multiple(() => - { - Assert.AreEqual(2, entries.Length); - Assert.AreEqual(expected.PerformingUserId + 5, entries[0].PerformingUserId); - Assert.AreEqual(expected.PerformingUserId + 4, entries[1].PerformingUserId); - }); - } - [Test] public void GetUserLogs() { @@ -72,37 +43,4 @@ public void GetUserLogs() Assert.AreEqual(numberOfEntries, logs.Count(x => x.AuditType == AuditType.Unpublish)); }); } - - [Test] - public void Write_and_GetAll() - { - var sut = (AuditService)Services.GetRequiredService(); - var expected = new AuditEntryBuilder().Build(); - - var actual = sut.Write( - expected.PerformingUserId, - expected.PerformingDetails, - expected.PerformingIp, - expected.EventDateUtc, - expected.AffectedUserId, - expected.AffectedDetails, - expected.EventType, - expected.EventDetails); - - var entries = sut.GetAll().ToArray(); - - Assert.Multiple(() => - { - Assert.AreEqual(expected.PerformingUserId, actual.PerformingUserId); - Assert.AreEqual(expected.PerformingDetails, actual.PerformingDetails); - Assert.AreEqual(expected.EventDateUtc, actual.EventDateUtc); - Assert.AreEqual(expected.AffectedUserId, actual.AffectedUserId); - Assert.AreEqual(expected.AffectedDetails, actual.AffectedDetails); - Assert.AreEqual(expected.EventType, actual.EventType); - Assert.AreEqual(expected.EventDetails, actual.EventDetails); - Assert.IsNotNull(entries); - Assert.AreEqual(1, entries.Length); - Assert.AreEqual(expected.PerformingUserId, entries[0].PerformingUserId); - }); - } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditEntryServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditEntryServiceTests.cs new file mode 100644 index 000000000000..a947f8f25d65 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditEntryServiceTests.cs @@ -0,0 +1,148 @@ +using System.Data; +using Microsoft.Extensions.Logging; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Persistence.Repositories; +using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.OperationStatus; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Services; + +[TestFixture] +public class AuditEntryServiceTests +{ + private IAuditEntryService _auditEntryService; + private Mock _scopeProviderMock; + private Mock _auditEntryRepositoryMock; + private Mock _userIdKeyResolverMock; + + [SetUp] + public void Setup() + { + _scopeProviderMock = new Mock(MockBehavior.Strict); + _auditEntryRepositoryMock = new Mock(MockBehavior.Strict); + _userIdKeyResolverMock = new Mock(MockBehavior.Strict); + + _auditEntryService = new AuditEntryService( + _auditEntryRepositoryMock.Object, + _userIdKeyResolverMock.Object, + _scopeProviderMock.Object, + Mock.Of(MockBehavior.Strict), + Mock.Of(MockBehavior.Strict)); + } + + [Test] + public async Task WriteAsync_UsingIds_Calls_Repository_With_Correct_Values() + { + SetupScopeProviderMock(); + + var date = DateTime.UtcNow; + _auditEntryRepositoryMock.Setup(x => x.IsAvailable()).Returns(true); + _auditEntryRepositoryMock.Setup(x => x.Save(It.IsAny())) + .Callback(item => + { + Assert.AreEqual(Constants.Security.SuperUserId, item.PerformingUserId); + Assert.AreEqual("performingDetails", item.PerformingDetails); + Assert.AreEqual("performingIp", item.PerformingIp); + Assert.AreEqual(date, item.EventDateUtc); + Assert.AreEqual(Constants.Security.UnknownUserId, item.AffectedUserId); + Assert.AreEqual("affectedDetails", item.AffectedDetails); + Assert.AreEqual("umbraco/test", item.EventType); + Assert.AreEqual("eventDetails", item.EventDetails); + }); + _userIdKeyResolverMock.Setup(x => x.TryGetAsync(Constants.Security.SuperUserId)) + .ReturnsAsync(Attempt.Succeed(Constants.Security.SuperUserKey)); + + var result = await _auditEntryService.WriteAsync( + Constants.Security.SuperUserId, + "performingDetails", + "performingIp", + date, + Constants.Security.UnknownUserId, + "affectedDetails", + "umbraco/test", + "eventDetails"); + _auditEntryRepositoryMock.Verify(x => x.IsAvailable(), Times.AtLeastOnce); + _auditEntryRepositoryMock.Verify(x => x.Save(It.IsAny()), Times.Once); + + Assert.IsTrue(result.Success); + Assert.AreEqual(AuditEntryOperationStatus.Success, result.Status); + Assert.Multiple(() => + { + Assert.AreEqual(Constants.Security.SuperUserId, result.Result.PerformingUserId); + Assert.AreEqual("performingDetails", result.Result.PerformingDetails); + Assert.AreEqual("performingIp", result.Result.PerformingIp); + Assert.AreEqual(date, result.Result.EventDateUtc); + Assert.AreEqual(Constants.Security.UnknownUserId, result.Result.AffectedUserId); + Assert.AreEqual("affectedDetails", result.Result.AffectedDetails); + Assert.AreEqual("umbraco/test", result.Result.EventType); + Assert.AreEqual("eventDetails", result.Result.EventDetails); + }); + } + + [Test] + public async Task WriteAsync_UsingKeys_Calls_Repository_With_Correct_Values() + { + SetupScopeProviderMock(); + + var date = DateTime.UtcNow; + _auditEntryRepositoryMock.Setup(x => x.IsAvailable()).Returns(true); + _auditEntryRepositoryMock.Setup(x => x.Save(It.IsAny())) + .Callback(item => + { + Assert.AreEqual(Constants.Security.SuperUserId, item.PerformingUserId); + Assert.AreEqual("performingDetails", item.PerformingDetails); + Assert.AreEqual("performingIp", item.PerformingIp); + Assert.AreEqual(date, item.EventDateUtc); + Assert.AreEqual(Constants.Security.UnknownUserId, item.AffectedUserId); + Assert.AreEqual("affectedDetails", item.AffectedDetails); + Assert.AreEqual("umbraco/test", item.EventType); + Assert.AreEqual("eventDetails", item.EventDetails); + }); + _userIdKeyResolverMock.Setup(x => x.TryGetAsync(Constants.Security.SuperUserKey)) + .ReturnsAsync(Attempt.Succeed(Constants.Security.SuperUserId)); + + var result = await _auditEntryService.WriteAsync( + Constants.Security.SuperUserKey, + "performingDetails", + "performingIp", + date, + Constants.Security.UnknownUserKey, + "affectedDetails", + "umbraco/test", + "eventDetails"); + + _auditEntryRepositoryMock.Verify(x => x.IsAvailable(), Times.AtLeastOnce); + _auditEntryRepositoryMock.Verify(x => x.Save(It.IsAny()), Times.Once); + + Assert.IsTrue(result.Success); + Assert.AreEqual(AuditEntryOperationStatus.Success, result.Status); + Assert.Multiple(() => + { + Assert.AreEqual(Constants.Security.SuperUserId, result.Result.PerformingUserId); + Assert.AreEqual("performingDetails", result.Result.PerformingDetails); + Assert.AreEqual("performingIp", result.Result.PerformingIp); + Assert.AreEqual(date, result.Result.EventDateUtc); + Assert.AreEqual(Constants.Security.UnknownUserId, result.Result.AffectedUserId); + Assert.AreEqual("affectedDetails", result.Result.AffectedDetails); + Assert.AreEqual("umbraco/test", result.Result.EventType); + Assert.AreEqual("eventDetails", result.Result.EventDetails); + }); + } + + private void SetupScopeProviderMock() => + _scopeProviderMock + .Setup(x => x.CreateCoreScope( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(Mock.Of()); +} From 066b68b5886f85d0556d790c7a56158da3dd450a Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Fri, 16 May 2025 17:24:36 +0200 Subject: [PATCH 02/12] Audit service rework - Added new async and paged methods - Marked (now) redundant methods as obsolete - Updated all of the usages to use the non-obsolete methods - Added unit tests class `AuditServiceTests` and some unit tests - Updated existing integration test --- .../Events/RelateOnCopyNotificationHandler.cs | 39 +- src/Umbraco.Core/Services/AuditService.cs | 476 +++++++++++------- src/Umbraco.Core/Services/IAuditService.cs | 201 +++++--- .../BackgroundJobs/Jobs/LogScrubberJob.cs | 19 +- .../UmbracoBuilder.CoreServices.cs | 6 +- .../RelateOnTrashNotificationHandler.cs | 52 +- .../Services/Implement/PackagingService.cs | 31 +- .../Services/ContentServiceTests.cs | 10 +- .../Services/AuditServiceTests.cs | 17 +- .../Services/ContentEditingServiceTests.cs | 2 +- .../Services/AuditServiceTests.cs | 250 +++++++++ .../Jobs/LogScrubberJobTests.cs | 2 +- 12 files changed, 820 insertions(+), 285 deletions(-) create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs diff --git a/src/Umbraco.Core/Events/RelateOnCopyNotificationHandler.cs b/src/Umbraco.Core/Events/RelateOnCopyNotificationHandler.cs index 3817f93f6f4c..ad0cdaac8c31 100644 --- a/src/Umbraco.Core/Events/RelateOnCopyNotificationHandler.cs +++ b/src/Umbraco.Core/Events/RelateOnCopyNotificationHandler.cs @@ -1,31 +1,53 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Services; namespace Umbraco.Cms.Core.Events; -public class RelateOnCopyNotificationHandler : INotificationHandler +public class RelateOnCopyNotificationHandler : + INotificationHandler, + INotificationAsyncHandler { private readonly IAuditService _auditService; + private readonly IUserIdKeyResolver _userIdKeyResolver; private readonly IRelationService _relationService; - public RelateOnCopyNotificationHandler(IRelationService relationService, IAuditService auditService) + public RelateOnCopyNotificationHandler( + IRelationService relationService, + IAuditService auditService, + IUserIdKeyResolver userIdKeyResolver) { _relationService = relationService; _auditService = auditService; + _userIdKeyResolver = userIdKeyResolver; } - public void Handle(ContentCopiedNotification notification) + [Obsolete("Use the non-obsolete constructor instead. Scheduled for removal in V19.")] + public RelateOnCopyNotificationHandler( + IRelationService relationService, + IAuditService auditService) + : this( + relationService, + auditService, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + /// + public async Task HandleAsync(ContentCopiedNotification notification, CancellationToken cancellationToken) { if (notification.RelateToOriginal == false) { return; } - IRelationType? relationType = _relationService.GetRelationTypeByAlias(Constants.Conventions.RelationTypes.RelateDocumentOnCopyAlias); + IRelationType? relationType = + _relationService.GetRelationTypeByAlias(Constants.Conventions.RelationTypes.RelateDocumentOnCopyAlias); if (relationType == null) { @@ -43,11 +65,16 @@ public void Handle(ContentCopiedNotification notification) var relation = new Relation(notification.Original.Id, notification.Copy.Id, relationType); _relationService.Save(relation); - _auditService.Add( + Guid writerKey = await _userIdKeyResolver.GetAsync(notification.Copy.WriterId); + await _auditService.AddAsync( AuditType.Copy, - notification.Copy.WriterId, + writerKey, notification.Copy.Id, UmbracoObjectTypes.Document.GetName() ?? string.Empty, $"Copied content with Id: '{notification.Copy.Id}' related to original content with Id: '{notification.Original.Id}'"); } + + [Obsolete("Use the INotificationAsyncHandler.HandleAsync implementation instead. Scheduled for removal in V19.")] + public void Handle(ContentCopiedNotification notification) => + HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); } diff --git a/src/Umbraco.Core/Services/AuditService.cs b/src/Umbraco.Core/Services/AuditService.cs index 7eaa1c4a303e..1b5e2c4b2152 100644 --- a/src/Umbraco.Core/Services/AuditService.cs +++ b/src/Umbraco.Core/Services/AuditService.cs @@ -7,15 +7,23 @@ using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.Persistence.Repositories; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Core.Services.Implement; +/// +/// Audit service for logging and retrieving audit entries. +/// public sealed class AuditService : RepositoryService, IAuditService { - private readonly IUserService _userService; private readonly IAuditRepository _auditRepository; private readonly IEntityService _entityService; + private readonly IUserService _userService; + /// + /// Initializes a new instance of the class. + /// + [ActivatorUtilitiesConstructor] public AuditService( ICoreScopeProvider provider, ILoggerFactory loggerFactory, @@ -30,7 +38,10 @@ public AuditService( _entityService = entityService; } - [Obsolete("Use the non-obsolete constructor instead. Scheduled for removal in Umbraco 19.")] + /// + /// Initializes a new instance of the class. + /// + [Obsolete("Use the non-obsolete constructor. Scheduled for removal in Umbraco 19.")] public AuditService( ICoreScopeProvider provider, ILoggerFactory loggerFactory, @@ -39,143 +50,153 @@ public AuditService( IAuditEntryRepository auditEntryRepository, IUserService userService, IEntityService entityService) - : this( - provider, - loggerFactory, - eventMessagesFactory, - auditRepository, - userService, - entityService) + : this(provider, loggerFactory, eventMessagesFactory, auditRepository, userService, entityService) { } - public void Add(AuditType type, int userId, int objectId, string? entityType, string comment, string? parameters = null) + /// + public async Task> AddAsync( + AuditType type, + Guid userKey, + int objectId, + string? entityType, + string? comment = null, + string? parameters = null) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + var userId = userKey switch { - _auditRepository.Save(new AuditItem(objectId, type, userId, entityType, comment, parameters)); - scope.Complete(); - } - } + { } when userKey == Constants.Security.UnknownUserKey => Constants.Security.UnknownUserId, + _ => (await _userService.GetAsync(userKey))?.Id, + }; - public IEnumerable GetLogs(int objectId) - { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + if (userId is null) { - IEnumerable result = _auditRepository.Get(Query().Where(x => x.Id == objectId)); - scope.Complete(); - return result; + return Attempt.Fail(AuditLogOperationStatus.UserNotFound); } + + return AddInner(type, userId.Value, objectId, entityType, comment, parameters); } - public IEnumerable GetUserLogs(int userId, AuditType type, DateTime? sinceDate = null) + /// + [Obsolete("Use AddAsync() instead. Scheduled for removal in Umbraco 19.")] + public void Add( + AuditType type, + int userId, + int objectId, + string? entityType, + string comment, + string? parameters = null) => + AddInner(type, userId, objectId, entityType, comment, parameters); + + /// + public Task> GetItemsAsync( + int skip, + int take, + Direction orderDirection = Direction.Descending, + DateTimeOffset? sinceDate = null, + AuditType[]? auditTypeFilter = null) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) - { - IEnumerable result = sinceDate.HasValue == false - ? _auditRepository.Get(type, Query().Where(x => x.UserId == userId)) - : _auditRepository.Get( - type, - Query().Where(x => x.UserId == userId && x.CreateDate >= sinceDate.Value)); - scope.Complete(); - return result; - } + ArgumentOutOfRangeException.ThrowIfNegative(skip); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(take); + + PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageIndex, out var pageSize); + IQuery? customFilter = sinceDate.HasValue + ? Query().Where(x => x.CreateDate >= sinceDate) + : null; + + PagedModel result = GetItemsInner( + null, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + return Task.FromResult(result); } + /// + [Obsolete("Use GetItemsAsync() instead. Scheduled for removal in Umbraco 19.")] public IEnumerable GetLogs(AuditType type, DateTime? sinceDate = null) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) - { - IEnumerable result = sinceDate.HasValue == false - ? _auditRepository.Get(type, Query()) - : _auditRepository.Get(type, Query().Where(x => x.CreateDate >= sinceDate.Value)); - scope.Complete(); - return result; - } + IQuery? customFilter = sinceDate.HasValue + ? Query().Where(x => x.CreateDate >= sinceDate) + : null; + + PagedModel result = GetItemsInner( + null, + 0, + int.MaxValue, + Direction.Ascending, + customFilter: customFilter); + return result.Items; } - public void CleanLogs(int maximumAgeOfLogsInMinutes) + /// + public Task> GetItemsByKeyAsync( + Guid entityKey, + UmbracoObjectTypes entityType, + int skip, + int take, + Direction orderDirection = Direction.Descending, + DateTimeOffset? sinceDate = null, + AuditType[]? auditTypeFilter = null) { - using (ICoreScope scope = ScopeProvider.CreateCoreScope()) + ArgumentOutOfRangeException.ThrowIfNegative(skip); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(take); + + Attempt keyToIdAttempt = _entityService.GetId(entityKey, entityType); + if (keyToIdAttempt.Success is false) { - _auditRepository.CleanLogs(maximumAgeOfLogsInMinutes); - scope.Complete(); + return Task.FromResult(new PagedModel { Items = [], Total = 0 }); } + + PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageIndex, out var pageSize); + IQuery? customFilter = + sinceDate.HasValue ? Query().Where(x => x.CreateDate >= sinceDate) : null; + + PagedModel result = GetItemsByEntityInner( + keyToIdAttempt.Result, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + return Task.FromResult(result); } - /// - /// Returns paged items in the audit trail for a given entity - /// - /// - /// - /// - /// - /// - /// By default this will always be ordered descending (newest first) - /// - /// - /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query - /// or the custom filter - /// so we need to do that here - /// - /// - /// Optional filter to be applied - /// - /// - public IEnumerable GetPagedItemsByEntity( + /// + public Task> GetItemsByEntityAsync( int entityId, - long pageIndex, - int pageSize, - out long totalRecords, + int skip, + int take, Direction orderDirection = Direction.Descending, AuditType[]? auditTypeFilter = null, IQuery? customFilter = null) { - if (pageIndex < 0) - { - throw new ArgumentOutOfRangeException(nameof(pageIndex)); - } + ArgumentOutOfRangeException.ThrowIfNegative(skip); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(take); - if (pageSize <= 0) + if (entityId is Constants.System.Root or <= 0) { - throw new ArgumentOutOfRangeException(nameof(pageSize)); + return Task.FromResult(new PagedModel { Items = [], Total = 0 }); } - if (entityId == Constants.System.Root || entityId <= 0) - { - totalRecords = 0; - return Enumerable.Empty(); - } + PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageIndex, out var pageSize); + PagedModel result = GetItemsByEntityInner( + entityId, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); - using (ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.Id == entityId); - - return _auditRepository.GetPagedResultsByQuery(query, pageIndex, pageSize, out totalRecords, orderDirection, auditTypeFilter, customFilter); - } + return Task.FromResult(result); } - /// - /// Returns paged items in the audit trail for a given user - /// - /// - /// - /// - /// - /// - /// By default this will always be ordered descending (newest first) - /// - /// - /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query - /// or the custom filter - /// so we need to do that here - /// - /// - /// Optional filter to be applied - /// - /// - public IEnumerable GetPagedItemsByUser( - int userId, + /// + [Obsolete("Use GetItemsByEntityAsync() instead. Scheduled for removal in Umbraco 19.")] + public IEnumerable GetPagedItemsByEntity( + int entityId, long pageIndex, int pageSize, out long totalRecords, @@ -183,66 +204,36 @@ public IEnumerable GetPagedItemsByUser( AuditType[]? auditTypeFilter = null, IQuery? customFilter = null) { - if (pageIndex < 0) - { - throw new ArgumentOutOfRangeException(nameof(pageIndex)); - } - - if (pageSize <= 0) - { - throw new ArgumentOutOfRangeException(nameof(pageSize)); - } + ArgumentOutOfRangeException.ThrowIfNegative(pageIndex); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(pageSize); - if (userId < Constants.Security.SuperUserId) + if (entityId is Constants.System.Root or <= 0) { - totalRecords = 0; - return Enumerable.Empty(); + totalRecords = 0L; + return []; } - using (ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.UserId == userId); + PagedModel result = GetItemsByEntityInner( + entityId, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + totalRecords = result.Total; - return _auditRepository.GetPagedResultsByQuery(query, pageIndex, pageSize, out totalRecords, orderDirection, auditTypeFilter, customFilter); - } + return result.Items; } - public Task> GetItemsByKeyAsync( - Guid entityKey, - UmbracoObjectTypes entityType, - int skip, - int take, - Direction orderDirection = Direction.Descending, - DateTimeOffset? sinceDate = null, - AuditType[]? auditTypeFilter = null) - { - if (skip < 0) - { - throw new ArgumentOutOfRangeException(nameof(skip)); - } - - if (take <= 0) - { - throw new ArgumentOutOfRangeException(nameof(take)); - } - - Attempt keyToIdAttempt = _entityService.GetId(entityKey, entityType); - if (keyToIdAttempt.Success is false) - { - return Task.FromResult(new PagedModel { Items = Enumerable.Empty(), Total = 0 }); - } - - using (ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.Id == keyToIdAttempt.Result); - IQuery? customFilter = sinceDate.HasValue ? Query().Where(x => x.CreateDate >= sinceDate.Value.LocalDateTime) : null; - PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageNumber, out var pageSize); - - IEnumerable auditItems = _auditRepository.GetPagedResultsByQuery(query, pageNumber, pageSize, out var totalRecords, orderDirection, auditTypeFilter, customFilter); - return Task.FromResult(new PagedModel { Items = auditItems, Total = totalRecords }); - } - } + /// + [Obsolete("Use GetItemsByEntityAsync() instead. Scheduled for removal in Umbraco 19.")] + public IEnumerable GetLogs(int objectId) + { + PagedModel result = GetItemsByEntityInner(objectId, 0, int.MaxValue, Direction.Ascending); + return result.Items; + } + /// public async Task> GetPagedItemsByUserAsync( Guid userKey, int skip, @@ -251,32 +242,76 @@ public async Task> GetPagedItemsByUserAsync( AuditType[]? auditTypeFilter = null, DateTime? sinceDate = null) { - if (skip < 0) - { - throw new ArgumentOutOfRangeException(nameof(skip)); - } - - if (take <= 0) - { - throw new ArgumentOutOfRangeException(nameof(take)); - } + ArgumentOutOfRangeException.ThrowIfNegative(skip); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(take); IUser? user = await _userService.GetAsync(userKey); - if (user is null) { return new PagedModel(); } - using (ScopeProvider.CreateCoreScope(autoComplete: true)) - { - IQuery query = Query().Where(x => x.UserId == user.Id); - IQuery? customFilter = sinceDate.HasValue ? Query().Where(x => x.CreateDate >= sinceDate) : null; - PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageNumber, out var pageSize); + PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageIndex, out var pageSize); + IQuery? customFilter = + sinceDate.HasValue ? Query().Where(x => x.CreateDate >= sinceDate) : null; + + PagedModel result = GetItemsByUserInner( + user.Id, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + return result; + } - IEnumerable auditItems = _auditRepository.GetPagedResultsByQuery(query, pageNumber, pageSize, out var totalRecords, orderDirection, auditTypeFilter, customFilter); - return new PagedModel { Items = auditItems, Total = totalRecords }; + /// + [Obsolete("Use GetPagedItemsByUserAsync() instead. Scheduled for removal in Umbraco 19.")] + public IEnumerable GetPagedItemsByUser( + int userId, + long pageIndex, + int pageSize, + out long totalRecords, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + IQuery? customFilter = null) + { + ArgumentOutOfRangeException.ThrowIfNegative(pageIndex); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(pageSize); + + if (userId is Constants.System.Root or <= 0) + { + totalRecords = 0L; + return []; } + + PagedModel items = GetItemsByUserInner( + userId, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + totalRecords = items.Total; + + return items.Items; + } + + /// + [Obsolete("Use GetPagedItemsByUserAsync() instead. Scheduled for removal in Umbraco 19.")] + public IEnumerable GetUserLogs(int userId, AuditType type, DateTime? sinceDate = null) + { + IQuery? customFilter = sinceDate.HasValue + ? Query().Where(x => x.AuditType == type && x.CreateDate >= sinceDate) + : null; + PagedModel result = GetItemsByUserInner( + userId, + 0, + int.MaxValue, + Direction.Ascending, + [type], + customFilter); + return result.Items; } /// @@ -305,4 +340,97 @@ public IAuditEntry Write( eventType, eventDetails).GetAwaiter().GetResult().Result; } + + /// + public Task CleanLogsAsync(int maximumAgeOfLogsInMinutes) + { + CleanLogsInner(maximumAgeOfLogsInMinutes); + return Task.CompletedTask; + } + + /// + [Obsolete("Use CleanLogsAsync() instead. Scheduled for removal in Umbraco 19.")] + public void CleanLogs(int maximumAgeOfLogsInMinutes) + => CleanLogsInner(maximumAgeOfLogsInMinutes); + + private Attempt AddInner( + AuditType type, + int userId, + int objectId, + string? entityType, + string? comment = null, + string? parameters = null) + { + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + _auditRepository.Save(new AuditItem(objectId, type, userId, entityType, comment, parameters)); + scope.Complete(); + + return Attempt.Succeed(AuditLogOperationStatus.Success); + } + + private PagedModel GetItemsInner( + IQuery? query, + long pageIndex, + int pageSize, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + IQuery? customFilter = null) + { + using (ScopeProvider.CreateCoreScope(autoComplete: true)) + { + IEnumerable auditItems = _auditRepository.GetPagedResultsByQuery( + query ?? Query(), + pageIndex, + pageSize, + out var totalRecords, + orderDirection, + auditTypeFilter, + customFilter); + return new PagedModel { Items = auditItems, Total = totalRecords }; + } + } + + private PagedModel GetItemsByEntityInner( + int entityId, + long pageIndex, + int pageSize, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + IQuery? customFilter = null) + { + IQuery query = Query().Where(x => x.Id == entityId); + + PagedModel result = GetItemsInner( + query, + pageIndex, + pageSize, + orderDirection, + auditTypeFilter, + customFilter); + return result; + } + + private PagedModel GetItemsByUserInner( + int userId, + long pageIndex, + int pageSize, + Direction orderDirection = Direction.Descending, + AuditType[]? auditTypeFilter = null, + IQuery? customFilter = null) + { + if (userId is < Constants.Security.SuperUserId) + { + return new PagedModel { Items = [], Total = 0 }; + } + + IQuery query = Query().Where(x => x.UserId == userId); + return GetItemsInner(query, pageIndex, pageSize, orderDirection, auditTypeFilter, customFilter); + } + + private void CleanLogsInner(int maximumAgeOfLogsInMinutes) + { + using ICoreScope scope = ScopeProvider.CreateCoreScope(); + _auditRepository.CleanLogs(maximumAgeOfLogsInMinutes); + scope.Complete(); + } } diff --git a/src/Umbraco.Core/Services/IAuditService.cs b/src/Umbraco.Core/Services/IAuditService.cs index 0ca72e401527..eaca8dde697a 100644 --- a/src/Umbraco.Core/Services/IAuditService.cs +++ b/src/Umbraco.Core/Services/IAuditService.cs @@ -1,5 +1,6 @@ using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence.Querying; +using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Core.Services; @@ -8,65 +9,126 @@ namespace Umbraco.Cms.Core.Services; /// public interface IAuditService : IService { - void Add(AuditType type, int userId, int objectId, string? entityType, string comment, string? parameters = null); + /// + /// Adds an audit entry. + /// + /// The type of the audit. + /// The key of the user triggering the event. + /// The identifier of the affected object. + /// The entity type of the affected object. + /// The comment associated with the audit entry. + /// The parameters associated with the audit entry. + /// Result of the add audit log operation. + public Task> AddAsync( + AuditType type, + Guid userKey, + int objectId, + string? entityType, + string? comment = null, + string? parameters = null) => throw new NotImplementedException(); - IEnumerable GetLogs(int objectId); + [Obsolete("Use AddAsync() instead. Scheduled for removal in Umbraco 19.")] + void Add(AuditType type, int userId, int objectId, string? entityType, string comment, string? parameters = null); - IEnumerable GetUserLogs(int userId, AuditType type, DateTime? sinceDate = null); + /// + /// Returns paged items in the audit trail. + /// + /// The number of audit trail entries to skip. + /// The number of audit trail entries to take. + /// + /// By default, this will always be ordered descending (newest first). + /// + /// + /// If populated, will only return entries after this time. + /// + /// + /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query + /// or the custom filter, so we need to do that here. + /// + /// The paged audit logs. + public Task> GetItemsAsync( + int skip, + int take, + Direction orderDirection = Direction.Descending, + DateTimeOffset? sinceDate = null, + AuditType[]? auditTypeFilter = null) => throw new NotImplementedException(); + [Obsolete("Use GetItemsAsync() instead. Scheduled for removal in Umbraco 19.")] IEnumerable GetLogs(AuditType type, DateTime? sinceDate = null); - void CleanLogs(int maximumAgeOfLogsInMinutes); + /// + /// Returns paged items in the audit trail for a given entity. + /// + /// The key of the entity. + /// The entity type. + /// The number of audit trail entries to skip. + /// The number of audit trail entries to take. + /// + /// By default, this will always be ordered descending (newest first). + /// + /// + /// If populated, will only return entries after this time. + /// + /// + /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query + /// or the custom filter, so we need to do that here. + /// + /// The paged items in the audit trail for the specified entity. + Task> GetItemsByKeyAsync( + Guid entityKey, + UmbracoObjectTypes entityType, + int skip, + int take, + Direction orderDirection = Direction.Descending, + DateTimeOffset? sinceDate = null, + AuditType[]? auditTypeFilter = null) => throw new NotImplementedException(); /// - /// Returns paged items in the audit trail for a given entity + /// Returns paged items in the audit trail for a given entity. /// - /// - /// - /// - /// + /// The identifier of the entity. + /// The number of audit trail entries to skip. + /// The number of audit trail entries to take. /// - /// By default this will always be ordered descending (newest first) + /// By default, this will always be ordered descending (newest first). /// /// /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query - /// or the custom filter - /// so we need to do that here + /// or the custom filter, so we need to do that here. /// /// - /// Optional filter to be applied + /// Optional filter to be applied. /// - /// - IEnumerable GetPagedItemsByEntity( + /// The paged items in the audit trail for the specified entity. + public Task> GetItemsByEntityAsync( int entityId, - long pageIndex, - int pageSize, - out long totalRecords, + int skip, + int take, Direction orderDirection = Direction.Descending, AuditType[]? auditTypeFilter = null, - IQuery? customFilter = null); + IQuery? customFilter = null) => throw new NotImplementedException(); /// - /// Returns paged items in the audit trail for a given user + /// Returns paged items in the audit trail for a given entity. /// - /// - /// - /// - /// + /// The identifier of the entity. + /// The index of tha page (pagination). + /// The number of results to return. + /// The total number of records. /// - /// By default this will always be ordered descending (newest first) + /// By default, this will always be ordered descending (newest first). /// /// /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query - /// or the custom filter - /// so we need to do that here + /// or the custom filter, so we need to do that here. /// /// - /// Optional filter to be applied + /// Optional filter to be applied. /// - /// - IEnumerable GetPagedItemsByUser( - int userId, + /// The paged items in the audit trail for the specified entity. + [Obsolete("Use GetItemsByEntityAsync() instead. Scheduled for removal in Umbraco 19.")] + IEnumerable GetPagedItemsByEntity( + int entityId, long pageIndex, int pageSize, out long totalRecords, @@ -74,57 +136,63 @@ IEnumerable GetPagedItemsByUser( AuditType[]? auditTypeFilter = null, IQuery? customFilter = null); + [Obsolete("Use GetItemsByEntityAsync() instead. Scheduled for removal in Umbraco 19.")] + IEnumerable GetLogs(int objectId); + /// - /// Returns paged items in the audit trail for a given entity + /// Returns paged items in the audit trail for a given user. /// - /// The key of the entity - /// The entity type - /// The amount of audit trail entries to skip - /// The amount of audit trail entries to take + /// The key of the user. + /// The number of audit trail entries to skip. + /// The number of audit trail entries to take. /// - /// By default this will always be ordered descending (newest first) + /// By default, this will always be ordered descending (newest first). /// /// /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query - /// or the custom filter - /// so we need to do that here - /// - /// - /// If populated, will only return entries after this time. + /// or the custom filter, so we need to do that here. /// - /// - Task> GetItemsByKeyAsync( - Guid entityKey, - UmbracoObjectTypes entityType, + /// The date to filter the audit entries. + /// The paged items in the audit trail for the specified user. + Task> GetPagedItemsByUserAsync( + Guid userKey, int skip, int take, Direction orderDirection = Direction.Descending, - DateTimeOffset? sinceDate = null, - AuditType[]? auditTypeFilter = null) => throw new NotImplementedException(); + AuditType[]? auditTypeFilter = null, + DateTime? sinceDate = null) => throw new NotImplementedException(); /// - /// Returns paged items in the audit trail for a given user + /// Returns paged items in the audit trail for a given user. /// - /// - /// - /// + /// + /// + /// + /// /// - /// By default this will always be ordered descending (newest first) + /// By default this will always be ordered descending (newest first). /// /// /// Since we currently do not have enum support with our expression parser, we cannot query on AuditType in the query /// or the custom filter - /// so we need to do that here + /// so we need to do that here. + /// + /// + /// Optional filter to be applied. /// - /// /// - Task> GetPagedItemsByUserAsync( - Guid userKey, - int skip, - int take, + [Obsolete("Use GetPagedItemsByUserAsync() instead. Scheduled for removal in Umbraco 19.")] + IEnumerable GetPagedItemsByUser( + int userId, + long pageIndex, + int pageSize, + out long totalRecords, Direction orderDirection = Direction.Descending, AuditType[]? auditTypeFilter = null, - DateTime? sinceDate = null) => throw new NotImplementedException(); + IQuery? customFilter = null); + + [Obsolete("Use GetPagedItemsByUserAsync() instead. Scheduled for removal in Umbraco 19.")] + IEnumerable GetUserLogs(int userId, AuditType type, DateTime? sinceDate = null); /// /// Writes an audit entry for an audited event. @@ -144,7 +212,8 @@ Task> GetPagedItemsByUserAsync( /// /// /// Free-form details about the audited event. - [Obsolete("Use AuditEntryService.WriteAsync() instead. Scheduled for removal in Umbraco 18.")] + /// The created audit entry. + [Obsolete("Use IAuditEntryService.WriteAsync() instead. Scheduled for removal in Umbraco 19.")] IAuditEntry Write( int performingUserId, string perfomingDetails, @@ -154,4 +223,14 @@ IAuditEntry Write( string affectedDetails, string eventType, string eventDetails); + + /// + /// Cleans the audit logs older than the specified maximum age. + /// + /// The maximum age of logs in minutes. + /// Task representing the asynchronous operation. + public Task CleanLogsAsync(int maximumAgeOfLogsInMinutes) => throw new NotImplementedException(); + + [Obsolete("Use CleanLogsAsync() instead. Scheduled for removal in Umbraco 19.")] + void CleanLogs(int maximumAgeOfLogsInMinutes); } diff --git a/src/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJob.cs b/src/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJob.cs index 3fcbe8d3d656..725c66c2f8bb 100644 --- a/src/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJob.cs +++ b/src/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJob.cs @@ -21,18 +21,17 @@ namespace Umbraco.Cms.Infrastructure.BackgroundJobs.Jobs; /// public class LogScrubberJob : IRecurringBackgroundJob { - public TimeSpan Period { get => TimeSpan.FromHours(4); } - - // No-op event as the period never changes on this job - public event EventHandler PeriodChanged { add { } remove { } } - - private readonly IAuditService _auditService; private readonly ILogger _logger; private readonly IProfilingLogger _profilingLogger; private readonly ICoreScopeProvider _scopeProvider; private LoggingSettings _settings; + public TimeSpan Period => TimeSpan.FromHours(4); + + // No-op event as the period never changes on this job + public event EventHandler PeriodChanged { add { } remove { } } + /// /// Initializes a new instance of the class. /// @@ -48,7 +47,6 @@ public LogScrubberJob( ILogger logger, IProfilingLogger profilingLogger) { - _auditService = auditService; _settings = settings.CurrentValue; _scopeProvider = scopeProvider; @@ -57,17 +55,14 @@ public LogScrubberJob( settings.OnChange(x => _settings = x); } - public Task RunJobAsync() + public async Task RunJobAsync() { - // Ensure we use an explicit scope since we are running on a background thread. using (ICoreScope scope = _scopeProvider.CreateCoreScope()) using (_profilingLogger.DebugDuration("Log scrubbing executing", "Log scrubbing complete")) { - _auditService.CleanLogs((int)_settings.MaxLogAge.TotalMinutes); + await _auditService.CleanLogsAsync((int)_settings.MaxLogAge.TotalMinutes); _ = scope.Complete(); } - - return Task.CompletedTask; } } diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index 0d7ebc0d5056..9a4fa1f7a009 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -336,11 +336,11 @@ public static IUmbracoBuilder AddCoreNotifications(this IUmbracoBuilder builder) // add handlers for building content relations builder - .AddNotificationHandler() + .AddNotificationAsyncHandler() .AddNotificationHandler() - .AddNotificationHandler() + .AddNotificationAsyncHandler() .AddNotificationHandler() - .AddNotificationHandler(); + .AddNotificationAsyncHandler(); // add notification handlers for property editors builder diff --git a/src/Umbraco.Infrastructure/Events/RelateOnTrashNotificationHandler.cs b/src/Umbraco.Infrastructure/Events/RelateOnTrashNotificationHandler.cs index 1c2064a78f49..266857d088c5 100644 --- a/src/Umbraco.Infrastructure/Events/RelateOnTrashNotificationHandler.cs +++ b/src/Umbraco.Infrastructure/Events/RelateOnTrashNotificationHandler.cs @@ -2,6 +2,8 @@ // See LICENSE for more details. using System.Globalization; +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Scoping; @@ -15,14 +17,17 @@ namespace Umbraco.Cms.Core.Events; public sealed class RelateOnTrashNotificationHandler : INotificationHandler, INotificationHandler, + INotificationAsyncHandler, INotificationHandler, - INotificationHandler + INotificationHandler, + INotificationAsyncHandler { private readonly IAuditService _auditService; private readonly IEntityService _entityService; private readonly IRelationService _relationService; private readonly ICoreScopeProvider _scopeProvider; private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; + private readonly IUserIdKeyResolver _userIdKeyResolver; private readonly ILocalizedTextService _textService; public RelateOnTrashNotificationHandler( @@ -31,7 +36,8 @@ public RelateOnTrashNotificationHandler( ILocalizedTextService textService, IAuditService auditService, IScopeProvider scopeProvider, - IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, + IUserIdKeyResolver userIdKeyResolver) { _relationService = relationService; _entityService = entityService; @@ -39,6 +45,26 @@ public RelateOnTrashNotificationHandler( _auditService = auditService; _scopeProvider = scopeProvider; _backOfficeSecurityAccessor = backOfficeSecurityAccessor; + _userIdKeyResolver = userIdKeyResolver; + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled for removal in V19.")] + public RelateOnTrashNotificationHandler( + IRelationService relationService, + IEntityService entityService, + ILocalizedTextService textService, + IAuditService auditService, + IScopeProvider scopeProvider, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + : this( + relationService, + entityService, + textService, + auditService, + scopeProvider, + backOfficeSecurityAccessor, + StaticServiceProvider.Instance.GetRequiredService()) + { } public void Handle(ContentMovedNotification notification) @@ -57,7 +83,8 @@ public void Handle(ContentMovedNotification notification) } } - public void Handle(ContentMovedToRecycleBinNotification notification) + /// + public async Task HandleAsync(ContentMovedToRecycleBinNotification notification, CancellationToken cancellationToken) { using (ICoreScope scope = _scopeProvider.CreateCoreScope()) { @@ -91,9 +118,9 @@ public void Handle(ContentMovedToRecycleBinNotification notification) new Relation(originalParentId, item.Entity.Id, relationType); _relationService.Save(relation); - _auditService.Add( + await _auditService.AddAsync( AuditType.Delete, - _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Id ?? item.Entity.WriterId, + _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Key ?? await _userIdKeyResolver.GetAsync(item.Entity.WriterId), item.Entity.Id, UmbracoObjectTypes.Document.GetName(), string.Format(_textService.Localize("recycleBin", "contentTrashed"), item.Entity.Id, originalParentId)); @@ -104,6 +131,10 @@ public void Handle(ContentMovedToRecycleBinNotification notification) } } + [Obsolete("Use the INotificationAsyncHandler.HandleAsync implementation instead. Scheduled for removal in V19.")] + public void Handle(ContentMovedToRecycleBinNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); + public void Handle(MediaMovedNotification notification) { foreach (MoveEventInfo item in notification.MoveInfoCollection.Where(x => @@ -119,7 +150,8 @@ public void Handle(MediaMovedNotification notification) } } - public void Handle(MediaMovedToRecycleBinNotification notification) + /// + public async Task HandleAsync(MediaMovedToRecycleBinNotification notification, CancellationToken cancellationToken) { using (ICoreScope scope = _scopeProvider.CreateCoreScope()) { @@ -151,9 +183,9 @@ public void Handle(MediaMovedToRecycleBinNotification notification) _relationService.GetByParentAndChildId(originalParentId, item.Entity.Id, relationType) ?? new Relation(originalParentId, item.Entity.Id, relationType); _relationService.Save(relation); - _auditService.Add( + await _auditService.AddAsync( AuditType.Delete, - _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Id ?? item.Entity.WriterId, + _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser?.Key ?? await _userIdKeyResolver.GetAsync(item.Entity.WriterId), item.Entity.Id, UmbracoObjectTypes.Media.GetName(), string.Format(_textService.Localize("recycleBin", "mediaTrashed"), item.Entity.Id, originalParentId)); @@ -163,4 +195,8 @@ public void Handle(MediaMovedToRecycleBinNotification notification) scope.Complete(); } } + + [Obsolete("Use the INotificationAsyncHandler.HandleAsync implementation instead. Scheduled for removal in V19.")] + public void Handle(MediaMovedToRecycleBinNotification notification) + => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); } diff --git a/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs b/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs index cd82fa6c8bb5..428ba23e0c6b 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs @@ -8,6 +8,7 @@ using Umbraco.Cms.Core.Extensions; using Umbraco.Cms.Core.Manifest; using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Models.Packaging; using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Packaging; @@ -82,7 +83,13 @@ public InstallationSummary InstallCompiledPackageData(XDocument? packageXml, int InstallationSummary summary = _packageInstallation.InstallPackageData(compiledPackage, userId, out _); - _auditService.Add(AuditType.PackagerInstall, userId, -1, "Package", $"Package data installed for package '{compiledPackage.Name}'."); + IUser? user = _userService.GetUserById(userId); + _auditService.AddAsync( + AuditType.PackagerInstall, + user?.Key ?? Constants.Security.SuperUserKey, + -1, + "Package", + $"Package data installed for package '{compiledPackage.Name}'.").GetAwaiter().GetResult(); // trigger the ImportedPackage event _eventAggregator.Publish(new ImportedPackageNotification(summary).WithStateFrom(importingPackageNotification)); @@ -115,8 +122,12 @@ public InstallationSummary InstallCompiledPackageData(FileInfo packageXmlFile, i return Attempt.FailWithStatus(PackageOperationStatus.NotFound, null); } - int currentUserId = (await _userService.GetRequiredUserAsync(userKey)).Id; - _auditService.Add(AuditType.Delete, currentUserId, -1, "Package", $"Created package '{package.Name}' deleted. Package key: {key}"); + Attempt result = await _auditService.AddAsync(AuditType.Delete, userKey, -1, "Package", $"Created package '{package.Name}' deleted. Package key: {key}"); + if (result is { Success: false, Result: AuditLogOperationStatus.UserNotFound }) + { + throw new InvalidOperationException($"Could not find user with key: {userKey}"); + } + _createdPackages.Delete(package.Id); scope.Complete(); @@ -163,8 +174,11 @@ public async Task> CreateCrea return Attempt.FailWithStatus(PackageOperationStatus.DuplicateItemName, package); } - int currentUserId = (await _userService.GetRequiredUserAsync(userKey)).Id; - _auditService.Add(AuditType.New, currentUserId, -1, "Package", $"Created package '{package.Name}' created. Package key: {package.PackageId}"); + Attempt result = await _auditService.AddAsync(AuditType.New, userKey, -1, "Package", $"Created package '{package.Name}' created. Package key: {package.PackageId}"); + if (result is { Success: false, Result: AuditLogOperationStatus.UserNotFound }) + { + throw new InvalidOperationException($"Could not find user with key: {userKey}"); + } scope.Complete(); @@ -180,8 +194,11 @@ public async Task> UpdateCrea return Attempt.FailWithStatus(PackageOperationStatus.NotFound, package); } - int currentUserId = (await _userService.GetRequiredUserAsync(userKey)).Id; - _auditService.Add(AuditType.New, currentUserId, -1, "Package", $"Created package '{package.Name}' updated. Package key: {package.PackageId}"); + Attempt result = await _auditService.AddAsync(AuditType.New, userKey, -1, "Package", $"Created package '{package.Name}' updated. Package key: {package.PackageId}"); + if (result is { Success: false, Result: AuditLogOperationStatus.UserNotFound }) + { + throw new InvalidOperationException($"Could not find user with key: {userKey}"); + } scope.Complete(); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs index 242be715570b..0c883614ef6b 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentServiceTests.cs @@ -1018,7 +1018,7 @@ public async Task Pending_Invariant_Property_Changes_Affect_Default_Language_Edi } [Test] - public void Can_Publish_Content_Variation_And_Detect_Changed_Cultures() + public async Task Can_Publish_Content_Variation_And_Detect_Changed_Cultures() { CreateEnglishAndFrenchDocumentType(out var langUk, out var langFr, out var contentType); @@ -1028,7 +1028,7 @@ public void Can_Publish_Content_Variation_And_Detect_Changed_Cultures() var published = ContentService.Publish(content, new[] { langFr.IsoCode }); // audit log will only show that french was published - var lastLog = AuditService.GetLogs(content.Id).Last(); + var lastLog = (await AuditService.GetItemsByEntityAsync(content.Id, 0, 1)).Items.First(); Assert.AreEqual("Published languages: fr-FR", lastLog.Comment); // re-get @@ -1038,7 +1038,7 @@ public void Can_Publish_Content_Variation_And_Detect_Changed_Cultures() published = ContentService.Publish(content, new[] { langUk.IsoCode }); // audit log will only show that english was published - lastLog = AuditService.GetLogs(content.Id).Last(); + lastLog = (await AuditService.GetItemsByEntityAsync(content.Id, 0, 1)).Items.First(); Assert.AreEqual("Published languages: en-GB", lastLog.Comment); } @@ -1075,7 +1075,7 @@ public async Task Can_Unpublish_Content_Variation_And_Detect_Changed_Cultures() var unpublished = ContentService.Unpublish(content, langFr.IsoCode); // audit log will only show that french was unpublished - var lastLog = AuditService.GetLogs(content.Id).Last(); + var lastLog = (await AuditService.GetItemsByEntityAsync(content.Id, 0, 1)).Items.First(); Assert.AreEqual("Unpublished languages: fr-FR", lastLog.Comment); // re-get @@ -1084,7 +1084,7 @@ public async Task Can_Unpublish_Content_Variation_And_Detect_Changed_Cultures() unpublished = ContentService.Unpublish(content, langGb.IsoCode); // audit log will only show that english was published - var logs = AuditService.GetLogs(content.Id).ToList(); + var logs = (await AuditService.GetItemsByEntityAsync(content.Id, 0, int.MaxValue, Direction.Ascending)).Items.ToList(); Assert.AreEqual("Unpublished languages: en-GB", logs[^2].Comment); Assert.AreEqual("Unpublished (mandatory language unpublished)", logs[^1].Comment); } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs index 248fa38a056e..21ece2832826 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs @@ -1,13 +1,12 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System.Linq; using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.Implement; -using Umbraco.Cms.Tests.Common.Builders; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; @@ -18,22 +17,26 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; internal sealed class AuditServiceTests : UmbracoIntegrationTest { [Test] - public void GetUserLogs() + public async Task GetUserLogs() { var sut = (AuditService)Services.GetRequiredService(); var eventDateUtc = DateTime.UtcNow.AddDays(-1); - var numberOfEntries = 10; + const int numberOfEntries = 10; for (var i = 0; i < numberOfEntries; i++) { eventDateUtc = eventDateUtc.AddMinutes(1); - sut.Add(AuditType.Unpublish, -1, 33, string.Empty, "blah"); + await sut.AddAsync(AuditType.Unpublish, Constants.Security.SuperUserKey, 33, string.Empty, "blah"); } - sut.Add(AuditType.Publish, -1, 33, string.Empty, "blah"); + await sut.AddAsync(AuditType.Publish, Constants.Security.SuperUserKey, 33, string.Empty, "blah"); - var logs = sut.GetUserLogs(-1, AuditType.Unpublish).ToArray(); + var logs = (await sut.GetPagedItemsByUserAsync( + Constants.Security.SuperUserKey, + 0, + int.MaxValue, + auditTypeFilter: [AuditType.Unpublish])).Items.ToArray(); Assert.Multiple(() => { diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs index 683ee8134bc7..4fca28f2ffd1 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.cs @@ -25,7 +25,7 @@ public void Relate(IContent parent, IContent child) } protected override void CustomTestSetup(IUmbracoBuilder builder) - => builder.AddNotificationHandler(); + => builder.AddNotificationAsyncHandler(); private ITemplateService TemplateService => GetRequiredService(); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs new file mode 100644 index 000000000000..65ddacd2397e --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs @@ -0,0 +1,250 @@ +using System.Data; +using AutoFixture; +using Microsoft.Extensions.Logging; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Persistence.Querying; +using Umbraco.Cms.Core.Persistence.Repositories; +using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Services.Implement; +using Umbraco.Cms.Core.Services.OperationStatus; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Services; + +[TestFixture] +public class AuditServiceTests +{ + private IAuditService _auditService; + private Mock _scopeProviderMock; + private Mock _auditRepositoryMock; + private Mock _entityServiceMock; + private Mock _userServiceMock; + + [SetUp] + public void Setup() + { + _scopeProviderMock = new Mock(MockBehavior.Strict); + _auditRepositoryMock = new Mock(MockBehavior.Strict); + _entityServiceMock = new Mock(MockBehavior.Strict); + _userServiceMock = new Mock(MockBehavior.Strict); + + _auditService = new AuditService( + _scopeProviderMock.Object, + Mock.Of(MockBehavior.Strict), + Mock.Of(MockBehavior.Strict), + _auditRepositoryMock.Object, + _userServiceMock.Object, + _entityServiceMock.Object); + } + + [TestCase(AuditType.Publish, 33, null, null, null)] + [TestCase(AuditType.Copy, 1, "entityType", "comment", "parameters")] + public async Task AddAsync_Calls_Repository_With_Correct_Values(AuditType type, int objectId, string? entityType, string? comment, string? parameters = null) + { + SetupScopeProviderMock(); + + _auditRepositoryMock.Setup(x => x.Save(It.IsAny())) + .Callback(item => + { + Assert.AreEqual(type, item.AuditType); + Assert.AreEqual(Constants.Security.SuperUserId, item.UserId); + Assert.AreEqual(objectId, item.Id); + Assert.AreEqual(entityType, item.EntityType); + Assert.AreEqual(comment, item.Comment); + Assert.AreEqual(parameters, item.Parameters); + }); + + Mock mockUser = new Mock(); + mockUser.Setup(x => x.Id).Returns(Constants.Security.SuperUserId); + + _userServiceMock.Setup(x => x.GetAsync(Constants.Security.SuperUserKey)).ReturnsAsync(mockUser.Object); + + var result = await _auditService.AddAsync( + type, + Constants.Security.SuperUserKey, + objectId, + entityType, + comment, + parameters); + _auditRepositoryMock.Verify(x => x.Save(It.IsAny()), Times.Once); + Assert.IsTrue(result.Success); + Assert.AreEqual(AuditLogOperationStatus.Success, result.Result); + } + + [Test] + public async Task AddAsync_Does_Not_Succeed_When_Non_Existing_User_Is_Provided() + { + _userServiceMock.Setup(x => x.GetAsync(It.IsAny())).ReturnsAsync((IUser?)null); + + var result = await _auditService.AddAsync( + AuditType.Publish, + Guid.Parse("00000000-0000-0000-0000-000000000001"), + 1, + "entityType", + "comment", + "parameters"); + Assert.IsFalse(result.Success); + Assert.AreEqual(AuditLogOperationStatus.UserNotFound, result.Result); + } + + [Test] + public void GetItemsAsync_Throws_When_Invalid_Pagination_Arguments_Are_Provided() + { + Assert.ThrowsAsync(async () => await _auditService.GetItemsAsync(-1, 10), "Skip must be greater than or equal to 0."); + Assert.ThrowsAsync(async () => await _auditService.GetItemsAsync(0, -1), "Take must be greater than 0."); + Assert.ThrowsAsync(async () => await _auditService.GetItemsAsync(0, 0), "Take must be greater than 0."); + } + + [Test] + public async Task GetItemsAsync_Returns_Correct_Total_And_Item_Count() + { + SetupScopeProviderMock(); + + Fixture fixture = new Fixture(); + long totalRecords = 12; + _auditRepositoryMock.Setup(x => x.GetPagedResultsByQuery( + It.IsAny>(), + 2, + 5, + out totalRecords, + Direction.Descending, + null, + null)) + .Returns(fixture.CreateMany(count: 2)); + + _scopeProviderMock.Setup(x => x.CreateQuery()).Returns(Mock.Of>()); + + var result = await _auditService.GetItemsAsync(10, 5); + Assert.AreEqual(totalRecords, result.Total); + Assert.AreEqual(2, result.Items.Count()); + } + + [Test] + public void GetItemsByKeyAsync_Throws_When_Invalid_Pagination_Arguments_Are_Provided() + { + Assert.ThrowsAsync(async () => await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, -1, 10), "Skip must be greater than or equal to 0."); + Assert.ThrowsAsync(async () => await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, 0, -1), "Take must be greater than 0."); + Assert.ThrowsAsync(async () => await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, 0, 0), "Take must be greater than 0."); + } + + [Test] + public async Task GetItemsByKeyAsync_Returns_No_Results_When_Key_Is_Not_Found() + { + SetupScopeProviderMock(); + + _entityServiceMock.Setup(x => x.GetId(Guid.Empty, UmbracoObjectTypes.Document)).Returns(Attempt.Fail()); + + var result = await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, 10, 10); + Assert.AreEqual(0, result.Total); + Assert.AreEqual(0, result.Items.Count()); + } + + [Test] + public async Task GetItemsByKeyAsync_Returns_Correct_Total_And_Item_Count() + { + SetupScopeProviderMock(); + + _entityServiceMock.Setup(x => x.GetId(Guid.Empty, UmbracoObjectTypes.Document)).Returns(Attempt.Succeed(2)); + + Fixture fixture = new Fixture(); + long totalRecords = 12; + + // TODO: Test whether the provided query has a filter on id + _auditRepositoryMock.Setup(x => x.GetPagedResultsByQuery( + It.IsAny>(), + 2, + 5, + out totalRecords, + Direction.Descending, + null, + null)) + .Returns(fixture.CreateMany(count: 2)); + + _scopeProviderMock.Setup(x => x.CreateQuery()) + .Returns(Mock.Of>()); + + var result = await _auditService.GetItemsByKeyAsync(Guid.Empty, UmbracoObjectTypes.Document, 10, 5); + Assert.AreEqual(totalRecords, result.Total); + Assert.AreEqual(2, result.Items.Count()); + } + + [Test] + public async Task GetItemsByEntityAsync_Returns_No_Results_When_Key_Is_Not_Found() + { + SetupScopeProviderMock(); + + _entityServiceMock.Setup(x => x.GetId(Guid.Empty, UmbracoObjectTypes.Document)).Returns(Attempt.Fail()); + + var result = await _auditService.GetItemsByEntityAsync(-1, 10, 10); + Assert.AreEqual(0, result.Total); + Assert.AreEqual(0, result.Items.Count()); + } + + [TestCase(Constants.System.Root)] + [TestCase(-100)] + public async Task GetItemsByEntityAsync_Returns_No_Results_When_Id_Is_Root_Or_Lower(int userId) + { + SetupScopeProviderMock(); + + _entityServiceMock.Setup(x => x.GetId(Guid.Empty, UmbracoObjectTypes.Document)).Returns(Attempt.Fail()); + + var result = await _auditService.GetItemsByEntityAsync(userId, 10, 10); + Assert.AreEqual(0, result.Total); + Assert.AreEqual(0, result.Items.Count()); + } + + [Test] + public async Task GetItemsByEntityAsync_Returns_Correct_Total_And_Item_Count() + { + SetupScopeProviderMock(); + + _entityServiceMock.Setup(x => x.GetId(Guid.Empty, UmbracoObjectTypes.Document)).Returns(Attempt.Succeed(2)); + + Fixture fixture = new Fixture(); + long totalRecords = 12; + + // TODO: Test whether the provided query has a filter on id + _auditRepositoryMock.Setup(x => x.GetPagedResultsByQuery( + It.IsAny>(), + 2, + 5, + out totalRecords, + Direction.Descending, + null, + null)) + .Returns(fixture.CreateMany(count: 2)); + + _scopeProviderMock.Setup(x => x.CreateQuery()) + .Returns(Mock.Of>()); + + var result = await _auditService.GetItemsByEntityAsync(1, 10, 5); + Assert.AreEqual(totalRecords, result.Total); + Assert.AreEqual(2, result.Items.Count()); + } + + [Test] + public async Task CleanLogsAsync_Calls_Repository_With_Correct_Values() + { + SetupScopeProviderMock(); + _auditRepositoryMock.Setup(x => x.CleanLogs(100)); + await _auditService.CleanLogsAsync(100); + _auditRepositoryMock.Verify(x => x.CleanLogs(It.IsAny()), Times.Once); + } + + private void SetupScopeProviderMock() => + _scopeProviderMock + .Setup(x => x.CreateCoreScope( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(Mock.Of()); +} diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJobTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJobTests.cs index 6dd479364c3b..515111cd64fa 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJobTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/BackgroundJobs/Jobs/LogScrubberJobTests.cs @@ -68,5 +68,5 @@ private LogScrubberJob CreateLogScrubber() private void VerifyLogsScrubbed() => VerifyLogsScrubbed(Times.Once()); private void VerifyLogsScrubbed(Times times) => - _mockAuditService.Verify(x => x.CleanLogs(It.Is(y => y == MaxLogAgeInMinutes)), times); + _mockAuditService.Verify(x => x.CleanLogsAsync(It.Is(y => y == MaxLogAgeInMinutes)), times); } From a4de0b6188c293bb31cb4984d8d45275bb977561 Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Mon, 19 May 2025 12:01:01 +0200 Subject: [PATCH 03/12] Use the audit service instead of the repository directly in services --- src/Umbraco.Core/Services/AuditService.cs | 35 ++---- .../ContentBlueprintContainerService.cs | 4 +- src/Umbraco.Core/Services/ContentService.cs | 51 +++++--- .../Services/ContentTypeContainerService.cs | 4 +- .../Services/ContentTypeService.cs | 63 +++++++++- ...peServiceBaseOfTRepositoryTItemTService.cs | 49 +++++++- .../Services/ContentVersionService.cs | 23 ++-- .../Services/DataTypeContainerService.cs | 4 +- src/Umbraco.Core/Services/DataTypeService.cs | 115 ++++++++++++------ .../Services/DictionaryItemService.cs | 24 ++-- .../Services/EntityTypeContainerService.cs | 20 +-- src/Umbraco.Core/Services/FileService.cs | 88 +++++++++++++- .../Services/FileServiceOperationBase.cs | 44 +++++-- src/Umbraco.Core/Services/LanguageService.cs | 21 ++-- .../Services/LocalizationService.cs | 5 - src/Umbraco.Core/Services/MediaService.cs | 78 +++++++++++- .../Services/MediaTypeContainerService.cs | 4 +- src/Umbraco.Core/Services/MediaTypeService.cs | 67 +++++++++- src/Umbraco.Core/Services/MemberService.cs | 83 ++++++++++++- .../Services/MemberTypeService.cs | 63 +++++++++- .../Services/PartialViewService.cs | 51 +++++++- src/Umbraco.Core/Services/RelationService.cs | 79 ++++++++++-- src/Umbraco.Core/Services/ScriptService.cs | 47 ++++++- .../Services/StylesheetService.cs | 47 ++++++- src/Umbraco.Core/Services/TemplateService.cs | 73 ++++++++--- .../Services/AuditServiceTests.cs | 14 +-- 26 files changed, 959 insertions(+), 197 deletions(-) diff --git a/src/Umbraco.Core/Services/AuditService.cs b/src/Umbraco.Core/Services/AuditService.cs index 1b5e2c4b2152..6833d49c6ef1 100644 --- a/src/Umbraco.Core/Services/AuditService.cs +++ b/src/Umbraco.Core/Services/AuditService.cs @@ -16,44 +16,27 @@ namespace Umbraco.Cms.Core.Services.Implement; /// public sealed class AuditService : RepositoryService, IAuditService { + private readonly IUserIdKeyResolver _userIdKeyResolver; private readonly IAuditRepository _auditRepository; private readonly IEntityService _entityService; - private readonly IUserService _userService; /// /// Initializes a new instance of the class. /// - [ActivatorUtilitiesConstructor] public AuditService( ICoreScopeProvider provider, ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IAuditRepository auditRepository, - IUserService userService, + IUserIdKeyResolver userIdKeyResolver, IEntityService entityService) : base(provider, loggerFactory, eventMessagesFactory) { _auditRepository = auditRepository; - _userService = userService; + _userIdKeyResolver = userIdKeyResolver; _entityService = entityService; } - /// - /// Initializes a new instance of the class. - /// - [Obsolete("Use the non-obsolete constructor. Scheduled for removal in Umbraco 19.")] - public AuditService( - ICoreScopeProvider provider, - ILoggerFactory loggerFactory, - IEventMessagesFactory eventMessagesFactory, - IAuditRepository auditRepository, - IAuditEntryRepository auditEntryRepository, - IUserService userService, - IEntityService entityService) - : this(provider, loggerFactory, eventMessagesFactory, auditRepository, userService, entityService) - { - } - /// public async Task> AddAsync( AuditType type, @@ -63,10 +46,12 @@ public async Task> AddAsync( string? comment = null, string? parameters = null) { - var userId = userKey switch + int? userId = userKey switch { { } when userKey == Constants.Security.UnknownUserKey => Constants.Security.UnknownUserId, - _ => (await _userService.GetAsync(userKey))?.Id, + _ => await _userIdKeyResolver.TryGetAsync(userKey) is { Success: true } userIdAttempt + ? userIdAttempt.Result + : null, }; if (userId is null) @@ -245,8 +230,7 @@ public async Task> GetPagedItemsByUserAsync( ArgumentOutOfRangeException.ThrowIfNegative(skip); ArgumentOutOfRangeException.ThrowIfNegativeOrZero(take); - IUser? user = await _userService.GetAsync(userKey); - if (user is null) + if (await _userIdKeyResolver.TryGetAsync(userKey) is not { Success: true } userIdAttempt) { return new PagedModel(); } @@ -256,7 +240,7 @@ public async Task> GetPagedItemsByUserAsync( sinceDate.HasValue ? Query().Where(x => x.CreateDate >= sinceDate) : null; PagedModel result = GetItemsByUserInner( - user.Id, + userIdAttempt.Result, pageIndex, pageSize, orderDirection, @@ -434,3 +418,4 @@ private void CleanLogsInner(int maximumAgeOfLogsInMinutes) scope.Complete(); } } + diff --git a/src/Umbraco.Core/Services/ContentBlueprintContainerService.cs b/src/Umbraco.Core/Services/ContentBlueprintContainerService.cs index e8d742b70ee1..046c4756324e 100644 --- a/src/Umbraco.Core/Services/ContentBlueprintContainerService.cs +++ b/src/Umbraco.Core/Services/ContentBlueprintContainerService.cs @@ -13,10 +13,10 @@ public ContentBlueprintContainerService( ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IDocumentBlueprintContainerRepository entityContainerRepository, - IAuditRepository auditRepository, + IAuditService auditService, IEntityRepository entityRepository, IUserIdKeyResolver userIdKeyResolver) - : base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditRepository, entityRepository, userIdKeyResolver) + : base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditService, entityRepository, userIdKeyResolver) { } diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index c72218e8f33f..0a06f50b43fe 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -27,7 +27,7 @@ namespace Umbraco.Cms.Core.Services; /// public class ContentService : RepositoryService, IContentService { - private readonly IAuditRepository _auditRepository; + private readonly IAuditService _auditService; private readonly IContentTypeRepository _contentTypeRepository; private readonly IDocumentBlueprintRepository _documentBlueprintRepository; private readonly IDocumentRepository _documentRepository; @@ -52,7 +52,7 @@ public ContentService( IEventMessagesFactory eventMessagesFactory, IDocumentRepository documentRepository, IEntityRepository entityRepository, - IAuditRepository auditRepository, + IAuditService auditService, IContentTypeRepository contentTypeRepository, IDocumentBlueprintRepository documentBlueprintRepository, ILanguageRepository languageRepository, @@ -68,7 +68,7 @@ public ContentService( { _documentRepository = documentRepository; _entityRepository = entityRepository; - _auditRepository = auditRepository; + _auditService = auditService; _contentTypeRepository = contentTypeRepository; _documentBlueprintRepository = documentBlueprintRepository; _languageRepository = languageRepository; @@ -87,8 +87,7 @@ public ContentService( _logger = loggerFactory.CreateLogger(); } - [Obsolete("Use non-obsolete constructor. Scheduled for removal in V17.")] - + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] public ContentService( ICoreScopeProvider provider, ILoggerFactory loggerFactory, @@ -104,14 +103,16 @@ public ContentService( ICultureImpactFactory cultureImpactFactory, IUserIdKeyResolver userIdKeyResolver, PropertyEditorCollection propertyEditorCollection, - IIdKeyMap idKeyMap) + IIdKeyMap idKeyMap, + IOptionsMonitor optionsMonitor, + IRelationService relationService) : this( provider, loggerFactory, eventMessagesFactory, documentRepository, entityRepository, - auditRepository, + StaticServiceProvider.Instance.GetRequiredService(), contentTypeRepository, documentBlueprintRepository, languageRepository, @@ -121,11 +122,12 @@ public ContentService( userIdKeyResolver, propertyEditorCollection, idKeyMap, - StaticServiceProvider.Instance.GetRequiredService>(), - StaticServiceProvider.Instance.GetRequiredService()) + optionsMonitor, + relationService) { } - [Obsolete("Use non-obsolete constructor. Scheduled for removal in V17.")] + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] public ContentService( ICoreScopeProvider provider, ILoggerFactory loggerFactory, @@ -133,6 +135,7 @@ public ContentService( IDocumentRepository documentRepository, IEntityRepository entityRepository, IAuditRepository auditRepository, + IAuditService auditService, IContentTypeRepository contentTypeRepository, IDocumentBlueprintRepository documentBlueprintRepository, ILanguageRepository languageRepository, @@ -140,14 +143,17 @@ public ContentService( IShortStringHelper shortStringHelper, ICultureImpactFactory cultureImpactFactory, IUserIdKeyResolver userIdKeyResolver, - PropertyEditorCollection propertyEditorCollection) + PropertyEditorCollection propertyEditorCollection, + IIdKeyMap idKeyMap, + IOptionsMonitor optionsMonitor, + IRelationService relationService) : this( provider, loggerFactory, eventMessagesFactory, documentRepository, entityRepository, - auditRepository, + auditService, contentTypeRepository, documentBlueprintRepository, languageRepository, @@ -156,7 +162,9 @@ public ContentService( cultureImpactFactory, userIdKeyResolver, propertyEditorCollection, - StaticServiceProvider.Instance.GetRequiredService()) + idKeyMap, + optionsMonitor, + relationService) { } @@ -3082,7 +3090,22 @@ internal IEnumerable GetPublishedDescendantsLocked(IContent content) #region Private Methods private void Audit(AuditType type, int userId, int objectId, string? message = null, string? parameters = null) => - _auditRepository.Save(new AuditItem(objectId, type, userId, UmbracoObjectTypes.Document.GetName(), message, parameters)); + AuditAsync(type, userId, objectId, message, parameters).GetAwaiter().GetResult(); + + private async Task AuditAsync(AuditType type, int userId, int objectId, string? message = null, string? parameters = null) + { + Guid userKey = await _userIdKeyResolver.TryGetAsync(userId) is { Success: true } userKeyAttempt + ? userKeyAttempt.Result + : Constants.Security.UnknownUserKey; + + await _auditService.AddAsync( + type, + userKey, + objectId, + UmbracoObjectTypes.Document.GetName(), + message, + parameters); + } private string GetLanguageDetailsForAuditEntry(IEnumerable affectedCultures) => GetLanguageDetailsForAuditEntry(_languageRepository.GetMany(), affectedCultures); diff --git a/src/Umbraco.Core/Services/ContentTypeContainerService.cs b/src/Umbraco.Core/Services/ContentTypeContainerService.cs index a6ac5e811f6a..6258216c8e9c 100644 --- a/src/Umbraco.Core/Services/ContentTypeContainerService.cs +++ b/src/Umbraco.Core/Services/ContentTypeContainerService.cs @@ -14,10 +14,10 @@ public ContentTypeContainerService( ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IDocumentTypeContainerRepository entityContainerRepository, - IAuditRepository auditRepository, + IAuditService auditService, IEntityRepository entityRepository, IUserIdKeyResolver userIdKeyResolver) - : base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditRepository, entityRepository, userIdKeyResolver) + : base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditService, entityRepository, userIdKeyResolver) { } diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index 6840d18bfe9a..cae2f1c134f8 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -1,4 +1,6 @@ +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; @@ -22,7 +24,7 @@ public ContentTypeService( IEventMessagesFactory eventMessagesFactory, IContentService contentService, IContentTypeRepository repository, - IAuditRepository auditRepository, + IAuditService auditService, IDocumentTypeContainerRepository entityContainerRepository, IEntityRepository entityRepository, IEventAggregator eventAggregator, @@ -33,7 +35,7 @@ public ContentTypeService( loggerFactory, eventMessagesFactory, repository, - auditRepository, + auditService, entityContainerRepository, entityRepository, eventAggregator, @@ -41,6 +43,63 @@ public ContentTypeService( contentTypeFilters) => ContentService = contentService; + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public ContentTypeService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IContentService contentService, + IContentTypeRepository repository, + IAuditRepository auditRepository, + IDocumentTypeContainerRepository entityContainerRepository, + IEntityRepository entityRepository, + IEventAggregator eventAggregator, + IUserIdKeyResolver userIdKeyResolver, + ContentTypeFilterCollection contentTypeFilters) + : this( + provider, + loggerFactory, + eventMessagesFactory, + contentService, + repository, + StaticServiceProvider.Instance.GetRequiredService(), + entityContainerRepository, + entityRepository, + eventAggregator, + userIdKeyResolver, + contentTypeFilters) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public ContentTypeService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IContentService contentService, + IContentTypeRepository repository, + IAuditRepository auditRepository, + IAuditService auditService, + IDocumentTypeContainerRepository entityContainerRepository, + IEntityRepository entityRepository, + IEventAggregator eventAggregator, + IUserIdKeyResolver userIdKeyResolver, + ContentTypeFilterCollection contentTypeFilters) + : this( + provider, + loggerFactory, + eventMessagesFactory, + contentService, + repository, + auditService, + entityContainerRepository, + entityRepository, + eventAggregator, + userIdKeyResolver, + contentTypeFilters) + { + } + protected override int[] ReadLockIds => ContentTypeLocks.ReadLockIds; protected override int[] WriteLockIds => ContentTypeLocks.WriteLockIds; diff --git a/src/Umbraco.Core/Services/ContentTypeServiceBaseOfTRepositoryTItemTService.cs b/src/Umbraco.Core/Services/ContentTypeServiceBaseOfTRepositoryTItemTService.cs index cd72be54ab85..3b068debf1c3 100644 --- a/src/Umbraco.Core/Services/ContentTypeServiceBaseOfTRepositoryTItemTService.cs +++ b/src/Umbraco.Core/Services/ContentTypeServiceBaseOfTRepositoryTItemTService.cs @@ -1,6 +1,8 @@ using System.Globalization; using System.Runtime.InteropServices; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Exceptions; using Umbraco.Cms.Core.Models; @@ -20,7 +22,7 @@ public abstract class ContentTypeServiceBase : ContentTypeSe where TRepository : IContentTypeRepositoryBase where TItem : class, IContentTypeComposition { - private readonly IAuditRepository _auditRepository; + private readonly IAuditService _auditService; private readonly IEntityContainerRepository _containerRepository; private readonly IEntityRepository _entityRepository; private readonly IEventAggregator _eventAggregator; @@ -32,7 +34,7 @@ protected ContentTypeServiceBase( ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, TRepository repository, - IAuditRepository auditRepository, + IAuditService auditService, IEntityContainerRepository containerRepository, IEntityRepository entityRepository, IEventAggregator eventAggregator, @@ -41,7 +43,7 @@ protected ContentTypeServiceBase( : base(provider, loggerFactory, eventMessagesFactory) { Repository = repository; - _auditRepository = auditRepository; + _auditService = auditService; _containerRepository = containerRepository; _entityRepository = entityRepository; _eventAggregator = eventAggregator; @@ -49,6 +51,32 @@ protected ContentTypeServiceBase( _contentTypeFilters = contentTypeFilters; } + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + protected ContentTypeServiceBase( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + TRepository repository, + IAuditRepository auditRepository, + IEntityContainerRepository containerRepository, + IEntityRepository entityRepository, + IEventAggregator eventAggregator, + IUserIdKeyResolver userIdKeyResolver, + ContentTypeFilterCollection contentTypeFilters) + : this( + provider, + loggerFactory, + eventMessagesFactory, + repository, + StaticServiceProvider.Instance.GetRequiredService(), + containerRepository, + entityRepository, + eventAggregator, + userIdKeyResolver, + contentTypeFilters) + { + } + protected TRepository Repository { get; } protected abstract int[] WriteLockIds { get; } protected abstract int[] ReadLockIds { get; } @@ -1398,9 +1426,20 @@ public IEnumerable GetContainers(string name, int level) #region Audit - private void Audit(AuditType type, int userId, int objectId) + private void Audit(AuditType type, int userId, int objectId) => + AuditAsync(type, userId, objectId).GetAwaiter().GetResult(); + + private async Task AuditAsync(AuditType type, int userId, int objectId) { - _auditRepository.Save(new AuditItem(objectId, type, userId, ObjectTypes.GetUmbracoObjectType(ContainedObjectType).GetName())); + Guid userKey = await _userIdKeyResolver.TryGetAsync(userId) is { Success: true } userKeyAttempt + ? userKeyAttempt.Result + : Constants.Security.UnknownUserKey; + + await _auditService.AddAsync( + type, + userKey, + objectId, + ObjectTypes.GetUmbracoObjectType(ContainedObjectType).GetName()); } #endregion diff --git a/src/Umbraco.Core/Services/ContentVersionService.cs b/src/Umbraco.Core/Services/ContentVersionService.cs index 9d5d569b3a55..02ceab84d106 100644 --- a/src/Umbraco.Core/Services/ContentVersionService.cs +++ b/src/Umbraco.Core/Services/ContentVersionService.cs @@ -15,7 +15,7 @@ namespace Umbraco.Cms.Core.Services; internal class ContentVersionService : IContentVersionService { - private readonly IAuditRepository _auditRepository; + private readonly IAuditService _auditService; private readonly IContentVersionCleanupPolicy _contentVersionCleanupPolicy; private readonly IDocumentVersionRepository _documentVersionRepository; private readonly IEventMessagesFactory _eventMessagesFactory; @@ -32,7 +32,7 @@ public ContentVersionService( IContentVersionCleanupPolicy contentVersionCleanupPolicy, ICoreScopeProvider scopeProvider, IEventMessagesFactory eventMessagesFactory, - IAuditRepository auditRepository, + IAuditService auditService, ILanguageRepository languageRepository, IEntityService entityService, IContentService contentService, @@ -43,7 +43,7 @@ public ContentVersionService( _contentVersionCleanupPolicy = contentVersionCleanupPolicy; _scopeProvider = scopeProvider; _eventMessagesFactory = eventMessagesFactory; - _auditRepository = auditRepository; + _auditService = auditService; _languageRepository = languageRepository; _entityService = entityService; _contentService = contentService; @@ -299,16 +299,21 @@ private IReadOnlyCollection CleanupDocumentVersions(DateTime return versionsToDelete; } - private void Audit(AuditType type, int userId, int objectId, string? message = null, string? parameters = null) + private void Audit(AuditType type, int userId, int objectId, string? message = null, string? parameters = null) => + AuditAsync(type, userId, objectId, message, parameters).GetAwaiter().GetResult(); + + private async Task AuditAsync(AuditType type, int userId, int objectId, string? message = null, string? parameters = null) { - var entry = new AuditItem( - objectId, + Guid userKey = await _userIdKeyResolver.TryGetAsync(userId) is { Success: true } userKeyAttempt + ? userKeyAttempt.Result + : Constants.Security.UnknownUserKey; + + await _auditService.AddAsync( type, - userId, + userKey, + objectId, UmbracoObjectTypes.Document.GetName(), message, parameters); - - _auditRepository.Save(entry); } } diff --git a/src/Umbraco.Core/Services/DataTypeContainerService.cs b/src/Umbraco.Core/Services/DataTypeContainerService.cs index 0e6292f7f1fa..8b892226efe0 100644 --- a/src/Umbraco.Core/Services/DataTypeContainerService.cs +++ b/src/Umbraco.Core/Services/DataTypeContainerService.cs @@ -13,10 +13,10 @@ public DataTypeContainerService( ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IDataTypeContainerRepository entityContainerRepository, - IAuditRepository auditRepository, + IAuditService auditService, IEntityRepository entityRepository, IUserIdKeyResolver userIdKeyResolver) - : base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditRepository, entityRepository, userIdKeyResolver) + : base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditService, entityRepository, userIdKeyResolver) { } diff --git a/src/Umbraco.Core/Services/DataTypeService.cs b/src/Umbraco.Core/Services/DataTypeService.cs index 83effe740069..fc1ac5f1dce8 100644 --- a/src/Umbraco.Core/Services/DataTypeService.cs +++ b/src/Umbraco.Core/Services/DataTypeService.cs @@ -27,13 +27,42 @@ public class DataTypeService : RepositoryService, IDataTypeService private readonly IContentTypeRepository _contentTypeRepository; private readonly IMediaTypeRepository _mediaTypeRepository; private readonly IMemberTypeRepository _memberTypeRepository; - private readonly IAuditRepository _auditRepository; + private readonly IAuditService _auditService; private readonly IIOHelper _ioHelper; private readonly IDataTypeContainerService _dataTypeContainerService; private readonly IUserIdKeyResolver _userIdKeyResolver; private readonly Lazy _idKeyMap; - [Obsolete("Please use the constructor taking all parameters. Scheduled for removal in Umbraco 17.")] + public DataTypeService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IDataTypeRepository dataTypeRepository, + IDataValueEditorFactory dataValueEditorFactory, + IAuditService auditService, + IContentTypeRepository contentTypeRepository, + IMediaTypeRepository mediaTypeRepository, + IMemberTypeRepository memberTypeRepository, + IIOHelper ioHelper, + Lazy idKeyMap) + : base(provider, loggerFactory, eventMessagesFactory) + { + _dataValueEditorFactory = dataValueEditorFactory; + _dataTypeRepository = dataTypeRepository; + _auditService = auditService; + _contentTypeRepository = contentTypeRepository; + _mediaTypeRepository = mediaTypeRepository; + _memberTypeRepository = memberTypeRepository; + _ioHelper = ioHelper; + _idKeyMap = idKeyMap; + + // resolve dependencies for obsolete methods through the static service provider, so they don't pollute the constructor signature + _dataTypeContainerService = StaticServiceProvider.Instance.GetRequiredService(); + _dataTypeContainerRepository = StaticServiceProvider.Instance.GetRequiredService(); + _userIdKeyResolver = StaticServiceProvider.Instance.GetRequiredService(); + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] public DataTypeService( ICoreScopeProvider provider, ILoggerFactory loggerFactory, @@ -42,23 +71,26 @@ public DataTypeService( IDataValueEditorFactory dataValueEditorFactory, IAuditRepository auditRepository, IContentTypeRepository contentTypeRepository, + IMediaTypeRepository mediaTypeRepository, + IMemberTypeRepository memberTypeRepository, IIOHelper ioHelper, Lazy idKeyMap) : this( - provider, - loggerFactory, - eventMessagesFactory, - dataTypeRepository, - dataValueEditorFactory, - auditRepository, - contentTypeRepository, - StaticServiceProvider.Instance.GetRequiredService(), - StaticServiceProvider.Instance.GetRequiredService(), - ioHelper, - idKeyMap) + provider, + loggerFactory, + eventMessagesFactory, + dataTypeRepository, + dataValueEditorFactory, + StaticServiceProvider.Instance.GetRequiredService(), + contentTypeRepository, + mediaTypeRepository, + memberTypeRepository, + ioHelper, + idKeyMap) { } + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] public DataTypeService( ICoreScopeProvider provider, ILoggerFactory loggerFactory, @@ -66,26 +98,25 @@ public DataTypeService( IDataTypeRepository dataTypeRepository, IDataValueEditorFactory dataValueEditorFactory, IAuditRepository auditRepository, + IAuditService auditService, IContentTypeRepository contentTypeRepository, IMediaTypeRepository mediaTypeRepository, IMemberTypeRepository memberTypeRepository, IIOHelper ioHelper, Lazy idKeyMap) - : base(provider, loggerFactory, eventMessagesFactory) + : this( + provider, + loggerFactory, + eventMessagesFactory, + dataTypeRepository, + dataValueEditorFactory, + auditService, + contentTypeRepository, + mediaTypeRepository, + memberTypeRepository, + ioHelper, + idKeyMap) { - _dataValueEditorFactory = dataValueEditorFactory; - _dataTypeRepository = dataTypeRepository; - _auditRepository = auditRepository; - _contentTypeRepository = contentTypeRepository; - _mediaTypeRepository = mediaTypeRepository; - _memberTypeRepository = memberTypeRepository; - _ioHelper = ioHelper; - _idKeyMap = idKeyMap; - - // resolve dependencies for obsolete methods through the static service provider, so they don't pollute the constructor signature - _dataTypeContainerService = StaticServiceProvider.Instance.GetRequiredService(); - _dataTypeContainerRepository = StaticServiceProvider.Instance.GetRequiredService(); - _userIdKeyResolver = StaticServiceProvider.Instance.GetRequiredService(); } #region Containers @@ -474,8 +505,7 @@ public async Task> MoveAsync(IDataTy scope.Notifications.Publish(new DataTypeMovedNotification(moveEventInfo, eventMessages).WithStateFrom(movingDataTypeNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(AuditType.Move, currentUserId, toMove.Id); + await AuditAsync(AuditType.Move, userKey, toMove.Id); scope.Complete(); } @@ -597,7 +627,7 @@ public async Task> UpdateAsync(IData : DataTypeOperationStatus.Success; }, userKey, - AuditType.New); + AuditType.Save); /// /// Saves a collection of @@ -705,8 +735,7 @@ public void Delete(IDataType dataType, int userId = Constants.Security.SuperUser scope.Notifications.Publish(new DataTypeDeletedNotification(dataType, eventMessages).WithStateFrom(deletingDataTypeNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(AuditType.Delete, currentUserId, dataType.Id); + await AuditAsync(AuditType.Delete, userKey, dataType.Id); scope.Complete(); @@ -902,7 +931,7 @@ private async Task> SaveAsync( scope.Notifications.Publish(new DataTypeSavedNotification(dataType, eventMessages).WithStateFrom(savingDataTypeNotification)); - Audit(auditType, currentUserId, dataType.Id); + await AuditAsync(auditType, userKey, dataType.Id); scope.Complete(); return Attempt.SucceedWithStatus(DataTypeOperationStatus.Success, dataType); @@ -915,7 +944,23 @@ private async Task> SaveAsync( { Result: var intId } => _dataTypeRepository.Get(intId), }; - private void Audit(AuditType type, int userId, int objectId) - => _auditRepository.Save(new AuditItem(objectId, type, userId, ObjectTypes.GetName(UmbracoObjectTypes.DataType))); + private void Audit(AuditType type, int userId, int objectId) => + AuditAsync(type, userId, objectId).GetAwaiter().GetResult(); + + private async Task AuditAsync(AuditType type, int userId, int objectId) + { + Guid userKey = await _userIdKeyResolver.TryGetAsync(userId) is { Success: true } userKeyAttempt + ? userKeyAttempt.Result + : Constants.Security.UnknownUserKey; + + await AuditAsync(type, userKey, objectId); + } + + private async Task AuditAsync(AuditType type, Guid userKey, int objectId) => + await _auditService.AddAsync( + type, + userKey, + objectId, + UmbracoObjectTypes.DataType.GetName()); } } diff --git a/src/Umbraco.Core/Services/DictionaryItemService.cs b/src/Umbraco.Core/Services/DictionaryItemService.cs index 467f4ea26ac6..c8cb2605b551 100644 --- a/src/Umbraco.Core/Services/DictionaryItemService.cs +++ b/src/Umbraco.Core/Services/DictionaryItemService.cs @@ -12,7 +12,7 @@ namespace Umbraco.Cms.Core.Services; internal sealed class DictionaryItemService : RepositoryService, IDictionaryItemService { private readonly IDictionaryRepository _dictionaryRepository; - private readonly IAuditRepository _auditRepository; + private readonly IAuditService _auditService; private readonly ILanguageService _languageService; private readonly IUserIdKeyResolver _userIdKeyResolver; @@ -21,13 +21,13 @@ public DictionaryItemService( ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IDictionaryRepository dictionaryRepository, - IAuditRepository auditRepository, + IAuditService auditService, ILanguageService languageService, IUserIdKeyResolver userIdKeyResolver) : base(provider, loggerFactory, eventMessagesFactory) { _dictionaryRepository = dictionaryRepository; - _auditRepository = auditRepository; + _auditService = auditService; _languageService = languageService; _userIdKeyResolver = userIdKeyResolver; } @@ -199,8 +199,7 @@ public async Task> Updat new DictionaryItemDeletedNotification(dictionaryItem, eventMessages) .WithStateFrom(deletingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(AuditType.Delete, "Delete DictionaryItem", currentUserId, dictionaryItem.Id, nameof(DictionaryItem)); + await AuditAsync(AuditType.Delete, "Delete DictionaryItem", userKey, dictionaryItem.Id, nameof(DictionaryItem)); scope.Complete(); return Attempt.SucceedWithStatus(DictionaryItemOperationStatus.Success, dictionaryItem); @@ -261,8 +260,7 @@ public async Task> MoveA scope.Notifications.Publish( new DictionaryItemMovedNotification(moveEventInfo, eventMessages).WithStateFrom(movingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(AuditType.Move, "Move DictionaryItem", currentUserId, dictionaryItem.Id, nameof(DictionaryItem)); + await AuditAsync(AuditType.Move, "Move DictionaryItem", userKey, dictionaryItem.Id, nameof(DictionaryItem)); scope.Complete(); return Attempt.SucceedWithStatus(DictionaryItemOperationStatus.Success, dictionaryItem); @@ -322,8 +320,7 @@ private async Task> Save scope.Notifications.Publish( new DictionaryItemSavedNotification(dictionaryItem, eventMessages).WithStateFrom(savingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(auditType, auditMessage, currentUserId, dictionaryItem.Id, nameof(DictionaryItem)); + await AuditAsync(auditType, auditMessage, userKey, dictionaryItem.Id, nameof(DictionaryItem)); scope.Complete(); return Attempt.SucceedWithStatus(DictionaryItemOperationStatus.Success, dictionaryItem); @@ -339,8 +336,13 @@ private Task> GetByQueryAsync(IQuery - _auditRepository.Save(new AuditItem(objectId, type, userId, entityType, message)); + private async Task AuditAsync(AuditType type, string message, Guid userKey, int objectId, string? entityType) => + await _auditService.AddAsync( + type, + userKey, + objectId, + entityType, + message); private bool HasValidParent(IDictionaryItem dictionaryItem) => dictionaryItem.ParentId.HasValue == false || _dictionaryRepository.Get(dictionaryItem.ParentId.Value) != null; diff --git a/src/Umbraco.Core/Services/EntityTypeContainerService.cs b/src/Umbraco.Core/Services/EntityTypeContainerService.cs index 4a403e0c1fa6..040c5c716eff 100644 --- a/src/Umbraco.Core/Services/EntityTypeContainerService.cs +++ b/src/Umbraco.Core/Services/EntityTypeContainerService.cs @@ -14,7 +14,7 @@ internal abstract class EntityTypeContainerService> GetAllAsync() _entityContainerRepository.Delete(container); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(AuditType.Delete, currentUserId, container.Id); + await AuditAsync(AuditType.Delete, userKey, container.Id); scope.Complete(); scope.Notifications.Publish(new EntityContainerDeletedNotification(container, eventMessages).WithStateFrom(deletingEntityContainerNotification)); @@ -218,8 +217,7 @@ public async Task> GetAllAsync() _entityContainerRepository.Save(container); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(auditType, currentUserId, container.Id); + await AuditAsync(auditType, userKey, container.Id); scope.Complete(); scope.Notifications.Publish(new EntityContainerSavedNotification(container, eventMessages).WithStateFrom(savingEntityContainerNotification)); @@ -239,8 +237,12 @@ public async Task> GetAllAsync() return _entityContainerRepository.Get(treeEntity.ParentId); } - private void Audit(AuditType type, int userId, int objectId) - => _auditRepository.Save(new AuditItem(objectId, type, userId, ContainerObjectType.GetName())); + private async Task AuditAsync(AuditType type, Guid userKey, int objectId) => + await _auditService.AddAsync( + type, + userKey, + objectId, + ContainerObjectType.GetName()); private void ReadLock(ICoreScope scope) { diff --git a/src/Umbraco.Core/Services/FileService.cs b/src/Umbraco.Core/Services/FileService.cs index eae1944b5d79..4408d1c1f180 100644 --- a/src/Umbraco.Core/Services/FileService.cs +++ b/src/Umbraco.Core/Services/FileService.cs @@ -1,7 +1,9 @@ using System.Text.RegularExpressions; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Models; @@ -22,7 +24,7 @@ namespace Umbraco.Cms.Core.Services; public class FileService : RepositoryService, IFileService { private const string PartialViewHeader = "@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage"; - private readonly IAuditRepository _auditRepository; + private readonly IAuditService _auditService; private readonly IHostingEnvironment _hostingEnvironment; private readonly IPartialViewRepository _partialViewRepository; private readonly IScriptRepository _scriptRepository; @@ -38,7 +40,7 @@ public FileService( IStylesheetRepository stylesheetRepository, IScriptRepository scriptRepository, IPartialViewRepository partialViewRepository, - IAuditRepository auditRepository, + IAuditService auditService, IHostingEnvironment hostingEnvironment, ITemplateService templateService, ITemplateRepository templateRepository, @@ -50,13 +52,78 @@ public FileService( _stylesheetRepository = stylesheetRepository; _scriptRepository = scriptRepository; _partialViewRepository = partialViewRepository; - _auditRepository = auditRepository; + _auditService = auditService; _hostingEnvironment = hostingEnvironment; _templateService = templateService; _templateRepository = templateRepository; _userIdKeyResolver = userIdKeyResolver; } + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public FileService( + ICoreScopeProvider uowProvider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IStylesheetRepository stylesheetRepository, + IScriptRepository scriptRepository, + IPartialViewRepository partialViewRepository, + IAuditRepository auditRepository, + IHostingEnvironment hostingEnvironment, + ITemplateService templateService, + ITemplateRepository templateRepository, + IUserIdKeyResolver userIdKeyResolver, + IShortStringHelper shortStringHelper, + IOptions globalSettings) + : this( + uowProvider, + loggerFactory, + eventMessagesFactory, + stylesheetRepository, + scriptRepository, + partialViewRepository, + StaticServiceProvider.Instance.GetRequiredService(), + hostingEnvironment, + templateService, + templateRepository, + userIdKeyResolver, + shortStringHelper, + globalSettings) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public FileService( + ICoreScopeProvider uowProvider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IStylesheetRepository stylesheetRepository, + IScriptRepository scriptRepository, + IPartialViewRepository partialViewRepository, + IAuditService auditService, + IAuditRepository auditRepository, + IHostingEnvironment hostingEnvironment, + ITemplateService templateService, + ITemplateRepository templateRepository, + IUserIdKeyResolver userIdKeyResolver, + IShortStringHelper shortStringHelper, + IOptions globalSettings) + : this( + uowProvider, + loggerFactory, + eventMessagesFactory, + stylesheetRepository, + scriptRepository, + partialViewRepository, + auditService, + hostingEnvironment, + templateService, + templateRepository, + userIdKeyResolver, + shortStringHelper, + globalSettings) + { + } + #region Stylesheets /// @@ -70,7 +137,20 @@ public IEnumerable GetStylesheets(params string[] paths) } private void Audit(AuditType type, int userId, int objectId, string? entityType) => - _auditRepository.Save(new AuditItem(objectId, type, userId, entityType)); + AuditAsync(type, userId, objectId, entityType).GetAwaiter().GetResult(); + + private async Task AuditAsync(AuditType type, int userId, int objectId, string? entityType) + { + Guid userKey = await _userIdKeyResolver.TryGetAsync(userId) is { Success: true } userKeyAttempt + ? userKeyAttempt.Result + : Constants.Security.UnknownUserKey; + + await _auditService.AddAsync( + type, + userKey, + objectId, + entityType); + } /// [Obsolete("Please use IStylesheetService for stylesheet operations - will be removed in Umbraco 15")] diff --git a/src/Umbraco.Core/Services/FileServiceOperationBase.cs b/src/Umbraco.Core/Services/FileServiceOperationBase.cs index e7b392d94931..88501af28a7c 100644 --- a/src/Umbraco.Core/Services/FileServiceOperationBase.cs +++ b/src/Umbraco.Core/Services/FileServiceOperationBase.cs @@ -1,4 +1,6 @@ -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; @@ -16,14 +18,41 @@ public abstract class FileServiceOperationBase _logger; private readonly IUserIdKeyResolver _userIdKeyResolver; - private readonly IAuditRepository _auditRepository; - - protected FileServiceOperationBase(ICoreScopeProvider provider, ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, TRepository repository, ILogger logger, IUserIdKeyResolver userIdKeyResolver, IAuditRepository auditRepository) + private readonly IAuditService _auditService; + + protected FileServiceOperationBase( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + TRepository repository, + ILogger logger, + IUserIdKeyResolver userIdKeyResolver, + IAuditService auditService) : base(provider, loggerFactory, eventMessagesFactory, repository) { _logger = logger; _userIdKeyResolver = userIdKeyResolver; - _auditRepository = auditRepository; + _auditService = auditService; + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + protected FileServiceOperationBase( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + TRepository repository, + ILogger logger, + IUserIdKeyResolver userIdKeyResolver, + IAuditRepository auditRepository) + : this( + provider, + loggerFactory, + eventMessagesFactory, + repository, + logger, + userIdKeyResolver, + StaticServiceProvider.Instance.GetRequiredService()) + { } protected abstract TOperationStatus Success { get; } @@ -249,10 +278,7 @@ private TOperationStatus ValidateRename(string newName, string newPath) } private async Task AuditAsync(AuditType type, Guid userKey) - { - var userId = await _userIdKeyResolver.GetAsync(userKey); - _auditRepository.Save(new AuditItem(-1, type, userId, EntityType)); - } + => await _auditService.AddAsync(type, userKey, -1, EntityType); private string GetFilePath(string? parentPath, string fileName) => Path.Join(parentPath, fileName); diff --git a/src/Umbraco.Core/Services/LanguageService.cs b/src/Umbraco.Core/Services/LanguageService.cs index 01a249dbd0ed..e32b2c988e33 100644 --- a/src/Umbraco.Core/Services/LanguageService.cs +++ b/src/Umbraco.Core/Services/LanguageService.cs @@ -13,7 +13,7 @@ namespace Umbraco.Cms.Core.Services; internal sealed class LanguageService : RepositoryService, ILanguageService { private readonly ILanguageRepository _languageRepository; - private readonly IAuditRepository _auditRepository; + private readonly IAuditService _auditService; private readonly IUserIdKeyResolver _userIdKeyResolver; private readonly IIsoCodeValidator _isoCodeValidator; @@ -22,13 +22,13 @@ public LanguageService( ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, ILanguageRepository languageRepository, - IAuditRepository auditRepository, + IAuditService auditService, IUserIdKeyResolver userIdKeyResolver, IIsoCodeValidator isoCodeValidator) : base(provider, loggerFactory, eventMessagesFactory) { _languageRepository = languageRepository; - _auditRepository = auditRepository; + _auditService = auditService; _userIdKeyResolver = userIdKeyResolver; _isoCodeValidator = isoCodeValidator; } @@ -160,8 +160,7 @@ public async Task> CreateAsync(ILang scope.Notifications.Publish( new LanguageDeletedNotification(language, eventMessages).WithStateFrom(deletingLanguageNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(AuditType.Delete, "Delete Language", currentUserId, language.Id, UmbracoObjectTypes.Language.GetName()); + await AuditAsync(AuditType.Delete, "Delete Language", userKey, language.Id, UmbracoObjectTypes.Language.GetName()); scope.Complete(); return Attempt.SucceedWithStatus(LanguageOperationStatus.Success, language); } @@ -213,16 +212,20 @@ private async Task> SaveAsync( scope.Notifications.Publish( new LanguageSavedNotification(language, eventMessages).WithStateFrom(savingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(auditType, auditMessage, currentUserId, language.Id, UmbracoObjectTypes.Language.GetName()); + await AuditAsync(auditType, auditMessage, userKey, language.Id, UmbracoObjectTypes.Language.GetName()); scope.Complete(); return Attempt.SucceedWithStatus(LanguageOperationStatus.Success, language); } } - private void Audit(AuditType type, string message, int userId, int objectId, string? entityType) => - _auditRepository.Save(new AuditItem(objectId, type, userId, entityType, message)); + private async Task AuditAsync(AuditType type, string message, Guid userKey, int objectId, string? entityType) => + await _auditService.AddAsync( + type, + userKey, + objectId, + entityType, + message); private bool HasInvalidFallbackLanguage(ILanguage language) { diff --git a/src/Umbraco.Core/Services/LocalizationService.cs b/src/Umbraco.Core/Services/LocalizationService.cs index 989b8f8206db..0e0d2ad922fd 100644 --- a/src/Umbraco.Core/Services/LocalizationService.cs +++ b/src/Umbraco.Core/Services/LocalizationService.cs @@ -17,7 +17,6 @@ namespace Umbraco.Cms.Core.Services; [Obsolete("Please use ILanguageService and IDictionaryItemService for localization. Will be removed in V15.")] internal class LocalizationService : RepositoryService, ILocalizationService { - private readonly IAuditRepository _auditRepository; private readonly IDictionaryRepository _dictionaryRepository; private readonly ILanguageRepository _languageRepository; private readonly ILanguageService _languageService; @@ -30,14 +29,12 @@ public LocalizationService( ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IDictionaryRepository dictionaryRepository, - IAuditRepository auditRepository, ILanguageRepository languageRepository) : this( provider, loggerFactory, eventMessagesFactory, dictionaryRepository, - auditRepository, languageRepository, StaticServiceProvider.Instance.GetRequiredService(), StaticServiceProvider.Instance.GetRequiredService(), @@ -51,7 +48,6 @@ public LocalizationService( ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IDictionaryRepository dictionaryRepository, - IAuditRepository auditRepository, ILanguageRepository languageRepository, ILanguageService languageService, IDictionaryItemService dictionaryItemService, @@ -59,7 +55,6 @@ public LocalizationService( : base(provider, loggerFactory, eventMessagesFactory) { _dictionaryRepository = dictionaryRepository; - _auditRepository = auditRepository; _languageRepository = languageRepository; _languageService = languageService; _dictionaryItemService = dictionaryItemService; diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index c98ba53267e7..3419065c71b5 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/src/Umbraco.Core/Services/MediaService.cs @@ -1,5 +1,7 @@ using System.Globalization; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; @@ -22,7 +24,7 @@ public class MediaService : RepositoryService, IMediaService { private readonly IMediaRepository _mediaRepository; private readonly IMediaTypeRepository _mediaTypeRepository; - private readonly IAuditRepository _auditRepository; + private readonly IAuditService _auditService; private readonly IEntityRepository _entityRepository; private readonly IShortStringHelper _shortStringHelper; private readonly IUserIdKeyResolver _userIdKeyResolver; @@ -37,7 +39,7 @@ public MediaService( ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IMediaRepository mediaRepository, - IAuditRepository auditRepository, + IAuditService auditService, IMediaTypeRepository mediaTypeRepository, IEntityRepository entityRepository, IShortStringHelper shortStringHelper, @@ -46,13 +48,66 @@ public MediaService( { _mediaFileManager = mediaFileManager; _mediaRepository = mediaRepository; - _auditRepository = auditRepository; + _auditService = auditService; _mediaTypeRepository = mediaTypeRepository; _entityRepository = entityRepository; _shortStringHelper = shortStringHelper; _userIdKeyResolver = userIdKeyResolver; } + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public MediaService( + ICoreScopeProvider provider, + MediaFileManager mediaFileManager, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IMediaRepository mediaRepository, + IAuditRepository auditRepository, + IMediaTypeRepository mediaTypeRepository, + IEntityRepository entityRepository, + IShortStringHelper shortStringHelper, + IUserIdKeyResolver userIdKeyResolver) + : this( + provider, + mediaFileManager, + loggerFactory, + eventMessagesFactory, + mediaRepository, + StaticServiceProvider.Instance.GetRequiredService(), + mediaTypeRepository, + entityRepository, + shortStringHelper, + userIdKeyResolver) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public MediaService( + ICoreScopeProvider provider, + MediaFileManager mediaFileManager, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IMediaRepository mediaRepository, + IAuditService auditService, + IAuditRepository auditRepository, + IMediaTypeRepository mediaTypeRepository, + IEntityRepository entityRepository, + IShortStringHelper shortStringHelper, + IUserIdKeyResolver userIdKeyResolver) + : this( + provider, + mediaFileManager, + loggerFactory, + eventMessagesFactory, + mediaRepository, + auditService, + mediaTypeRepository, + entityRepository, + shortStringHelper, + userIdKeyResolver) + { + } + #endregion #region Count @@ -1235,9 +1290,22 @@ public ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportO #region Private Methods - private void Audit(AuditType type, int userId, int objectId, string? message = null) + private void Audit(AuditType type, int userId, int objectId, string? message = null) => + AuditAsync(type, userId, objectId, message).GetAwaiter().GetResult(); + + private async Task AuditAsync(AuditType type, int userId, int objectId, string? message = null, string? parameters = null) { - _auditRepository.Save(new AuditItem(objectId, type, userId, ObjectTypes.GetName(UmbracoObjectTypes.Media), message)); + Guid userKey = await _userIdKeyResolver.TryGetAsync(userId) is { Success: true } userKeyAttempt + ? userKeyAttempt.Result + : Constants.Security.UnknownUserKey; + + await _auditService.AddAsync( + type, + userKey, + objectId, + UmbracoObjectTypes.Media.GetName(), + message, + parameters); } #endregion diff --git a/src/Umbraco.Core/Services/MediaTypeContainerService.cs b/src/Umbraco.Core/Services/MediaTypeContainerService.cs index 6aeb9af4842a..548b38b70db0 100644 --- a/src/Umbraco.Core/Services/MediaTypeContainerService.cs +++ b/src/Umbraco.Core/Services/MediaTypeContainerService.cs @@ -14,10 +14,10 @@ public MediaTypeContainerService( ILoggerFactory loggerFactory, IEventMessagesFactory eventMessagesFactory, IMediaTypeContainerRepository entityContainerRepository, - IAuditRepository auditRepository, + IAuditService auditService, IEntityRepository entityRepository, IUserIdKeyResolver userIdKeyResolver) - : base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditRepository, entityRepository, userIdKeyResolver) + : base(provider, loggerFactory, eventMessagesFactory, entityContainerRepository, auditService, entityRepository, userIdKeyResolver) { } diff --git a/src/Umbraco.Core/Services/MediaTypeService.cs b/src/Umbraco.Core/Services/MediaTypeService.cs index 2ab943a9c78d..22166dfdae22 100644 --- a/src/Umbraco.Core/Services/MediaTypeService.cs +++ b/src/Umbraco.Core/Services/MediaTypeService.cs @@ -1,4 +1,6 @@ +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; @@ -19,7 +21,7 @@ public MediaTypeService( IEventMessagesFactory eventMessagesFactory, IMediaService mediaService, IMediaTypeRepository mediaTypeRepository, - IAuditRepository auditRepository, + IAuditService auditService, IMediaTypeContainerRepository entityContainerRepository, IEntityRepository entityRepository, IEventAggregator eventAggregator, @@ -30,13 +32,72 @@ public MediaTypeService( loggerFactory, eventMessagesFactory, mediaTypeRepository, - auditRepository, + auditService, + entityContainerRepository, + entityRepository, + eventAggregator, + userIdKeyResolver, + contentTypeFilters) + { + MediaService = mediaService; + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public MediaTypeService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IMediaService mediaService, + IMediaTypeRepository mediaTypeRepository, + IAuditRepository auditRepository, + IMediaTypeContainerRepository entityContainerRepository, + IEntityRepository entityRepository, + IEventAggregator eventAggregator, + IUserIdKeyResolver userIdKeyResolver, + ContentTypeFilterCollection contentTypeFilters) + : this( + provider, + loggerFactory, + eventMessagesFactory, + mediaService, + mediaTypeRepository, + StaticServiceProvider.Instance.GetRequiredService(), entityContainerRepository, entityRepository, eventAggregator, userIdKeyResolver, - contentTypeFilters) => MediaService = mediaService; + contentTypeFilters) + { + } + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public MediaTypeService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IMediaService mediaService, + IMediaTypeRepository mediaTypeRepository, + IAuditService auditService, + IAuditRepository auditRepository, + IMediaTypeContainerRepository entityContainerRepository, + IEntityRepository entityRepository, + IEventAggregator eventAggregator, + IUserIdKeyResolver userIdKeyResolver, + ContentTypeFilterCollection contentTypeFilters) + : this( + provider, + loggerFactory, + eventMessagesFactory, + mediaService, + mediaTypeRepository, + auditService, + entityContainerRepository, + entityRepository, + eventAggregator, + userIdKeyResolver, + contentTypeFilters) + { + } protected override int[] ReadLockIds => MediaTypeLocks.ReadLockIds; diff --git a/src/Umbraco.Core/Services/MemberService.cs b/src/Umbraco.Core/Services/MemberService.cs index 86efb2abfddc..5f631a77fb79 100644 --- a/src/Umbraco.Core/Services/MemberService.cs +++ b/src/Umbraco.Core/Services/MemberService.cs @@ -1,4 +1,6 @@ +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; @@ -19,9 +21,10 @@ public class MemberService : RepositoryService, IMemberService private readonly IMemberRepository _memberRepository; private readonly IMemberTypeRepository _memberTypeRepository; private readonly IMemberGroupRepository _memberGroupRepository; - private readonly IAuditRepository _auditRepository; + private readonly IAuditService _auditService; private readonly IMemberGroupService _memberGroupService; private readonly Lazy _idKeyMap; + private readonly IUserIdKeyResolver _userIdKeyResolver; #region Constructor @@ -33,18 +36,72 @@ public MemberService( IMemberRepository memberRepository, IMemberTypeRepository memberTypeRepository, IMemberGroupRepository memberGroupRepository, - IAuditRepository auditRepository, - Lazy idKeyMap) + IAuditService auditService, + Lazy idKeyMap, + IUserIdKeyResolver userIdKeyResolver) : base(provider, loggerFactory, eventMessagesFactory) { _memberRepository = memberRepository; _memberTypeRepository = memberTypeRepository; _memberGroupRepository = memberGroupRepository; - _auditRepository = auditRepository; + _auditService = auditService; _idKeyMap = idKeyMap; + _userIdKeyResolver = userIdKeyResolver; _memberGroupService = memberGroupService ?? throw new ArgumentNullException(nameof(memberGroupService)); } + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public MemberService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IMemberGroupService memberGroupService, + IMemberRepository memberRepository, + IMemberTypeRepository memberTypeRepository, + IMemberGroupRepository memberGroupRepository, + IAuditRepository auditRepository, + Lazy idKeyMap) + : this( + provider, + loggerFactory, + eventMessagesFactory, + memberGroupService, + memberRepository, + memberTypeRepository, + memberGroupRepository, + StaticServiceProvider.Instance.GetRequiredService(), + idKeyMap, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public MemberService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IMemberGroupService memberGroupService, + IMemberRepository memberRepository, + IMemberTypeRepository memberTypeRepository, + IMemberGroupRepository memberGroupRepository, + IAuditService auditService, + IAuditRepository auditRepository, + Lazy idKeyMap, + IUserIdKeyResolver userIdKeyResolver) + : this( + provider, + loggerFactory, + eventMessagesFactory, + memberGroupService, + memberRepository, + memberTypeRepository, + memberGroupRepository, + auditService, + idKeyMap, + userIdKeyResolver) + { + } + #endregion #region Count @@ -1084,7 +1141,23 @@ public ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportO #region Private Methods - private void Audit(AuditType type, int userId, int objectId, string? message = null) => _auditRepository.Save(new AuditItem(objectId, type, userId, ObjectTypes.GetName(UmbracoObjectTypes.Member), message)); + private void Audit(AuditType type, int userId, int objectId, string? message = null) => + AuditAsync(type, userId, objectId, message).GetAwaiter().GetResult(); + + private async Task AuditAsync(AuditType type, int userId, int objectId, string? message = null, string? parameters = null) + { + Guid userKey = await _userIdKeyResolver.TryGetAsync(userId) is { Success: true } userKeyAttempt + ? userKeyAttempt.Result + : Constants.Security.UnknownUserKey; + + await _auditService.AddAsync( + type, + userKey, + objectId, + UmbracoObjectTypes.Member.GetName(), + message, + parameters); + } private IMember? GetMemberFromRepository(Guid id) => _idKeyMap.Value.GetIdForKey(id, UmbracoObjectTypes.Member) switch diff --git a/src/Umbraco.Core/Services/MemberTypeService.cs b/src/Umbraco.Core/Services/MemberTypeService.cs index c8313ad61814..228a10a026d3 100644 --- a/src/Umbraco.Core/Services/MemberTypeService.cs +++ b/src/Umbraco.Core/Services/MemberTypeService.cs @@ -1,4 +1,6 @@ +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; @@ -20,7 +22,7 @@ public MemberTypeService( IEventMessagesFactory eventMessagesFactory, IMemberService memberService, IMemberTypeRepository memberTypeRepository, - IAuditRepository auditRepository, + IAuditService auditService, IMemberTypeContainerRepository entityContainerRepository, IEntityRepository entityRepository, IEventAggregator eventAggregator, @@ -31,7 +33,7 @@ public MemberTypeService( loggerFactory, eventMessagesFactory, memberTypeRepository, - auditRepository, + auditService, entityContainerRepository, entityRepository, eventAggregator, @@ -42,6 +44,63 @@ public MemberTypeService( _memberTypeRepository = memberTypeRepository; } + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public MemberTypeService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IMemberService memberService, + IMemberTypeRepository memberTypeRepository, + IAuditRepository auditRepository, + IMemberTypeContainerRepository entityContainerRepository, + IEntityRepository entityRepository, + IEventAggregator eventAggregator, + IUserIdKeyResolver userIdKeyResolver, + ContentTypeFilterCollection contentTypeFilters) + : this( + provider, + loggerFactory, + eventMessagesFactory, + memberService, + memberTypeRepository, + StaticServiceProvider.Instance.GetRequiredService(), + entityContainerRepository, + entityRepository, + eventAggregator, + userIdKeyResolver, + contentTypeFilters) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public MemberTypeService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IMemberService memberService, + IMemberTypeRepository memberTypeRepository, + IAuditService auditService, + IAuditRepository auditRepository, + IMemberTypeContainerRepository entityContainerRepository, + IEntityRepository entityRepository, + IEventAggregator eventAggregator, + IUserIdKeyResolver userIdKeyResolver, + ContentTypeFilterCollection contentTypeFilters) + : this( + provider, + loggerFactory, + eventMessagesFactory, + memberService, + memberTypeRepository, + auditService, + entityContainerRepository, + entityRepository, + eventAggregator, + userIdKeyResolver, + contentTypeFilters) + { + } + // beware! order is important to avoid deadlocks protected override int[] ReadLockIds { get; } = { Constants.Locks.MemberTypes }; diff --git a/src/Umbraco.Core/Services/PartialViewService.cs b/src/Umbraco.Core/Services/PartialViewService.cs index 9543f45f2892..043eac5ac441 100644 --- a/src/Umbraco.Core/Services/PartialViewService.cs +++ b/src/Umbraco.Core/Services/PartialViewService.cs @@ -1,4 +1,6 @@ +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; @@ -21,11 +23,56 @@ public PartialViewService( IPartialViewRepository repository, ILogger logger, IUserIdKeyResolver userIdKeyResolver, - IAuditRepository auditRepository, + IAuditService auditService, PartialViewSnippetCollection snippetCollection) - : base(provider, loggerFactory, eventMessagesFactory, repository, logger, userIdKeyResolver, auditRepository) + : base(provider, loggerFactory, eventMessagesFactory, repository, logger, userIdKeyResolver, auditService) => _snippetCollection = snippetCollection; + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public PartialViewService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IPartialViewRepository repository, + ILogger logger, + IUserIdKeyResolver userIdKeyResolver, + IAuditRepository auditRepository, + PartialViewSnippetCollection snippetCollection) + : this( + provider, + loggerFactory, + eventMessagesFactory, + repository, + logger, + userIdKeyResolver, + StaticServiceProvider.Instance.GetRequiredService(), + snippetCollection) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public PartialViewService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IPartialViewRepository repository, + ILogger logger, + IUserIdKeyResolver userIdKeyResolver, + IAuditService auditService, + IAuditRepository auditRepository, + PartialViewSnippetCollection snippetCollection) + : this( + provider, + loggerFactory, + eventMessagesFactory, + repository, + logger, + userIdKeyResolver, + auditService, + snippetCollection) + { + } + protected override string[] AllowedFileExtensions { get; } = { ".cshtml" }; protected override PartialViewOperationStatus Success => PartialViewOperationStatus.Success; diff --git a/src/Umbraco.Core/Services/RelationService.cs b/src/Umbraco.Core/Services/RelationService.cs index 7dfcb4ae2f18..d15d773a01ce 100644 --- a/src/Umbraco.Core/Services/RelationService.cs +++ b/src/Umbraco.Core/Services/RelationService.cs @@ -1,4 +1,6 @@ +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Entities; @@ -13,7 +15,7 @@ namespace Umbraco.Cms.Core.Services; public class RelationService : RepositoryService, IRelationService { - private readonly IAuditRepository _auditRepository; + private readonly IAuditService _auditService; private readonly IUserIdKeyResolver _userIdKeyResolver; private readonly IEntityService _entityService; private readonly IRelationRepository _relationRepository; @@ -26,17 +28,62 @@ public RelationService( IEntityService entityService, IRelationRepository relationRepository, IRelationTypeRepository relationTypeRepository, - IAuditRepository auditRepository, + IAuditService auditService, IUserIdKeyResolver userIdKeyResolver) : base(uowProvider, loggerFactory, eventMessagesFactory) { _relationRepository = relationRepository; _relationTypeRepository = relationTypeRepository; - _auditRepository = auditRepository; + _auditService = auditService; _userIdKeyResolver = userIdKeyResolver; _entityService = entityService ?? throw new ArgumentNullException(nameof(entityService)); } + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public RelationService( + ICoreScopeProvider uowProvider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IEntityService entityService, + IRelationRepository relationRepository, + IRelationTypeRepository relationTypeRepository, + IAuditRepository auditRepository, + IUserIdKeyResolver userIdKeyResolver) + : this( + uowProvider, + loggerFactory, + eventMessagesFactory, + entityService, + relationRepository, + relationTypeRepository, + StaticServiceProvider.Instance.GetRequiredService(), + userIdKeyResolver) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public RelationService( + ICoreScopeProvider uowProvider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IEntityService entityService, + IRelationRepository relationRepository, + IRelationTypeRepository relationTypeRepository, + IAuditService auditService, + IAuditRepository auditRepository, + IUserIdKeyResolver userIdKeyResolver) + : this( + uowProvider, + loggerFactory, + eventMessagesFactory, + entityService, + relationRepository, + relationTypeRepository, + auditService, + userIdKeyResolver) + { + } + /// public IRelation? GetById(int id) { @@ -601,8 +648,7 @@ private async Task> SaveAsyn } _relationTypeRepository.Save(relationType); - var currentUser = await _userIdKeyResolver.GetAsync(userKey); - Audit(auditType, currentUser, relationType.Id, auditMessage); + await AuditAsync(auditType, userKey, relationType.Id, auditMessage); scope.Complete(); scope.Notifications.Publish( new RelationTypeSavedNotification(relationType, eventMessages).WithStateFrom(savingNotification)); @@ -666,8 +712,7 @@ public void Delete(IRelationType relationType) } _relationTypeRepository.Delete(relationType); - var currentUser = await _userIdKeyResolver.GetAsync(userKey); - Audit(AuditType.Delete, currentUser, relationType.Id, "Deleted relation type"); + await AuditAsync(AuditType.Delete, userKey, relationType.Id, "Deleted relation type"); scope.Notifications.Publish(new RelationTypeDeletedNotification(relationType, eventMessages).WithStateFrom(deletingNotification)); scope.Complete(); return await Task.FromResult(Attempt.SucceedWithStatus(RelationTypeOperationStatus.Success, relationType)); @@ -744,7 +789,25 @@ private IEnumerable GetRelationsByListOfTypeIds(IEnumerable rela } private void Audit(AuditType type, int userId, int objectId, string? message = null) => - _auditRepository.Save(new AuditItem(objectId, type, userId, UmbracoObjectTypes.RelationType.GetName(), message)); + AuditAsync(type, userId, objectId, message).GetAwaiter().GetResult(); + + private async Task AuditAsync(AuditType type, int userId, int objectId, string? message = null, string? parameters = null) + { + Guid userKey = await _userIdKeyResolver.TryGetAsync(userId) is { Success: true } userKeyAttempt + ? userKeyAttempt.Result + : Constants.Security.UnknownUserKey; + + await AuditAsync(type, userKey, objectId, message, parameters); + } + + private async Task AuditAsync(AuditType type, Guid userKey, int objectId, string? message = null, string? parameters = null) => + await _auditService.AddAsync( + type, + userKey, + objectId, + UmbracoObjectTypes.RelationType.GetName(), + message, + parameters); #endregion } diff --git a/src/Umbraco.Core/Services/ScriptService.cs b/src/Umbraco.Core/Services/ScriptService.cs index e64822c1e166..91a89298b242 100644 --- a/src/Umbraco.Core/Services/ScriptService.cs +++ b/src/Umbraco.Core/Services/ScriptService.cs @@ -1,4 +1,6 @@ -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; @@ -17,8 +19,49 @@ public ScriptService( IScriptRepository repository, ILogger logger, IUserIdKeyResolver userIdKeyResolver, + IAuditService auditService) + : base(provider, loggerFactory, eventMessagesFactory, repository, logger, userIdKeyResolver, auditService) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public ScriptService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IScriptRepository repository, + ILogger logger, + IUserIdKeyResolver userIdKeyResolver, + IAuditRepository auditRepository) + : this( + provider, + loggerFactory, + eventMessagesFactory, + repository, + logger, + userIdKeyResolver, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public ScriptService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IScriptRepository repository, + ILogger logger, + IUserIdKeyResolver userIdKeyResolver, + IAuditService auditService, IAuditRepository auditRepository) - : base(provider, loggerFactory, eventMessagesFactory, repository, logger, userIdKeyResolver, auditRepository) + : this( + provider, + loggerFactory, + eventMessagesFactory, + repository, + logger, + userIdKeyResolver, + auditService) { } diff --git a/src/Umbraco.Core/Services/StylesheetService.cs b/src/Umbraco.Core/Services/StylesheetService.cs index 08bab2a84a02..2137de92b8d3 100644 --- a/src/Umbraco.Core/Services/StylesheetService.cs +++ b/src/Umbraco.Core/Services/StylesheetService.cs @@ -1,4 +1,6 @@ -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Notifications; @@ -17,8 +19,49 @@ public StylesheetService( IStylesheetRepository repository, ILogger logger, IUserIdKeyResolver userIdKeyResolver, + IAuditService auditService) + : base(provider, loggerFactory, eventMessagesFactory, repository, logger, userIdKeyResolver, auditService) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public StylesheetService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IStylesheetRepository repository, + ILogger logger, + IUserIdKeyResolver userIdKeyResolver, + IAuditRepository auditRepository) + : this( + provider, + loggerFactory, + eventMessagesFactory, + repository, + logger, + userIdKeyResolver, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public StylesheetService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IStylesheetRepository repository, + ILogger logger, + IUserIdKeyResolver userIdKeyResolver, + IAuditService auditService, IAuditRepository auditRepository) - : base(provider, loggerFactory, eventMessagesFactory, repository, logger, userIdKeyResolver, auditRepository) + : this( + provider, + loggerFactory, + eventMessagesFactory, + repository, + logger, + userIdKeyResolver, + auditService) { } diff --git a/src/Umbraco.Core/Services/TemplateService.cs b/src/Umbraco.Core/Services/TemplateService.cs index 80391c28b5fe..e568e52ed67d 100644 --- a/src/Umbraco.Core/Services/TemplateService.cs +++ b/src/Umbraco.Core/Services/TemplateService.cs @@ -1,4 +1,6 @@ +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; @@ -16,10 +18,9 @@ public class TemplateService : RepositoryService, ITemplateService { private readonly IShortStringHelper _shortStringHelper; private readonly ITemplateRepository _templateRepository; - private readonly IAuditRepository _auditRepository; + private readonly IAuditService _auditService; private readonly ITemplateContentParserService _templateContentParserService; private readonly IUserIdKeyResolver _userIdKeyResolver; - private readonly IDefaultViewContentProvider _defaultViewContentProvider; public TemplateService( ICoreScopeProvider provider, @@ -27,18 +28,63 @@ public TemplateService( IEventMessagesFactory eventMessagesFactory, IShortStringHelper shortStringHelper, ITemplateRepository templateRepository, - IAuditRepository auditRepository, + IAuditService auditService, ITemplateContentParserService templateContentParserService, - IUserIdKeyResolver userIdKeyResolver, - IDefaultViewContentProvider defaultViewContentProvider) + IUserIdKeyResolver userIdKeyResolver) : base(provider, loggerFactory, eventMessagesFactory) { _shortStringHelper = shortStringHelper; _templateRepository = templateRepository; - _auditRepository = auditRepository; + _auditService = auditService; _templateContentParserService = templateContentParserService; _userIdKeyResolver = userIdKeyResolver; - _defaultViewContentProvider = defaultViewContentProvider; + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public TemplateService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IShortStringHelper shortStringHelper, + ITemplateRepository templateRepository, + IAuditRepository auditRepository, + ITemplateContentParserService templateContentParserService, + IUserIdKeyResolver userIdKeyResolver, + IDefaultViewContentProvider defaultViewContentProvider) + : this( + provider, + loggerFactory, + eventMessagesFactory, + shortStringHelper, + templateRepository, + StaticServiceProvider.Instance.GetRequiredService(), + templateContentParserService, + userIdKeyResolver) + { + } + + [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] + public TemplateService( + ICoreScopeProvider provider, + ILoggerFactory loggerFactory, + IEventMessagesFactory eventMessagesFactory, + IShortStringHelper shortStringHelper, + ITemplateRepository templateRepository, + IAuditService auditService, + IAuditRepository auditRepository, + ITemplateContentParserService templateContentParserService, + IUserIdKeyResolver userIdKeyResolver, + IDefaultViewContentProvider defaultViewContentProvider) + : this( + provider, + loggerFactory, + eventMessagesFactory, + shortStringHelper, + templateRepository, + auditService, + templateContentParserService, + userIdKeyResolver) + { } /// @@ -81,8 +127,7 @@ public async Task> CreateForContentT scope.Notifications.Publish( new TemplateSavedNotification(template, eventMessages).WithStateFrom(savingEvent)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(AuditType.New, currentUserId, template.Id, UmbracoObjectTypes.Template.GetName()); + await Audit(AuditType.New, userKey, template.Id, UmbracoObjectTypes.Template.GetName()); scope.Complete(); } @@ -270,8 +315,7 @@ private async Task> SaveAsync(ITempl scope.Notifications.Publish( new TemplateSavedNotification(template, eventMessages).WithStateFrom(savingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(auditType, currentUserId, template.Id, UmbracoObjectTypes.Template.GetName()); + await Audit(auditType, userKey, template.Id, UmbracoObjectTypes.Template.GetName()); scope.Complete(); return Attempt.SucceedWithStatus(TemplateOperationStatus.Success, template); } @@ -391,8 +435,8 @@ private async Task SetMasterTemplateAsync(ITemplate template, ITemplate? masterT } } - private void Audit(AuditType type, int userId, int objectId, string? entityType) => - _auditRepository.Save(new AuditItem(objectId, type, userId, entityType)); + private Task Audit(AuditType type, Guid userKey, int objectId, string? entityType) => + _auditService.AddAsync(type, userKey, objectId, entityType); private async Task> DeleteAsync(Func> getTemplate, Guid userKey) { @@ -424,8 +468,7 @@ private void Audit(AuditType type, int userId, int objectId, string? entityType) scope.Notifications.Publish( new TemplateDeletedNotification(template, eventMessages).WithStateFrom(deletingNotification)); - var currentUserId = await _userIdKeyResolver.GetAsync(userKey); - Audit(AuditType.Delete, currentUserId, template.Id, UmbracoObjectTypes.Template.GetName()); + await Audit(AuditType.Delete, userKey, template.Id, UmbracoObjectTypes.Template.GetName()); scope.Complete(); return Attempt.SucceedWithStatus(TemplateOperationStatus.Success, template); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs index 65ddacd2397e..21d74e3d0312 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs @@ -23,7 +23,7 @@ public class AuditServiceTests private Mock _scopeProviderMock; private Mock _auditRepositoryMock; private Mock _entityServiceMock; - private Mock _userServiceMock; + private Mock _userIdKeyResolverMock; [SetUp] public void Setup() @@ -31,14 +31,14 @@ public void Setup() _scopeProviderMock = new Mock(MockBehavior.Strict); _auditRepositoryMock = new Mock(MockBehavior.Strict); _entityServiceMock = new Mock(MockBehavior.Strict); - _userServiceMock = new Mock(MockBehavior.Strict); + _userIdKeyResolverMock = new Mock(MockBehavior.Strict); _auditService = new AuditService( _scopeProviderMock.Object, Mock.Of(MockBehavior.Strict), Mock.Of(MockBehavior.Strict), _auditRepositoryMock.Object, - _userServiceMock.Object, + _userIdKeyResolverMock.Object, _entityServiceMock.Object); } @@ -59,10 +59,8 @@ public async Task AddAsync_Calls_Repository_With_Correct_Values(AuditType type, Assert.AreEqual(parameters, item.Parameters); }); - Mock mockUser = new Mock(); - mockUser.Setup(x => x.Id).Returns(Constants.Security.SuperUserId); - - _userServiceMock.Setup(x => x.GetAsync(Constants.Security.SuperUserKey)).ReturnsAsync(mockUser.Object); + _userIdKeyResolverMock.Setup(x => x.TryGetAsync(Constants.Security.SuperUserKey)) + .ReturnsAsync(Attempt.Succeed(Constants.Security.SuperUserId)); var result = await _auditService.AddAsync( type, @@ -79,7 +77,7 @@ public async Task AddAsync_Calls_Repository_With_Correct_Values(AuditType type, [Test] public async Task AddAsync_Does_Not_Succeed_When_Non_Existing_User_Is_Provided() { - _userServiceMock.Setup(x => x.GetAsync(It.IsAny())).ReturnsAsync((IUser?)null); + _userIdKeyResolverMock.Setup(x => x.TryGetAsync(It.IsAny())).ReturnsAsync(Attempt.Fail()); var result = await _auditService.AddAsync( AuditType.Publish, From 85cd4ce5be7024d6bbe0fe8df21fb52099ea2dc5 Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Mon, 19 May 2025 13:57:31 +0200 Subject: [PATCH 04/12] Apply suggestions from code review --- src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs | 1 + src/Umbraco.Core/Services/AuditEntryService.cs | 4 ---- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs b/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs index 97e60fef655a..630eaf49c53e 100644 --- a/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs +++ b/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs @@ -356,6 +356,7 @@ await Audit( } } + [Obsolete("Use HandleAsync() instead. Scheduled for removal in V19.")] public void Handle(UserSavedNotification notification) => HandleAsync(notification, CancellationToken.None).GetAwaiter().GetResult(); diff --git a/src/Umbraco.Core/Services/AuditEntryService.cs b/src/Umbraco.Core/Services/AuditEntryService.cs index f3fc2f8b8065..eb571418ade9 100644 --- a/src/Umbraco.Core/Services/AuditEntryService.cs +++ b/src/Umbraco.Core/Services/AuditEntryService.cs @@ -1,7 +1,3 @@ -// -// Copyright (c) PlaceholderCompany. All rights reserved. -// - using Microsoft.Extensions.Logging; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; From 956f4c97e2ff88010008c18ec432649311f17d32 Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Mon, 19 May 2025 14:05:41 +0200 Subject: [PATCH 05/12] Small improvement --- .../Handlers/AuditNotificationsHandler.cs | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs b/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs index 630eaf49c53e..7041710b4881 100644 --- a/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs +++ b/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs @@ -105,22 +105,17 @@ public AuditNotificationsHandler( { } - private IUser? CurrentPerformingUser - { - get - { - IUser? identity = _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser; - IUser? user = identity == null ? null : _userService.GetAsync(identity.Key).GetAwaiter().GetResult(); - return user; - } - } + private async Task GetCurrentPerformingUser() => + _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser is { } identity + ? await _userService.GetAsync(identity.Key) + : null; private string PerformingIp => _ipResolver.GetCurrentRequestIpAddress(); /// public async Task HandleAsync(AssignedMemberRolesNotification notification, CancellationToken cancellationToken) { - IUser? performingUser = CurrentPerformingUser; + IUser? performingUser = await GetCurrentPerformingUser(); var roles = string.Join(", ", notification.Roles); var members = _memberService.GetAllMembers(notification.MemberIds).ToDictionary(x => x.Id, x => x); foreach (var id in notification.MemberIds) @@ -145,7 +140,7 @@ public async Task HandleAsync( AssignedUserGroupPermissionsNotification notification, CancellationToken cancellationToken) { - IUser? performingUser = CurrentPerformingUser; + IUser? performingUser = await GetCurrentPerformingUser(); IEnumerable perms = notification.EntityPermissions; foreach (EntityPermission perm in perms) { @@ -169,7 +164,7 @@ public void Handle(AssignedUserGroupPermissionsNotification notification) /// public async Task HandleAsync(ExportedMemberNotification notification, CancellationToken cancellationToken) { - IUser? performingUser = CurrentPerformingUser; + IUser? performingUser = await GetCurrentPerformingUser(); IMember member = notification.Member; await Audit( @@ -186,7 +181,7 @@ public void Handle(ExportedMemberNotification notification) public async Task HandleAsync(MemberDeletedNotification notification, CancellationToken cancellationToken) { - IUser? performingUser = CurrentPerformingUser; + IUser? performingUser = await GetCurrentPerformingUser(); IEnumerable members = notification.DeletedEntities; foreach (IMember member in members) { @@ -206,7 +201,7 @@ public void Handle(MemberDeletedNotification notification) /// public async Task HandleAsync(MemberSavedNotification notification, CancellationToken cancellationToken) { - IUser? performingUser = CurrentPerformingUser; + IUser? performingUser = await GetCurrentPerformingUser(); IEnumerable members = notification.SavedEntities; foreach (IMember member in members) { @@ -228,7 +223,7 @@ public void Handle(MemberSavedNotification notification) /// public async Task HandleAsync(RemovedMemberRolesNotification notification, CancellationToken cancellationToken) { - IUser? performingUser = CurrentPerformingUser; + IUser? performingUser = await GetCurrentPerformingUser(); var roles = string.Join(", ", notification.Roles); var members = _memberService.GetAllMembers(notification.MemberIds).ToDictionary(x => x.Id, x => x); foreach (var id in notification.MemberIds) @@ -251,7 +246,7 @@ public void Handle(RemovedMemberRolesNotification notification) /// public async Task HandleAsync(UserDeletedNotification notification, CancellationToken cancellationToken) { - IUser? performingUser = CurrentPerformingUser; + IUser? performingUser = await GetCurrentPerformingUser(); IEnumerable affectedUsers = notification.DeletedEntities; foreach (IUser affectedUser in affectedUsers) { @@ -270,7 +265,7 @@ public void Handle(UserDeletedNotification notification) public async Task HandleAsync(UserGroupWithUsersSavedNotification notification, CancellationToken cancellationToken) { - IUser? performingUser = CurrentPerformingUser; + IUser? performingUser = await GetCurrentPerformingUser(); foreach (UserGroupWithUsers groupWithUser in notification.SavedEntities) { IUserGroup group = groupWithUser.UserGroup; @@ -337,7 +332,7 @@ public void Handle(UserGroupWithUsersSavedNotification notification) /// public async Task HandleAsync(UserSavedNotification notification, CancellationToken cancellationToken) { - IUser? performingUser = CurrentPerformingUser; + IUser? performingUser = await GetCurrentPerformingUser(); IEnumerable affectedUsers = notification.SavedEntities; foreach (IUser affectedUser in affectedUsers) { From 5776d770d2b5157d3f3fdb46c02c9439a529ad5e Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Mon, 19 May 2025 14:29:32 +0200 Subject: [PATCH 06/12] Update src/Umbraco.Core/Services/AuditService.cs --- src/Umbraco.Core/Services/AuditService.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Umbraco.Core/Services/AuditService.cs b/src/Umbraco.Core/Services/AuditService.cs index 1b5e2c4b2152..59e86477c197 100644 --- a/src/Umbraco.Core/Services/AuditService.cs +++ b/src/Umbraco.Core/Services/AuditService.cs @@ -23,7 +23,6 @@ public sealed class AuditService : RepositoryService, IAuditService /// /// Initializes a new instance of the class. /// - [ActivatorUtilitiesConstructor] public AuditService( ICoreScopeProvider provider, ILoggerFactory loggerFactory, From 502b4a90a5e3774ffcf508f4a1b4fda456b2b463 Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Thu, 12 Jun 2025 11:16:19 +0200 Subject: [PATCH 07/12] Some minor adjustments following the merge --- .../Security/BackOfficeUserManagerAuditer.cs | 6 ------ src/Umbraco.Core/Constants-Security.cs | 5 ----- src/Umbraco.Core/Services/AuditService.cs | 9 ++------- .../Repositories/Implement/AuditEntryRepository.cs | 1 - 4 files changed, 2 insertions(+), 19 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs b/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs index f112006fee40..d844274afe93 100644 --- a/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs +++ b/src/Umbraco.Cms.Api.Management/Security/BackOfficeUserManagerAuditer.cs @@ -1,8 +1,4 @@ using System.Globalization; -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; @@ -129,8 +125,6 @@ private async Task WriteAudit( IUser? performingUser = performingId is not null ? _userService.GetUserById(performingId.Value) : null; IUser? affectedUser = affectedId is not null ? _userService.GetUserById(affectedId.Value) : null; - Guid? affectedKey = null; - affectedKey = affectedUser?.Key; await _auditEntryService.WriteAsync( performingUser?.Key, FormatDetails(performingId, performingUser), diff --git a/src/Umbraco.Core/Constants-Security.cs b/src/Umbraco.Core/Constants-Security.cs index dba58fcb5704..eb77642f1bb9 100644 --- a/src/Umbraco.Core/Constants-Security.cs +++ b/src/Umbraco.Core/Constants-Security.cs @@ -27,11 +27,6 @@ public static class Security /// public const int UnknownUserId = 0; - /// - /// The key for the 'unknown' user. - /// - public static readonly Guid UnknownUserKey = Guid.Empty; - /// /// The name of the 'unknown' user. /// diff --git a/src/Umbraco.Core/Services/AuditService.cs b/src/Umbraco.Core/Services/AuditService.cs index 70389cdf4052..342e62a00430 100644 --- a/src/Umbraco.Core/Services/AuditService.cs +++ b/src/Umbraco.Core/Services/AuditService.cs @@ -68,12 +68,7 @@ public async Task> AddAsync( string? comment = null, string? parameters = null) { - var userId = userKey switch - { - { } when userKey == Constants.Security.UnknownUserKey => Constants.Security.UnknownUserId, - _ => (await _userService.GetAsync(userKey))?.Id, - }; - + var userId = (await _userService.GetAsync(userKey))?.Id; if (userId is null) { return Attempt.Fail(AuditLogOperationStatus.UserNotFound); @@ -320,7 +315,7 @@ public IEnumerable GetUserLogs(int userId, AuditType type, DateTime? } /// - [Obsolete("Use AuditEntryService.WriteAsync() instead. Scheduled for removal in Umbraco 18.")] + [Obsolete("Use AuditEntryService.WriteAsync() instead. Scheduled for removal in Umbraco 19.")] public IAuditEntry Write( int performingUserId, string perfomingDetails, diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs index 3ec9ea8d7b81..2843432eb25e 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/AuditEntryRepository.cs @@ -13,7 +13,6 @@ using Umbraco.Cms.Infrastructure.Persistence.Querying; using Umbraco.Cms.Infrastructure.Scoping; using Umbraco.Extensions; -using ColumnInfo = Umbraco.Cms.Infrastructure.Persistence.SqlSyntax.ColumnInfo; namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement; From 81dc3fd92a32130fb34758c0857c676159aeeb2f Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Thu, 12 Jun 2025 11:45:08 +0200 Subject: [PATCH 08/12] Delete unnecessary file --- .../OperationStatus/AuditEntryOperationStatus.cs | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 src/Umbraco.Core/Services/OperationStatus/AuditEntryOperationStatus.cs diff --git a/src/Umbraco.Core/Services/OperationStatus/AuditEntryOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/AuditEntryOperationStatus.cs deleted file mode 100644 index 795c14b473c6..000000000000 --- a/src/Umbraco.Core/Services/OperationStatus/AuditEntryOperationStatus.cs +++ /dev/null @@ -1,10 +0,0 @@ -namespace Umbraco.Cms.Core.Services.OperationStatus; - -/// -/// Represents the status of an audit entry operation. -/// -public enum AuditEntryOperationStatus -{ - Success, - RepositoryNotAvailable, -} From 9ea925f3b1dc72671fa78130a637ac7e8d96ebc3 Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Thu, 12 Jun 2025 12:06:56 +0200 Subject: [PATCH 09/12] Small cleanup on the tests --- .../Umbraco.Core/Services/AuditServiceTests.cs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs index 65ddacd2397e..d5fdaa753716 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/AuditServiceTests.cs @@ -154,7 +154,6 @@ public async Task GetItemsByKeyAsync_Returns_Correct_Total_And_Item_Count() Fixture fixture = new Fixture(); long totalRecords = 12; - // TODO: Test whether the provided query has a filter on id _auditRepositoryMock.Setup(x => x.GetPagedResultsByQuery( It.IsAny>(), 2, @@ -173,26 +172,10 @@ public async Task GetItemsByKeyAsync_Returns_Correct_Total_And_Item_Count() Assert.AreEqual(2, result.Items.Count()); } - [Test] - public async Task GetItemsByEntityAsync_Returns_No_Results_When_Key_Is_Not_Found() - { - SetupScopeProviderMock(); - - _entityServiceMock.Setup(x => x.GetId(Guid.Empty, UmbracoObjectTypes.Document)).Returns(Attempt.Fail()); - - var result = await _auditService.GetItemsByEntityAsync(-1, 10, 10); - Assert.AreEqual(0, result.Total); - Assert.AreEqual(0, result.Items.Count()); - } - [TestCase(Constants.System.Root)] [TestCase(-100)] public async Task GetItemsByEntityAsync_Returns_No_Results_When_Id_Is_Root_Or_Lower(int userId) { - SetupScopeProviderMock(); - - _entityServiceMock.Setup(x => x.GetId(Guid.Empty, UmbracoObjectTypes.Document)).Returns(Attempt.Fail()); - var result = await _auditService.GetItemsByEntityAsync(userId, 10, 10); Assert.AreEqual(0, result.Total); Assert.AreEqual(0, result.Items.Count()); @@ -208,7 +191,6 @@ public async Task GetItemsByEntityAsync_Returns_Correct_Total_And_Item_Count() Fixture fixture = new Fixture(); long totalRecords = 12; - // TODO: Test whether the provided query has a filter on id _auditRepositoryMock.Setup(x => x.GetPagedResultsByQuery( It.IsAny>(), 2, From 81d3c1a4ec6393e18836f94f5bf46fec711e0d03 Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Fri, 18 Jul 2025 11:26:04 +0200 Subject: [PATCH 10/12] Remove changing user id to 0 (on audit) if user id is admin in media bulk save --- src/Umbraco.Core/Services/MediaService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index 6f2c92df09f9..2ff78811f348 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/src/Umbraco.Core/Services/MediaService.cs @@ -843,7 +843,7 @@ public bool HasChildren(int id) scope.Notifications.Publish(new MediaSavedNotification(mediasA, messages).WithStateFrom(savingNotification)); // TODO: See note about suppressing events in content service scope.Notifications.Publish(new MediaTreeChangeNotification(treeChanges, messages)); - Audit(AuditType.Save, userId == -1 ? 0 : userId, Constants.System.Root, "Bulk save media"); + Audit(AuditType.Save, userId, Constants.System.Root, "Bulk save media"); scope.Complete(); } From 6ed9841dce2a2d95eb14ff604d440230d851a533 Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Thu, 24 Jul 2025 10:48:12 +0200 Subject: [PATCH 11/12] Remove reference to unused IUserIdKeyResolver in TemplateService --- src/Umbraco.Core/Services/TemplateService.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Core/Services/TemplateService.cs b/src/Umbraco.Core/Services/TemplateService.cs index e568e52ed67d..cfb70aefe565 100644 --- a/src/Umbraco.Core/Services/TemplateService.cs +++ b/src/Umbraco.Core/Services/TemplateService.cs @@ -20,7 +20,6 @@ public class TemplateService : RepositoryService, ITemplateService private readonly ITemplateRepository _templateRepository; private readonly IAuditService _auditService; private readonly ITemplateContentParserService _templateContentParserService; - private readonly IUserIdKeyResolver _userIdKeyResolver; public TemplateService( ICoreScopeProvider provider, @@ -29,15 +28,13 @@ public TemplateService( IShortStringHelper shortStringHelper, ITemplateRepository templateRepository, IAuditService auditService, - ITemplateContentParserService templateContentParserService, - IUserIdKeyResolver userIdKeyResolver) + ITemplateContentParserService templateContentParserService) : base(provider, loggerFactory, eventMessagesFactory) { _shortStringHelper = shortStringHelper; _templateRepository = templateRepository; _auditService = auditService; _templateContentParserService = templateContentParserService; - _userIdKeyResolver = userIdKeyResolver; } [Obsolete("Use the non-obsolete constructor instead. Scheduled removal in v19.")] @@ -58,8 +55,7 @@ public TemplateService( shortStringHelper, templateRepository, StaticServiceProvider.Instance.GetRequiredService(), - templateContentParserService, - userIdKeyResolver) + templateContentParserService) { } @@ -82,8 +78,7 @@ public TemplateService( shortStringHelper, templateRepository, auditService, - templateContentParserService, - userIdKeyResolver) + templateContentParserService) { } From c73287ee898dd77702e5b9fc672710d0382ddefb Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Thu, 24 Jul 2025 10:50:31 +0200 Subject: [PATCH 12/12] Remove references to unused IShortStringHelper and GlobalSettings in FileService --- src/Umbraco.Core/Services/FileService.cs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/Umbraco.Core/Services/FileService.cs b/src/Umbraco.Core/Services/FileService.cs index 459c939cd534..cf0873c2e8bc 100644 --- a/src/Umbraco.Core/Services/FileService.cs +++ b/src/Umbraco.Core/Services/FileService.cs @@ -44,9 +44,7 @@ public FileService( IHostingEnvironment hostingEnvironment, ITemplateService templateService, ITemplateRepository templateRepository, - IUserIdKeyResolver userIdKeyResolver, - IShortStringHelper shortStringHelper, - IOptions globalSettings) + IUserIdKeyResolver userIdKeyResolver) : base(uowProvider, loggerFactory, eventMessagesFactory) { _stylesheetRepository = stylesheetRepository; @@ -85,9 +83,7 @@ public FileService( hostingEnvironment, templateService, templateRepository, - userIdKeyResolver, - shortStringHelper, - globalSettings) + userIdKeyResolver) { } @@ -118,9 +114,7 @@ public FileService( hostingEnvironment, templateService, templateRepository, - userIdKeyResolver, - shortStringHelper, - globalSettings) + userIdKeyResolver) { }