diff --git a/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs b/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs index 60fe4daace..c17ef25c43 100644 --- a/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs +++ b/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs @@ -9,7 +9,8 @@ namespace Umbraco.Core.Models.Mapping /// public void DefineMaps(UmbracoMapper mapper) => mapper.Define(Map); - // mappers + //TODO: put this here instead of a new mapper definition (like user). Can move + private static void Map(MemberSave source, IMember target, MapperContext context) { // TODO: ensure all properties are mapped as required @@ -18,8 +19,8 @@ namespace Umbraco.Core.Models.Mapping target.Email = source.Email; target.Key = source.Key; target.Username = source.Username; - target.Id = (int)(long)source.Id; target.Comments = source.Comments; + //TODO: add groups as required } } } diff --git a/src/Umbraco.Core/Security/MembersUserPasswordCheckerResult.cs b/src/Umbraco.Core/Security/MembersUserPasswordCheckerResult.cs deleted file mode 100644 index 3212609ed9..0000000000 --- a/src/Umbraco.Core/Security/MembersUserPasswordCheckerResult.cs +++ /dev/null @@ -1,12 +0,0 @@ -namespace Umbraco.Core.Security -{ - /// - /// The result returned from the IMembersUserPasswordChecker - /// - public enum MembersUserPasswordCheckerResult - { - ValidCredentials, - InvalidCredentials, - FallbackToDefaultChecker - } -} diff --git a/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs b/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs index 30b11cc8a8..be4ffdf2e2 100644 --- a/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs +++ b/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs @@ -243,6 +243,23 @@ namespace Umbraco.Infrastructure.Security /// A generated password string GeneratePassword(); + /// + /// Generates a hashed password for a null user based on the default password hasher + /// + /// The password to hash + /// The hashed password + string GeneratePassword(string password); + + /// + /// Used to validate the password without an identity user + /// Validation code is based on the default ValidatePasswordAsync code + /// Should return if validation is successful + /// + /// The password. + /// A representing whether validation was successful. + + Task ValidatePasswordAsync(string password); + /// /// Generates an email confirmation token for the specified user. /// diff --git a/src/Umbraco.Infrastructure/Security/MembersIdentityUser.cs b/src/Umbraco.Infrastructure/Security/MembersIdentityUser.cs index 4e13e6839d..e7b7450704 100644 --- a/src/Umbraco.Infrastructure/Security/MembersIdentityUser.cs +++ b/src/Umbraco.Infrastructure/Security/MembersIdentityUser.cs @@ -40,8 +40,6 @@ namespace Umbraco.Infrastructure.Security user.UserName = username; user.Email = email; user.MemberTypeAlias = memberTypeAlias; - // TODO: confirm if should be approved - user.IsApproved = true; user.Id = null; user.HasIdentity = false; user._name = name; diff --git a/src/Umbraco.Infrastructure/Security/MembersUserStore.cs b/src/Umbraco.Infrastructure/Security/MembersUserStore.cs index 70ca06a17a..aa28ee5d25 100644 --- a/src/Umbraco.Infrastructure/Security/MembersUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/MembersUserStore.cs @@ -65,21 +65,18 @@ namespace Umbraco.Infrastructure.Security } // create member - // TODO: are we keeping this method, e.g. the Member Service? - // The user service creates it directly, but this way we get the member type by alias first + // TODO: are we keeping this method? The user service creates the member directly + // but this way we get the member type by alias first IMember memberEntity = _memberService.CreateMember( user.UserName, user.Email, user.Name.IsNullOrWhiteSpace() ? user.UserName : user.Name, user.MemberTypeAlias.IsNullOrWhiteSpace() ? Constants.Security.DefaultMemberTypeAlias : user.MemberTypeAlias); - // [from backofficeuser] we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. - var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(MembersIdentityUser.Logins)); UpdateMemberProperties(memberEntity, user); - // TODO: do we want to accept empty passwords here - if third-party for example? - // In other method if so? + // create the member _memberService.Save(memberEntity); if (!memberEntity.HasIdentity) @@ -90,7 +87,9 @@ namespace Umbraco.Infrastructure.Security // re-assign id user.Id = UserIdToString(memberEntity.Id); - //TODO: confirm re externallogins implementation + // [from backofficeuser] we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. + // var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(MembersIdentityUser.Logins)); + // TODO: confirm re externallogins implementation //if (isLoginsPropertyDirty) //{ // _externalLoginService.Save( @@ -101,9 +100,9 @@ namespace Umbraco.Infrastructure.Security // x.UserData))); //} - return Task.FromResult(IdentityResult.Success); - // TODO: confirm re roles implementations + + return Task.FromResult(IdentityResult.Success); } /// @@ -230,7 +229,7 @@ namespace Umbraco.Infrastructure.Security return string.IsNullOrEmpty(user.PasswordHash) == false; } - return result; + return false; } /// diff --git a/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs b/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs index 6318218669..2000e108b7 100644 --- a/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs +++ b/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Identity; @@ -111,6 +112,54 @@ namespace Umbraco.Core.Security return password; } + /// + /// Generates a hashed password based on the default password hasher + /// No existing identity user is required and this does not validate the password + /// + /// The password to hash + /// The hashed password + public string GeneratePassword(string password) + { + IPasswordHasher passwordHasher = GetDefaultPasswordHasher(PasswordConfiguration); + + string hashedPassword = passwordHasher.HashPassword(null, password); + return hashedPassword; + } + + /// + /// Used to validate the password without an identity user + /// Validation code is based on the default ValidatePasswordAsync code + /// Should return if validation is successful + /// + /// The password. + /// A representing whether validation was successful. + public async Task ValidatePasswordAsync(string password) + { + var errors = new List(); + var isValid = true; + foreach (IPasswordValidator v in PasswordValidators) + { + IdentityResult result = await v.ValidateAsync(this, null, password); + if (!result.Succeeded) + { + if (result.Errors.Any()) + { + errors.AddRange(result.Errors); + } + + isValid = false; + } + } + + if (!isValid) + { + Logger.LogWarning(14, "Password validation failed: {errors}.", string.Join(";", errors.Select(e => e.Code))); + return IdentityResult.Failed(errors.ToArray()); + } + + return IdentityResult.Success; + } + /// public override async Task CheckPasswordAsync(TUser user, string password) { diff --git a/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs b/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs index a8875de286..60bab5805f 100644 --- a/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs +++ b/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs @@ -196,6 +196,8 @@ namespace Umbraco.Tests.Integration.Testing builder.AddRuntimeMinifier(); builder.AddBackOffice(); builder.AddBackOfficeIdentity(); + builder.AddMembersIdentity(); + services.AddMvc(); diff --git a/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/MembersServiceCollectionExtensionsTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/MembersServiceCollectionExtensionsTests.cs index 64d62036ec..4991df7bd1 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/MembersServiceCollectionExtensionsTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/MembersServiceCollectionExtensionsTests.cs @@ -5,6 +5,7 @@ using NUnit.Framework; using Umbraco.Extensions; using Umbraco.Infrastructure.Security; using Umbraco.Tests.Integration.Testing; +using Umbraco.Web.BackOffice.Extensions; namespace Umbraco.Tests.Integration.Umbraco.Web.BackOffice { diff --git a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs index d980e51fdf..c13c02293d 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs @@ -237,16 +237,6 @@ namespace Umbraco.Web.BackOffice.Controllers [MemberSaveValidation] public async Task> PostSave([ModelBinder(typeof(MemberBinder))] MemberSave contentItem) { - if (contentItem == null) - { - throw new ArgumentNullException(nameof(contentItem)); - } - - if (ModelState.IsValid == false) - { - throw new HttpResponseException(HttpStatusCode.BadRequest, ModelState); - } - // If we've reached here it means: // * Our model has been bound // * and validated @@ -257,7 +247,7 @@ namespace Umbraco.Web.BackOffice.Controllers // map the properties to the persisted entity MapPropertyValues(contentItem); - ValidateMemberData(contentItem); + await ValidateMemberDataAsync(contentItem); // Unlike content/media - if there are errors for a member, we do NOT proceed to save them, we cannot so return the errors if (ModelState.IsValid == false) @@ -266,23 +256,14 @@ namespace Umbraco.Web.BackOffice.Controllers forDisplay.Errors = ModelState.ToErrorDictionary(); throw HttpResponseException.CreateValidationErrorResponse(forDisplay); } - - // We're gonna look up the current roles now because the below code can cause - // events to be raised and developers could be manually adding roles to members in - // their handlers. If we don't look this up now there's a chance we'll just end up - // removing the roles they've assigned. - IEnumerable currentRoles = _memberService.GetAllRoles(contentItem.PersistedContent.Username); - - // find the ones to remove and remove them - IEnumerable roles = currentRoles.ToList(); - string[] rolesToRemove = roles.Except(contentItem.Groups).ToArray(); - + // Depending on the action we need to first do a create or update using the membership manager // this ensures that passwords are formatted correctly and also performs the validation on the provider itself. + switch (contentItem.Action) { case ContentSaveAction.Save: - await UpdateMemberDataAsync(contentItem); + await UpdateMemberAsync(contentItem); break; case ContentSaveAction.SaveNew: await CreateMemberAsync(contentItem); @@ -295,22 +276,7 @@ namespace Umbraco.Web.BackOffice.Controllers // TODO: There's 3 things saved here and we should do this all in one transaction, // which we can do here by wrapping in a scope // but it would be nicer to have this taken care of within the Save method itself - - // Now let's do the role provider stuff - now that we've saved the content item (that is important since - // if we are changing the username, it must be persisted before looking up the member roles). - if (rolesToRemove.Any()) - { - _memberService.DissociateRoles(new[] { contentItem.PersistedContent.Username }, rolesToRemove); - } - - // find the ones to add and add them - string[] toAdd = contentItem.Groups.Except(roles).ToArray(); - if (toAdd.Any()) - { - // add the ones submitted - _memberService.AssignRoles(new[] { contentItem.PersistedContent.Username }, toAdd); - } - + // return the updated model MemberDisplay display = _umbracoMapper.Map(contentItem.PersistedContent); @@ -365,7 +331,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// Member content data /// The identity result of the created member - private async Task CreateMemberAsync(MemberSave contentItem) + private async Task CreateMemberAsync(MemberSave contentItem) { IMemberType memberType = _memberTypeService.Get(contentItem.ContentTypeAlias); if (memberType == null) @@ -373,25 +339,13 @@ namespace Umbraco.Web.BackOffice.Controllers throw new InvalidOperationException($"No member type found with alias {contentItem.ContentTypeAlias}"); } - // Create the member with the MemberManager var identityMember = MembersIdentityUser.CreateNew( contentItem.Username, contentItem.Email, memberType.Alias, contentItem.Name); - if (contentItem.Password != null && !contentItem.Password.NewPassword.IsNullOrWhiteSpace()) - { - // TODO: should we show the password rules? - Task isPasswordValid = _memberManager.CheckPasswordAsync(identityMember, contentItem.Password.NewPassword); - if (isPasswordValid.Result == false) - { - ModelState.AddPropertyError( - new ValidationResult($"Invalid password", new[] { "value" }), - $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}password"); - } - } - + // TODO: may not need to add password like this IdentityResult created = await _memberManager.CreateAsync(identityMember, contentItem.Password.NewPassword); if (created.Succeeded == false) @@ -402,19 +356,17 @@ namespace Umbraco.Web.BackOffice.Controllers // now re-look the member back up which will now exist IMember member = _memberService.GetByEmail(contentItem.Email); - member.CreatorId = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id; - // TODO: should this be removed since we've moved passwords out? - - member.RawPasswordValue = identityMember.PasswordHash; - member.Comments = contentItem.Comments; - - // since the back office user is creating this member, they will be set to approved - member.IsApproved = true; + var creatorId = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id; + member.CreatorId = creatorId; // map the save info over onto the user - member = _umbracoMapper.Map(contentItem, member); + member = _umbracoMapper.Map(contentItem, member); + + // now the member has been saved via identity, resave the member with mapped content properties + _memberService.Save(member); contentItem.PersistedContent = member; - return created; + + AddOrUpdateRoles(contentItem); } /// @@ -422,20 +374,8 @@ namespace Umbraco.Web.BackOffice.Controllers /// If the password has been reset then this method will return the reset/generated password, otherwise will return null. /// /// The member to save - private async Task UpdateMemberDataAsync(MemberSave contentItem) + private async Task UpdateMemberAsync(MemberSave contentItem) { - MembersIdentityUser identityMember = await _memberManager.FindByIdAsync(contentItem.Id.ToString()); - if (identityMember == null) - { - } - - IdentityResult updatedResult = await _memberManager.UpdateAsync(identityMember); - - if (updatedResult.Succeeded == false) - { - throw HttpResponseException.CreateNotificationValidationErrorResponse(updatedResult.Errors.ToErrorMessage()); - } - contentItem.PersistedContent.WriterId = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id; // If the user doesn't have access to sensitive values, then we need to check if any of the built in member property types @@ -476,21 +416,59 @@ namespace Umbraco.Web.BackOffice.Controllers ModelState.AddModelError("custom", "An admin cannot lock a user"); } - // no password changes then exit ? + MembersIdentityUser identityMember = await _memberManager.FindByIdAsync(contentItem.Id.ToString()); + if (identityMember == null) + { + throw HttpResponseException.CreateNotificationValidationErrorResponse("Member was not found"); + } + if (contentItem.Password != null) { - // TODO: set the password using Identity core - contentItem.PersistedContent.RawPasswordValue = _memberManager.GeneratePassword(); + IdentityResult validatePassword = await _memberManager.ValidatePasswordAsync(contentItem.Password.NewPassword); + if (validatePassword.Succeeded == false) + { + throw HttpResponseException.CreateNotificationValidationErrorResponse(validatePassword.Errors.ToErrorMessage()); + } + + string newPassword = _memberManager.GeneratePassword(contentItem.Password.NewPassword); + identityMember.PasswordHash = newPassword; } + + IdentityResult updatedResult = await _memberManager.UpdateAsync(identityMember); + + if (updatedResult.Succeeded == false) + { + throw HttpResponseException.CreateNotificationValidationErrorResponse(updatedResult.Errors.ToErrorMessage()); + } + + contentItem.PersistedContent.RawPasswordValue = identityMember.PasswordHash; + + _memberService.Save(contentItem.PersistedContent); + + AddOrUpdateRoles(contentItem); } - private void ValidateMemberData(MemberSave contentItem) + private async Task ValidateMemberDataAsync(MemberSave contentItem) { if (contentItem.Name.IsNullOrWhiteSpace()) { ModelState.AddPropertyError( new ValidationResult("Invalid user name", new[] { "value" }), $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}login"); + return false; + } + + if (contentItem.Password != null && !contentItem.Password.NewPassword.IsNullOrWhiteSpace()) + { + // TODO: this currently stops the user interacting with the client-side when invalid + IdentityResult validPassword = await _memberManager.ValidatePasswordAsync(contentItem.Password.NewPassword); + if (!validPassword.Succeeded) + { + ModelState.AddPropertyError( + new ValidationResult("Invalid password: " + MapErrors(validPassword.Errors), new[] { "value" }), + $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}password"); + return false; + } } IMember byUsername = _memberService.GetByUsername(contentItem.Username); @@ -499,6 +477,7 @@ namespace Umbraco.Web.BackOffice.Controllers ModelState.AddPropertyError( new ValidationResult("Username is already in use", new[] { "value" }), $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}login"); + return false; } IMember byEmail = _memberService.GetByEmail(contentItem.Email); @@ -507,22 +486,57 @@ namespace Umbraco.Web.BackOffice.Controllers ModelState.AddPropertyError( new ValidationResult("Email address is already in use", new[] { "value" }), $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}email"); + return false; } + + return true; } - private string MapErrors(List result) + private string MapErrors(IEnumerable result) { var sb = new StringBuilder(); - IEnumerable errors = result.Where(x => x.Succeeded == false); - - foreach (IdentityResult error in errors) + IEnumerable identityErrors = result.ToList(); + foreach (IdentityError error in identityErrors) { - sb.AppendLine(error.Errors.ToErrorMessage()); + string errorString = $"{error.Description}"; + sb.AppendLine(errorString); } return sb.ToString(); } + /// + /// TODO: refactor using identity roles + /// + /// + private void AddOrUpdateRoles(MemberSave contentItem) + { + // We're gonna look up the current roles now because the below code can cause + // events to be raised and developers could be manually adding roles to members in + // their handlers. If we don't look this up now there's a chance we'll just end up + // removing the roles they've assigned. + IEnumerable currentRoles = _memberService.GetAllRoles(contentItem.PersistedContent.Username); + + // find the ones to remove and remove them + IEnumerable roles = currentRoles.ToList(); + string[] rolesToRemove = roles.Except(contentItem.Groups).ToArray(); + + // Now let's do the role provider stuff - now that we've saved the content item (that is important since + // if we are changing the username, it must be persisted before looking up the member roles). + if (rolesToRemove.Any()) + { + _memberService.DissociateRoles(new[] { contentItem.PersistedContent.Username }, rolesToRemove); + } + + // find the ones to add and add them + string[] toAdd = contentItem.Groups.Except(roles).ToArray(); + if (toAdd.Any()) + { + // add the ones submitted + _memberService.AssignRoles(new[] { contentItem.PersistedContent.Username }, toAdd); + } + } + /// /// Permanently deletes a member /// diff --git a/src/Umbraco.Web.BackOffice/Extensions/UmbracoMemberIdentityBuilderExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/MemberIdentityBuilderExtensions.cs similarity index 69% rename from src/Umbraco.Web.BackOffice/Extensions/UmbracoMemberIdentityBuilderExtensions.cs rename to src/Umbraco.Web.BackOffice/Extensions/MemberIdentityBuilderExtensions.cs index 888d963891..e3551aa363 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/UmbracoMemberIdentityBuilderExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/MemberIdentityBuilderExtensions.cs @@ -2,20 +2,19 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; using Umbraco.Infrastructure.Security; -namespace Umbraco.Extensions +namespace Umbraco.Web.BackOffice.Extensions { - public static class UmbracoMemberIdentityBuilderExtensions + public static class MemberIdentityBuilderExtensions { - /// - /// Adds a for the . + /// Adds a for the . /// /// The type of the user manager to add. /// /// The current instance. - public static IdentityBuilder AddUserManager(this IdentityBuilder identityBuilder) where TUserManager : UserManager, TInterface + public static IdentityBuilder AddUserManager(this IdentityBuilder identityBuilder) + where TUserManager : UserManager, TInterface { - identityBuilder.AddUserManager(); identityBuilder.Services.AddScoped(typeof(TInterface), typeof(TUserManager)); return identityBuilder; } diff --git a/src/Umbraco.Web.BackOffice/Extensions/MembersUserServiceCollectionExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/MembersUserServiceCollectionExtensions.cs index 62b4904838..64e583b4b9 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/MembersUserServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/MembersUserServiceCollectionExtensions.cs @@ -4,7 +4,7 @@ using Microsoft.Extensions.DependencyInjection.Extensions; using Umbraco.Infrastructure.Security; using Umbraco.Web.Common.Security; -namespace Umbraco.Extensions +namespace Umbraco.Web.BackOffice.Extensions { public static class MembersUserServiceCollectionExtensions { diff --git a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBuilderExtensions.cs index 8d35239e2a..82b2e88d5f 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBuilderExtensions.cs @@ -2,6 +2,7 @@ using System; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.DependencyInjection; using Umbraco.Core.DependencyInjection; +using Umbraco.Web.BackOffice.Extensions; using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.BackOffice.Security; diff --git a/src/Umbraco.Web.BackOffice/Mapping/MemberMapDefinition.cs b/src/Umbraco.Web.BackOffice/Mapping/MemberMapDefinition.cs index 2df45704d8..b331d17fdc 100644 --- a/src/Umbraco.Web.BackOffice/Mapping/MemberMapDefinition.cs +++ b/src/Umbraco.Web.BackOffice/Mapping/MemberMapDefinition.cs @@ -1,4 +1,4 @@ -using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http; using Umbraco.Core; using Umbraco.Core.Mapping; using Umbraco.Core.Models; @@ -93,14 +93,11 @@ namespace Umbraco.Web.BackOffice.Mapping target.Path = $"-1,{source.Id}"; target.Udi = Udi.Create(Constants.UdiEntityType.MemberGroup, source.Key); } - + // Umbraco.Code.MapAll private static void Map(IMember source, ContentPropertyCollectionDto target, MapperContext context) { target.Properties = context.MapEnumerable(source.Properties); } - - - } } diff --git a/src/Umbraco.Web.Common/Security/MembersUserManager.cs b/src/Umbraco.Web.Common/Security/MembersUserManager.cs index 6e4633934c..86ea7b1972 100644 --- a/src/Umbraco.Web.Common/Security/MembersUserManager.cs +++ b/src/Umbraco.Web.Common/Security/MembersUserManager.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Security.Principal; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -40,102 +41,6 @@ namespace Umbraco.Web.Common.Security { _httpContextAccessor = httpContextAccessor; } - - /// - /// Override to check the user approval value as well as the user lock out date, by default this only checks the user's locked out date - /// - /// The user - /// True if the user is locked out, else false - /// - /// In the ASP.NET Identity world, there is only one value for being locked out, in Umbraco we have 2 so when checking this for Umbraco we need to check both values - /// - public override async Task IsLockedOutAsync(MembersIdentityUser user) - { - if (user == null) - { - throw new ArgumentNullException(nameof(user)); - } - - if (user.IsApproved == false) - { - return true; - } - - return await base.IsLockedOutAsync(user); - } - - public override async Task AccessFailedAsync(MembersIdentityUser user) - { - IdentityResult result = await base.AccessFailedAsync(user); - - // Slightly confusing: this will return a Success if we successfully update the AccessFailed count - if (result.Succeeded) - { - RaiseLoginFailedEvent(_httpContextAccessor.HttpContext?.User, user.Id); - } - - return result; - } - - public override async Task ChangePasswordWithResetAsync(string userId, string token, string newPassword) - { - IdentityResult result = await base.ChangePasswordWithResetAsync(userId, token, newPassword); - if (result.Succeeded) - { - RaisePasswordChangedEvent(_httpContextAccessor.HttpContext?.User, userId); - } - - return result; - } - - public override async Task ChangePasswordAsync(MembersIdentityUser user, string currentPassword, string newPassword) - { - IdentityResult result = await base.ChangePasswordAsync(user, currentPassword, newPassword); - if (result.Succeeded) - { - RaisePasswordChangedEvent(_httpContextAccessor.HttpContext?.User, user.Id); - } - - return result; - } - - /// - public override async Task SetLockoutEndDateAsync(MembersIdentityUser user, DateTimeOffset? lockoutEnd) - { - if (user == null) - { - throw new ArgumentNullException(nameof(user)); - } - - IdentityResult result = await base.SetLockoutEndDateAsync(user, lockoutEnd); - - // The way we unlock is by setting the lockoutEnd date to the current datetime - if (result.Succeeded && lockoutEnd >= DateTimeOffset.UtcNow) - { - RaiseAccountLockedEvent(_httpContextAccessor.HttpContext?.User, user.Id); - } - else - { - RaiseAccountUnlockedEvent(_httpContextAccessor.HttpContext?.User, user.Id); - - // Resets the login attempt fails back to 0 when unlock is clicked - await ResetAccessFailedCountAsync(user); - } - - return result; - } - - /// - public override async Task ResetAccessFailedCountAsync(MembersIdentityUser user) - { - IdentityResult result = await base.ResetAccessFailedCountAsync(user); - - // raise the event now that it's reset - RaiseResetAccessFailedCountEvent(_httpContextAccessor.HttpContext?.User, user.Id); - - return result; - } - private string GetCurrentUserId(IPrincipal currentUser) { UmbracoBackOfficeIdentity umbIdentity = currentUser?.GetUmbracoIdentity(); @@ -150,14 +55,8 @@ namespace Umbraco.Web.Common.Security return new IdentityAuditEventArgs(auditEvent, ip, currentUserId, string.Empty, affectedUserId, affectedUsername); } - private IdentityAuditEventArgs CreateArgs(AuditEvent auditEvent, BackOfficeIdentityUser currentUser, string affectedUserId, string affectedUsername) - { - var currentUserId = currentUser.Id; - var ip = IpResolver.GetCurrentRequestIpAddress(); - return new IdentityAuditEventArgs(auditEvent, ip, currentUserId, string.Empty, affectedUserId, affectedUsername); - } - // TODO: Review where these are raised and see if they can be simplified and either done in the this usermanager or the signin manager, + // TODO: As per backoffice, review where these are raised and see if they can be simplified and either done in the this usermanager or the signin manager, // lastly we'll resort to the authentication controller but we should try to remove all instances of that occuring public void RaiseAccountLockedEvent(IPrincipal currentUser, string userId) => OnAccountLocked(CreateArgs(AuditEvent.AccountLocked, currentUser, userId, string.Empty)); @@ -181,10 +80,6 @@ namespace Umbraco.Web.Common.Security return args; } - public void RaisePasswordChangedEvent(IPrincipal currentUser, string userId) => OnPasswordChanged(CreateArgs(AuditEvent.LogoutSuccess, currentUser, userId, string.Empty)); - - public void RaiseResetAccessFailedCountEvent(IPrincipal currentUser, string userId) => OnResetAccessFailedCount(CreateArgs(AuditEvent.ResetAccessFailedCount, currentUser, userId, string.Empty)); - public UserInviteEventArgs RaiseSendingUserInvite(IPrincipal currentUser, UserInvite invite, IUser createdUser) { var currentUserId = GetCurrentUserId(currentUser); @@ -196,9 +91,7 @@ namespace Umbraco.Web.Common.Security public bool HasSendingUserInviteEventHandler => SendingUserInvite != null; - // TODO: These static events are problematic. Moving forward we don't want static events at all but we cannot - // have non-static events here because the user manager is a Scoped instance not a singleton - // so we'll have to deal with this a diff way i.e. refactoring how events are done entirely + // TODO: Comments re static events as per backofficeusermanager public static event EventHandler AccountLocked; public static event EventHandler AccountUnlocked; public static event EventHandler ForgotPasswordRequested;