From c4fa3aa6499db7a832d7965287c703b1503853d1 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 13 Feb 2018 00:15:04 +1100 Subject: [PATCH] Fixes issue with AffectedUser vs PerformingUser, updates the log formatting and uses the affectedDetails better, fixes the double event calling for auth stuff, fixes the change password event raising, fixes reporting the changed property values for members --- .../Auditing/AuditEventHandler.cs | 99 ++++++++++--------- .../Auditing/IdentityAuditEventArgs.cs | 32 ++++-- src/Umbraco.Core/Models/ContentBase.cs | 24 ++++- .../EntityBase/TracksChangesEntityBase.cs | 2 +- src/Umbraco.Core/Models/Member.cs | 16 ++- src/Umbraco.Core/Models/Membership/User.cs | 3 +- .../Models/Membership/UserGroup.cs | 2 +- .../Security/BackOfficeUserManager.cs | 51 +++++++--- src/Umbraco.Core/Services/AuditService.cs | 4 + .../Editors/CurrentUserController.cs | 7 +- src/Umbraco.Web/Editors/PasswordChanger.cs | 5 +- src/Umbraco.Web/Editors/UsersController.cs | 12 +-- .../Mapping/EntityModelMapperExtensions.cs | 2 +- .../Models/Mapping/UserModelMapper.cs | 9 +- 14 files changed, 179 insertions(+), 89 deletions(-) diff --git a/src/Umbraco.Core/Auditing/AuditEventHandler.cs b/src/Umbraco.Core/Auditing/AuditEventHandler.cs index 07e1ccafc2..44be3cd7ca 100644 --- a/src/Umbraco.Core/Auditing/AuditEventHandler.cs +++ b/src/Umbraco.Core/Auditing/AuditEventHandler.cs @@ -22,7 +22,7 @@ namespace Umbraco.Core.Auditing { var identity = Thread.CurrentPrincipal?.GetUmbracoIdentity(); return identity == null - ? new User { Id = 0, Name = "(no user)", Email = "" } + ? new User { Id = 0, Name = "SYSTEM", Email = "" } : _userServiceInstance.GetUserById(Convert.ToInt32(identity.Id)); } } @@ -68,6 +68,16 @@ namespace Umbraco.Core.Auditing MemberService.RemovedRoles += OnRemovedRoles; } + private string FormatEmail(IMember member) + { + return member == null ? string.Empty : member.Email.IsNullOrWhiteSpace() ? "" : $"<{member.Email}>"; + } + + private string FormatEmail(IUser user) + { + return user == null ? string.Empty : user.Email.IsNullOrWhiteSpace() ? "" : $"<{user.Email}>"; + } + private void OnRemovedRoles(IMemberService sender, RolesEventArgs args) { var performingUser = PerformingUser; @@ -76,10 +86,10 @@ namespace Umbraco.Core.Auditing foreach (var id in args.MemberIds) { members.TryGetValue(id, out var member); - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", PerformingIp, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", PerformingIp, DateTime.Now, - 0, null, - "umbraco/member/roles/removed", $"modified roles for member id:{id} \"{member?.Name ?? "(unknown)"}\" <{member?.Email ?? ""}>, removed {roles}"); + -1, $"Member {id} \"{member?.Name ?? "(unknown)"}\" {FormatEmail(member)}", + "umbraco/member/roles/removed", $"roles modified, removed {roles}"); } } @@ -91,10 +101,10 @@ namespace Umbraco.Core.Auditing foreach (var id in args.MemberIds) { members.TryGetValue(id, out var member); - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", PerformingIp, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", PerformingIp, DateTime.Now, - 0, null, - "umbraco/member/roles/assigned", $"modified roles for member id:{id} \"{member?.Name ?? "(unknown)"}\" <{member?.Email ?? ""}>, assigned {roles}"); + -1, $"Member {id} \"{member?.Name ?? "(unknown)"}\" {FormatEmail(member)}", + "umbraco/member/roles/assigned", $"roles modified, assigned {roles}"); } } @@ -104,15 +114,18 @@ namespace Umbraco.Core.Auditing var groups = saveEventArgs.SavedEntities; foreach (var group in groups) { - //var dp = string.Join(", ", member.Properties.Where(x => x.WasDirty()).Select(x => x.Alias)); - var dp = string.Join(", ", ((UserGroup) group).GetWereDirtyProperties()); - var sections = string.Join(", ", group.AllowedSections); - var perms = string.Join(", ", group.Permissions); + var dp = string.Join(", ", ((UserGroup) group).GetPreviouslyDirtyProperties()); + var sections = ((UserGroup)group).WasPropertyDirty("AllowedSections") + ? string.Join(", ", group.AllowedSections) + : null; + var perms = ((UserGroup)group).WasPropertyDirty("Permissions") + ? string.Join(", ", group.Permissions) + : null; - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", PerformingIp, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", PerformingIp, DateTime.Now, - 0, null, - "umbraco/user-group/save", $"save group id:{group.Id}:{group.Alias} \"{group.Name}\", updating {(string.IsNullOrWhiteSpace(dp) ? "(nothing)" : dp)}, sections: {sections}, perms: {perms}"); + -1, $"User Group {group.Id} \"{group.Name}\" ({group.Alias})", + "umbraco/user-group/save", $"updating {(string.IsNullOrWhiteSpace(dp) ? "(nothing)" : dp)};{(sections == null ? "" : $", assigned sections: {sections}")}{(perms == null ? "" : $", assigned perms: {perms}")}"); } } @@ -126,10 +139,10 @@ namespace Umbraco.Core.Auditing var assigned = string.Join(", ", perm.AssignedPermissions); var entity = _entityServiceInstance.Get(perm.EntityId); - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", PerformingIp, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", PerformingIp, DateTime.Now, - 0, null, - "umbraco/user-group/permissions-change", $"assign group {(perm.IsDefaultPermissions ? "default " : "")}perms id:{group.Id}:{group.Alias} \"{group.Name}\", assigning {(string.IsNullOrWhiteSpace(assigned) ? "(nothing)" : assigned)} on id:{perm.EntityId} \"{entity.Name}\""); + -1, $"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}\""); } } @@ -139,13 +152,12 @@ namespace Umbraco.Core.Auditing var members = saveEventArgs.SavedEntities; foreach (var member in members) { - //var dp = string.Join(", ", member.Properties.Where(x => x.WasDirty()).Select(x => x.Alias)); - var dp = string.Join(", ", ((Member) member).GetWereDirtyProperties()); + var dp = string.Join(", ", ((Member) member).GetPreviouslyDirtyProperties()); - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", PerformingIp, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", PerformingIp, DateTime.Now, - 0, null, - "umbraco/member/save", $"save member id:{member.Id} \"{member.Name}\" <{member.Email}>, updating {(string.IsNullOrWhiteSpace(dp) ? "(nothing)" : dp)}"); + -1, $"Member {member.Id} \"{member.Name}\" {FormatEmail(member)}", + "umbraco/member/save", $"updating {(string.IsNullOrWhiteSpace(dp) ? "(nothing)" : dp)}"); } } @@ -155,10 +167,10 @@ namespace Umbraco.Core.Auditing var members = deleteEventArgs.DeletedEntities; foreach (var member in members) { - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", PerformingIp, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", PerformingIp, DateTime.Now, - 0, null, - "umbraco/member/delete", $"delete member id:{member.Id} \"{member.Name}\" <{member.Email}>"); + -1, $"Member {member.Id} \"{member.Name}\" {FormatEmail(member)}", + "umbraco/member/delete", $"delete member id:{member.Id} \"{member.Name}\" {FormatEmail(member)}"); } } @@ -168,17 +180,16 @@ namespace Umbraco.Core.Auditing var affectedUsers = saveEventArgs.SavedEntities; foreach (var affectedUser in affectedUsers) { - var sections = affectedUser.WasPropertyDirty("AllowedSections") - ? string.Join(", ", affectedUser.AllowedSections) - : null; var groups = affectedUser.WasPropertyDirty("Groups") ? string.Join(", ", affectedUser.Groups.Select(x => x.Alias)) : null; - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", PerformingIp, + var dp = string.Join(", ", ((User)affectedUser).GetPreviouslyDirtyProperties()); + + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", PerformingIp, DateTime.Now, - affectedUser.Id, $"User \"{affectedUser.Name}\" <{affectedUser.Email}>", - "umbraco/user/save", $"save user{(sections == null ? "" : (", sections: " + sections))}{(groups == null ? "" : (", groups: " + groups))}"); + affectedUser.Id, $"User \"{affectedUser.Name}\" {FormatEmail(affectedUser)}", + "umbraco/user/save", $"updating {(string.IsNullOrWhiteSpace(dp) ? "(nothing)" : dp)}{(groups == null ? "" : "; groups assigned: " + groups)}"); } } @@ -187,9 +198,9 @@ namespace Umbraco.Core.Auditing var performingUser = PerformingUser; var affectedUsers = deleteEventArgs.DeletedEntities; foreach (var affectedUser in affectedUsers) - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", PerformingIp, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", PerformingIp, DateTime.Now, - affectedUser.Id, $"User \"{affectedUser.Name}\" <{affectedUser.Email}>", + affectedUser.Id, $"User \"{affectedUser.Name}\" {FormatEmail(affectedUser)}", "umbraco/user/delete", "delete user"); } @@ -199,7 +210,7 @@ namespace Umbraco.Core.Auditing { var performingUser = _userServiceInstance.GetUserById(identityArgs.PerformingUser); if (performingUser == null) throw new InvalidOperationException($"No user found with id {identityArgs.PerformingUser}"); - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", identityArgs.IpAddress, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", identityArgs.IpAddress, DateTime.Now, 0, null, "umbraco/user/sign-in/login", "login success"); @@ -212,7 +223,7 @@ namespace Umbraco.Core.Auditing { var performingUser = _userServiceInstance.GetUserById(identityArgs.PerformingUser); if (performingUser == null) throw new InvalidOperationException($"No user found with id {identityArgs.PerformingUser}"); - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", identityArgs.IpAddress, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", identityArgs.IpAddress, DateTime.Now, 0, null, "umbraco/user/sign-in/logout", "logout success"); @@ -228,9 +239,9 @@ namespace Umbraco.Core.Auditing if (performingUser == null) throw new InvalidOperationException($"No user found with id {identityArgs.PerformingUser}"); var affectedUser = _userServiceInstance.GetUserById(identityArgs.AffectedUser); if (affectedUser == null) throw new InvalidOperationException($"No user found with id {identityArgs.AffectedUser}"); - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", identityArgs.IpAddress, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", identityArgs.IpAddress, DateTime.Now, - affectedUser.Id, $"User \"{affectedUser.Name}\" <{affectedUser.Email}>", + affectedUser.Id, $"User \"{affectedUser.Name}\" {FormatEmail(affectedUser)}", "umbraco/user/password/reset", "password reset"); } } @@ -244,9 +255,9 @@ namespace Umbraco.Core.Auditing if (performingUser == null) throw new InvalidOperationException($"No user found with id {identityArgs.PerformingUser}"); var affectedUser = _userServiceInstance.GetUserById(identityArgs.AffectedUser); if (affectedUser == null) throw new InvalidOperationException($"No user found with id {identityArgs.AffectedUser}"); - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", identityArgs.IpAddress, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", identityArgs.IpAddress, DateTime.Now, - affectedUser.Id, $"User \"{affectedUser.Name}\" <{affectedUser.Email}>", + affectedUser.Id, $"User \"{affectedUser.Name}\" {FormatEmail(affectedUser)}", "umbraco/user/password/change", "password change"); } } @@ -258,7 +269,7 @@ namespace Umbraco.Core.Auditing if (identityArgs.PerformingUser < 0) return; var performingUser = _userServiceInstance.GetUserById(identityArgs.PerformingUser); if (performingUser == null) throw new InvalidOperationException($"No user found with id {identityArgs.PerformingUser}"); - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", identityArgs.IpAddress, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", identityArgs.IpAddress, DateTime.Now, 0, null, "umbraco/user/sign-in/failed", "login failed"); @@ -273,9 +284,9 @@ namespace Umbraco.Core.Auditing if (performingUser == null) throw new InvalidOperationException($"No user found with id {identityArgs.PerformingUser}"); var affectedUser = _userServiceInstance.GetUserById(identityArgs.AffectedUser); if (affectedUser == null) throw new InvalidOperationException($"No user found with id {identityArgs.AffectedUser}"); - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", identityArgs.IpAddress, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", identityArgs.IpAddress, DateTime.Now, - affectedUser.Id, $"User \"{affectedUser.Name}\" <{affectedUser.Email}>", + affectedUser.Id, $"User \"{affectedUser.Name}\" {FormatEmail(affectedUser)}", "umbraco/user/password/forgot/change", "password forgot/change"); } } @@ -289,9 +300,9 @@ namespace Umbraco.Core.Auditing if (performingUser == null) throw new InvalidOperationException($"No user found with id {identityArgs.PerformingUser}"); var affectedUser = _userServiceInstance.GetUserById(identityArgs.AffectedUser); if (affectedUser == null) throw new InvalidOperationException($"No user found with id {identityArgs.AffectedUser}"); - _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" <{performingUser.Email}>", identityArgs.IpAddress, + _auditServiceInstance.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", identityArgs.IpAddress, DateTime.Now, - affectedUser.Id, $"User \"{affectedUser.Name}\" <{affectedUser.Email}>", + affectedUser.Id, $"User \"{affectedUser.Name}\" {FormatEmail(affectedUser)}", "umbraco/user/password/forgot/request", "password forgot/request"); } } diff --git a/src/Umbraco.Core/Auditing/IdentityAuditEventArgs.cs b/src/Umbraco.Core/Auditing/IdentityAuditEventArgs.cs index 3dd6a86bd9..c58bb409b0 100644 --- a/src/Umbraco.Core/Auditing/IdentityAuditEventArgs.cs +++ b/src/Umbraco.Core/Auditing/IdentityAuditEventArgs.cs @@ -46,12 +46,8 @@ namespace Umbraco.Core.Auditing /// public string Username { get; private set; } - /// - /// Sets the properties on the event being raised, all parameters are optional except for the action being performed - /// - /// An action based on the AuditEvent enum - /// The client's IP address. This is usually automatically set but could be overridden if necessary - /// The Id of the user performing the action (if different from the user affected by the action) + [Obsolete("Use the method that has the affectedUser parameter instead")] + [EditorBrowsable(EditorBrowsableState.Never)] public IdentityAuditEventArgs(AuditEvent action, string ipAddress, int performingUser = -1) { DateTimeUtc = DateTime.UtcNow; @@ -65,7 +61,29 @@ namespace Umbraco.Core.Auditing } /// - /// Creates an instance without a performing user (the id will be set to -1) + /// Default constructor + /// + /// + /// + /// + /// + /// + public IdentityAuditEventArgs(AuditEvent action, string ipAddress, string comment = null, int performingUser = -1, int affectedUser = -1) + { + DateTimeUtc = DateTime.UtcNow; + Action = action; + + IpAddress = ipAddress; + Comment = comment; + AffectedUser = affectedUser; + + PerformingUser = performingUser == -1 + ? GetCurrentRequestBackofficeUserId() + : performingUser; + } + + /// + /// Creates an instance without a performing or affected user (the id will be set to -1) /// /// /// diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index ebbb3fa80e..dc6228d19d 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -544,6 +544,28 @@ namespace Umbraco.Core.Models return false; } + /// + /// Returns both instance dirty properties and property type properties + /// + /// + public override IEnumerable GetDirtyProperties() + { + var instanceProperties = base.GetDirtyProperties(); + var propertyTypes = Properties.Where(x => x.IsDirty()).Select(x => x.Alias); + return instanceProperties.Concat(propertyTypes); + } + + /// + /// Returns both instance dirty properties and property type properties + /// + /// + internal override IEnumerable GetPreviouslyDirtyProperties() + { + var instanceProperties = base.GetPreviouslyDirtyProperties(); + var propertyTypes = Properties.Where(x => x.WasDirty()).Select(x => x.Alias); + return instanceProperties.Concat(propertyTypes); + } + #endregion } -} \ No newline at end of file +} diff --git a/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs b/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs index e43c9ffb66..2591a3c73d 100644 --- a/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs +++ b/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs @@ -23,7 +23,7 @@ namespace Umbraco.Core.Models.EntityBase : _propertyChangedInfo.Where(x => x.Value).Select(x => x.Key); } - internal virtual IEnumerable GetWereDirtyProperties() + internal virtual IEnumerable GetPreviouslyDirtyProperties() { return _lastPropertyChangedInfo == null ? Enumerable.Empty() diff --git a/src/Umbraco.Core/Models/Member.cs b/src/Umbraco.Core/Models/Member.cs index 1ed11af93e..bd54f9c6f8 100644 --- a/src/Umbraco.Core/Models/Member.cs +++ b/src/Umbraco.Core/Models/Member.cs @@ -170,7 +170,19 @@ namespace Umbraco.Core.Models public string RawPasswordValue { get { return _rawPasswordValue; } - set { SetPropertyValueAndDetectChanges(value, ref _rawPasswordValue, Ps.Value.PasswordSelector); } + set + { + if (value == null) + { + //special case, this is used to ensure that the password is not updated when persisting, in this case + //we don't want to track changes either + _rawPasswordValue = null; + } + else + { + SetPropertyValueAndDetectChanges(value, ref _rawPasswordValue, Ps.Value.PasswordSelector); + } + } } /// @@ -628,4 +640,4 @@ namespace Umbraco.Core.Models } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index 892de4a7e1..cd9a84758e 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -706,5 +706,6 @@ namespace Umbraco.Core.Models.Membership return _user.GetHashCode(); } } + } -} \ No newline at end of file +} diff --git a/src/Umbraco.Core/Models/Membership/UserGroup.cs b/src/Umbraco.Core/Models/Membership/UserGroup.cs index 7042c63ec1..85b497d511 100644 --- a/src/Umbraco.Core/Models/Membership/UserGroup.cs +++ b/src/Umbraco.Core/Models/Membership/UserGroup.cs @@ -152,4 +152,4 @@ namespace Umbraco.Core.Models.Membership public int UserCount { get; private set; } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Core/Security/BackOfficeUserManager.cs b/src/Umbraco.Core/Security/BackOfficeUserManager.cs index 4969fa5f2f..2cb026ad3e 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserManager.cs @@ -420,6 +420,33 @@ namespace Umbraco.Core.Security return await base.CheckPasswordAsync(user, password); } + public override Task ResetPasswordAsync(int userId, string token, string newPassword) + { + var result = base.ResetPasswordAsync(userId, token, newPassword); + if (result.Result.Succeeded) + RaisePasswordResetEvent(userId); + return result; + } + + /// + /// This is a special method that will reset the password but will raise the Password Changed event instead of the reset event + /// + /// + /// + /// + /// + /// + /// We use this because in the back office the only way an admin can change another user's password without first knowing their password + /// is to generate a token and reset it, however, when we do this we want to track a password change, not a password reset + /// + public Task ChangePasswordWithResetAsync(int userId, string token, string newPassword) + { + var result = base.ResetPasswordAsync(userId, token, newPassword); + if (result.Result.Succeeded) + RaisePasswordChangedEvent(userId); + return result; + } + public override Task ChangePasswordAsync(int userId, string currentPassword, string newPassword) { var result = base.ChangePasswordAsync(userId, currentPassword, newPassword); @@ -554,27 +581,27 @@ namespace Umbraco.Core.Security internal void RaiseAccountLockedEvent(int userId) { - OnAccountLocked(new IdentityAuditEventArgs(AuditEvent.AccountLocked, GetCurrentRequestIpAddress(), userId)); + OnAccountLocked(new IdentityAuditEventArgs(AuditEvent.AccountLocked, GetCurrentRequestIpAddress(), affectedUser: userId)); } internal void RaiseAccountUnlockedEvent(int userId) { - OnAccountUnlocked(new IdentityAuditEventArgs(AuditEvent.AccountUnlocked, GetCurrentRequestIpAddress(), userId)); + OnAccountUnlocked(new IdentityAuditEventArgs(AuditEvent.AccountUnlocked, GetCurrentRequestIpAddress(), affectedUser: userId)); } internal void RaiseForgotPasswordRequestedEvent(int userId) { - OnForgotPasswordRequested(new IdentityAuditEventArgs(AuditEvent.ForgotPasswordRequested, GetCurrentRequestIpAddress(), userId)); + OnForgotPasswordRequested(new IdentityAuditEventArgs(AuditEvent.ForgotPasswordRequested, GetCurrentRequestIpAddress(), affectedUser: userId)); } internal void RaiseForgotPasswordChangedSuccessEvent(int userId) { - OnForgotPasswordChangedSuccess(new IdentityAuditEventArgs(AuditEvent.ForgotPasswordChangedSuccess, GetCurrentRequestIpAddress(), userId)); + OnForgotPasswordChangedSuccess(new IdentityAuditEventArgs(AuditEvent.ForgotPasswordChangedSuccess, GetCurrentRequestIpAddress(), affectedUser: userId)); } internal void RaiseLoginFailedEvent(int userId) { - OnLoginFailed(new IdentityAuditEventArgs(AuditEvent.LoginFailed, GetCurrentRequestIpAddress(), userId)); + OnLoginFailed(new IdentityAuditEventArgs(AuditEvent.LoginFailed, GetCurrentRequestIpAddress(), affectedUser: userId)); } internal void RaiseInvalidLoginAttemptEvent(string username) @@ -584,31 +611,33 @@ namespace Umbraco.Core.Security internal void RaiseLoginRequiresVerificationEvent(int userId) { - OnLoginRequiresVerification(new IdentityAuditEventArgs(AuditEvent.LoginRequiresVerification, GetCurrentRequestIpAddress(), userId)); + OnLoginRequiresVerification(new IdentityAuditEventArgs(AuditEvent.LoginRequiresVerification, GetCurrentRequestIpAddress(), affectedUser: userId)); } internal void RaiseLoginSuccessEvent(int userId) { - OnLoginSuccess(new IdentityAuditEventArgs(AuditEvent.LoginSucces, GetCurrentRequestIpAddress(), userId)); + OnLoginSuccess(new IdentityAuditEventArgs(AuditEvent.LoginSucces, GetCurrentRequestIpAddress(), affectedUser: userId)); } internal void RaiseLogoutSuccessEvent(int userId) { - OnLogoutSuccess(new IdentityAuditEventArgs(AuditEvent.LogoutSuccess, GetCurrentRequestIpAddress(), userId)); + OnLogoutSuccess(new IdentityAuditEventArgs(AuditEvent.LogoutSuccess, GetCurrentRequestIpAddress(), affectedUser: userId)); } internal void RaisePasswordChangedEvent(int userId) { - OnPasswordChanged(new IdentityAuditEventArgs(AuditEvent.PasswordChanged, GetCurrentRequestIpAddress(), userId)); + OnPasswordChanged(new IdentityAuditEventArgs(AuditEvent.PasswordChanged, GetCurrentRequestIpAddress(), affectedUser: userId)); } + //TODO: I don't think this is required anymore since from 7.7 we no longer display the reset password checkbox since that didn't make sense. internal void RaisePasswordResetEvent(int userId) { - OnPasswordReset(new IdentityAuditEventArgs(AuditEvent.PasswordReset, GetCurrentRequestIpAddress(), userId)); + OnPasswordReset(new IdentityAuditEventArgs(AuditEvent.PasswordReset, GetCurrentRequestIpAddress(), affectedUser: userId)); } + internal void RaiseResetAccessFailedCountEvent(int userId) { - OnResetAccessFailedCount(new IdentityAuditEventArgs(AuditEvent.ResetAccessFailedCount, GetCurrentRequestIpAddress(), userId)); + OnResetAccessFailedCount(new IdentityAuditEventArgs(AuditEvent.ResetAccessFailedCount, GetCurrentRequestIpAddress(), affectedUser: userId)); } public static event EventHandler AccountLocked; diff --git a/src/Umbraco.Core/Services/AuditService.cs b/src/Umbraco.Core/Services/AuditService.cs index 3d5dc628b7..82cec0385b 100644 --- a/src/Umbraco.Core/Services/AuditService.cs +++ b/src/Umbraco.Core/Services/AuditService.cs @@ -120,6 +120,10 @@ namespace Umbraco.Core.Services 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?.Substring(0, Math.Min(affectedDetails.Length, 1024)); + eventDetails = eventDetails.Substring(0, Math.Min(eventDetails.Length, 1024)); + //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) diff --git a/src/Umbraco.Web/Editors/CurrentUserController.cs b/src/Umbraco.Web/Editors/CurrentUserController.cs index 9ed65888d4..6e4326ad80 100644 --- a/src/Umbraco.Web/Editors/CurrentUserController.cs +++ b/src/Umbraco.Web/Editors/CurrentUserController.cs @@ -144,15 +144,12 @@ namespace Umbraco.Web.Editors { var userMgr = this.TryGetOwinContext().Result.GetBackOfficeUserManager(); - //raise the appropriate event + //raise the reset event + //TODO: I don't think this is required anymore since from 7.7 we no longer display the reset password checkbox since that didn't make sense. if (data.Reset.HasValue && data.Reset.Value) { userMgr.RaisePasswordResetEvent(Security.CurrentUser.Id); } - else - { - userMgr.RaisePasswordChangedEvent(Security.CurrentUser.Id); - } //even if we weren't resetting this, it is the correct value (null), otherwise if we were resetting then it will contain the new pword var result = new ModelWithNotifications(passwordChangeResult.Result.ResetPassword); diff --git a/src/Umbraco.Web/Editors/PasswordChanger.cs b/src/Umbraco.Web/Editors/PasswordChanger.cs index 7ddf361d23..52f96934a0 100644 --- a/src/Umbraco.Web/Editors/PasswordChanger.cs +++ b/src/Umbraco.Web/Editors/PasswordChanger.cs @@ -93,7 +93,7 @@ namespace Umbraco.Web.Editors ? userMgr.GeneratePassword() : passwordModel.NewPassword; - var resetResult = await userMgr.ResetPasswordAsync(savingUser.Id, resetToken, newPass); + var resetResult = await userMgr.ChangePasswordWithResetAsync(savingUser.Id, resetToken, newPass); if (resetResult.Succeeded == false) { @@ -166,6 +166,7 @@ namespace Umbraco.Web.Editors } //Are we resetting the password?? + //TODO: I don't think this is required anymore since from 7.7 we no longer display the reset password checkbox since that didn't make sense. if (passwordModel.Reset.HasValue && passwordModel.Reset.Value) { var canReset = membershipProvider.CanResetPassword(_userService); @@ -291,4 +292,4 @@ namespace Umbraco.Web.Editors } } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Web/Editors/UsersController.cs b/src/Umbraco.Web/Editors/UsersController.cs index 2e6cd5dbbf..a9359e4baf 100644 --- a/src/Umbraco.Web/Editors/UsersController.cs +++ b/src/Umbraco.Web/Editors/UsersController.cs @@ -561,18 +561,10 @@ namespace Umbraco.Web.Editors { var passwordChanger = new PasswordChanger(Logger, Services.UserService, UmbracoContext.HttpContext); + //this will change the password and raise appropriate events var passwordChangeResult = await passwordChanger.ChangePasswordWithIdentityAsync(Security.CurrentUser, found, userSave.ChangePassword, UserManager); if (passwordChangeResult.Success) { - var userMgr = this.TryGetOwinContext().Result.GetBackOfficeUserManager(); - - //raise the event - NOTE that the ChangePassword.Reset value here doesn't mean it's been 'reset', it means - //it's been changed by a back office user - if (userSave.ChangePassword.Reset.HasValue && userSave.ChangePassword.Reset.Value) - { - userMgr.RaisePasswordChangedEvent(intId.Result); - } - //need to re-get the user found = Services.UserService.GetUserById(intId.Result); } @@ -722,4 +714,4 @@ namespace Umbraco.Web.Editors } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Web/Models/Mapping/EntityModelMapperExtensions.cs b/src/Umbraco.Web/Models/Mapping/EntityModelMapperExtensions.cs index 39f92af5d2..f485002985 100644 --- a/src/Umbraco.Web/Models/Mapping/EntityModelMapperExtensions.cs +++ b/src/Umbraco.Web/Models/Mapping/EntityModelMapperExtensions.cs @@ -39,4 +39,4 @@ namespace Umbraco.Web.Models.Mapping } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs index 446a63ee99..737a66b7ce 100644 --- a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs @@ -28,10 +28,13 @@ namespace Umbraco.Web.Models.Mapping .ConstructUsing((UserGroupSave save) => new UserGroup() { CreateDate = DateTime.Now }) .IgnoreDeletableEntityCommonProperties() .ForMember(dest => dest.Id, map => map.Condition(source => GetIntId(source.Id) > 0)) - .ForMember(dest => dest.Id, map => map.MapFrom(source => GetIntId(source.Id))) - .ForMember(dest => dest.Permissions, map => map.MapFrom(source => source.DefaultPermissions)) + .ForMember(dest => dest.Id, map => map.MapFrom(source => GetIntId(source.Id))) + //.ForMember(dest => dest.Permissions, map => map.MapFrom(source => source.DefaultPermissions)) + .ForMember(dest => dest.Permissions, map => map.Ignore()) .AfterMap((save, userGroup) => { + userGroup.Permissions = save.DefaultPermissions; + userGroup.ClearAllowedSections(); foreach (var section in save.Sections) { @@ -466,4 +469,4 @@ namespace Umbraco.Web.Models.Mapping } } -} \ No newline at end of file +}