From 8572d7bbd9003b56290dd58ad7c18e6a6ff808e2 Mon Sep 17 00:00:00 2001 From: emmagarland Date: Sun, 29 Nov 2020 13:56:58 +0000 Subject: [PATCH] Updated member type, and added start of unit tests for new store. Removed all unused properties for now to implement later. --- src/Umbraco.Core/Constants-Security.cs | 3 + .../Members/UmbracoMembersIdentityUser.cs | 92 +---- .../Members/UmbracoMembersUserStore.cs | 386 ++++++++++-------- .../UmbracoMemberIdentityUserStoreTests.cs | 75 ++++ 4 files changed, 323 insertions(+), 233 deletions(-) create mode 100644 src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserStoreTests.cs diff --git a/src/Umbraco.Core/Constants-Security.cs b/src/Umbraco.Core/Constants-Security.cs index 24b8b20731..42bace2938 100644 --- a/src/Umbraco.Core/Constants-Security.cs +++ b/src/Umbraco.Core/Constants-Security.cs @@ -40,6 +40,9 @@ public const string EmptyPasswordPrefix = "___UIDEMPTYPWORD__"; public const string ForceReAuthFlag = "umbraco-force-auth"; + public const string DefaultMemberTypeAlias = "Member"; + + /// /// The prefix used for external identity providers for their authentication type /// diff --git a/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs b/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs index 531460794a..c561767024 100644 --- a/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs +++ b/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs @@ -1,7 +1,4 @@ using System; -using System.Collections.Generic; -using System.ComponentModel; -using System.Text; using Umbraco.Core.Models.Entities; using Umbraco.Core.Models.Identity; @@ -10,77 +7,28 @@ namespace Umbraco.Core.Members /// /// An Umbraco member user type /// - public class UmbracoMembersIdentityUser : IdentityUser, IdentityUserClaim>, IRememberBeingDirty + public class UmbracoMembersIdentityUser + //: IRememberBeingDirty + //TODO: use of identity classes + //: IdentityUser, IdentityUserClaim>, { - private readonly BeingDirty _beingDirty = new BeingDirty(); + public int Id; + public string Name; + public string Email; + public string UserName; + public string MemberTypeAlias; + public bool IsLockedOut; - #region BeingDirty - public event PropertyChangedEventHandler PropertyChanged - { - add - { - _beingDirty.PropertyChanged += value; - } - remove - { - _beingDirty.PropertyChanged -= value; - } - } + string Comment; + bool IsApproved; + DateTime LastLockoutDate; + DateTime CreationDate; + DateTime LastLoginDate; + DateTime LastActivityDate; + DateTime LastPasswordChangedDate; - public void DisableChangeTracking() - { - throw new NotImplementedException(); - } - - public void EnableChangeTracking() - { - throw new NotImplementedException(); - } - - public IEnumerable GetDirtyProperties() - { - throw new NotImplementedException(); - } - - public IEnumerable GetWereDirtyProperties() - { - throw new NotImplementedException(); - } - - public bool IsDirty() - { - throw new NotImplementedException(); - } - - public bool IsPropertyDirty(string propName) - { - throw new NotImplementedException(); - } - - public void ResetDirtyProperties(bool rememberDirty) - { - throw new NotImplementedException(); - } - - public void ResetDirtyProperties() - { - throw new NotImplementedException(); - } - - public void ResetWereDirtyProperties() - { - throw new NotImplementedException(); - } - - public bool WasDirty() - { - throw new NotImplementedException(); - } - - public bool WasPropertyDirty(string propertyName) - { - throw new NotImplementedException(); - } + //TODO: needed? + //public bool LoginsChanged; + //public bool RolesChanged; } - #endregion } diff --git a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs b/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs index 82df3a64ec..10c85a9def 100644 --- a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs +++ b/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs @@ -1,11 +1,17 @@ using Microsoft.AspNetCore.Identity; using System; using System.Collections.Generic; +using System.Data; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Umbraco.Core; using Umbraco.Core.BackOffice; using Umbraco.Core.Members; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Identity; +using Umbraco.Core.Models.Membership; +using Umbraco.Core.Services; namespace Umbraco.Infrastructure.Members { @@ -13,121 +19,253 @@ namespace Umbraco.Infrastructure.Members /// A custom user store that uses Umbraco member data /// public class UmbracoMembersUserStore : DisposableObjectSlim, - IUserStore, - IUserPasswordStore, - IUserEmailStore, - IUserLoginStore, - IUserRoleStore, - IUserSecurityStampStore, - IUserLockoutStore, - IUserTwoFactorStore, - IUserSessionStore + IUserStore + //IUserPasswordStore + //IUserEmailStore + //IUserLoginStore + //IUserRoleStore, + //IUserSecurityStampStore + //IUserLockoutStore + //IUserTwoFactorStore + //IUserSessionStore { - public UmbracoMembersUserStore() - { + private bool _disposed = false; + private IMemberService _memberService; + public UmbracoMembersUserStore(IMemberService memberService) + { + _memberService = memberService; } - public Task AddLoginAsync(UmbracoMembersIdentityUser user, UserLoginInfo login, CancellationToken cancellationToken) + + public Task CreateAsync(UmbracoMembersIdentityUser memberUser, CancellationToken cancellationToken) { - throw new NotImplementedException(); + //TODO: cancellationToken.ThrowIfCancellationRequested(); + //TODO: ThrowIfDisposed(); + if (memberUser == null) throw new ArgumentNullException(nameof(memberUser)); + + + // [Comments from Identity package and BackOfficeUser] + // the password must be 'something' it could be empty if authenticating + // with an external provider so we'll just generate one and prefix it, the + // prefix will help us determine if the password hasn't actually been specified yet. + //this will hash the guid with a salt so should be nicely random + + var aspHasher = new PasswordHasher(); + var emptyPasswordValue = + Constants.Security.EmptyPasswordPrefix + + aspHasher.HashPassword(memberUser, Guid.NewGuid().ToString("N")); + + //create member + //TODO: are we keeping this method, e.g. the Member Service? + IMember memberEntity = _memberService.CreateMember( + memberUser.UserName, + memberUser.Email, + memberUser.Name.IsNullOrWhiteSpace() ? memberUser.UserName : memberUser.Name, + memberUser.MemberTypeAlias.IsNullOrWhiteSpace() ? + Constants.Security.DefaultMemberTypeAlias : memberUser.MemberTypeAlias); + + + UpdateMemberProperties(memberEntity, memberUser); + + _memberService.Save(memberEntity); + + //re-assign id + memberUser.Id = memberEntity.Id; + + // TODO: do we need this? + // TODO: [from backofficeuser] we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. + //bool isLoginsPropertyDirty = member.IsPropertyDirty(nameof(UmbracoMembersIdentityUser.Logins)); + + //if (isLoginsPropertyDirty) + //{ + // _externalLoginService.Save( + // user.Id, + // user.Logins.Select(x => new ExternalLogin( + // x.LoginProvider, + // x.ProviderKey, + // x.UserData))); + //} + + if (!memberEntity.HasIdentity) throw new DataException("Could not create the user, check logs for details"); + + return Task.FromResult(IdentityResult.Success); + + //TODO: confirm + //if (memberUser.LoginsChanged) + //{ + // var logins = await GetLoginsAsync(memberUser); + // _externalLoginStore.SaveUserLogins(member.Id, logins); + //} + + //TODO: confirm + //if (memberUser.RolesChanged) + //{ + //IMembershipRoleService memberRoleService = _memberService; + + //var persistedRoles = memberRoleService.GetAllRoles(member.Id).ToArray(); + //var userRoles = memberUser.Roles.Select(x => x.RoleName).ToArray(); + + //var keep = persistedRoles.Intersect(userRoles).ToArray(); + //var remove = persistedRoles.Except(keep).ToArray(); + //var add = userRoles.Except(persistedRoles).ToArray(); + + //memberRoleService.DissociateRoles(new[] { member.Id }, remove); + //memberRoleService.AssignRoles(new[] { member.Id }, add); + //} } - public Task AddToRoleAsync(UmbracoMembersIdentityUser user, string roleName, CancellationToken cancellationToken) + + private bool UpdateMemberProperties(IMember member, UmbracoMembersIdentityUser memberIdentityUser) { - throw new NotImplementedException(); + //[Comments as per BackOfficeUserStore & identity package] + var anythingChanged = false; + //don't assign anything if nothing has changed as this will trigger the track changes of the model + + //if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.LastLoginDateUtc)) + // || (member.LastLoginDate != default(DateTime) && identityUser.LastLoginDateUtc.HasValue == false) + // || identityUser.LastLoginDateUtc.HasValue && member.LastLoginDate.ToUniversalTime() != identityUser.LastLoginDateUtc.Value) + //{ + // anythingChanged = true; + // //if the LastLoginDate is being set to MinValue, don't convert it ToLocalTime + // var dt = identityUser.LastLoginDateUtc == DateTime.MinValue ? DateTime.MinValue : identityUser.LastLoginDateUtc.Value.ToLocalTime(); + // member.LastLoginDate = dt; + //} + + //if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.LastPasswordChangeDateUtc)) + // || (member.LastPasswordChangeDate != default(DateTime) && identityUser.LastPasswordChangeDateUtc.HasValue == false) + // || identityUser.LastPasswordChangeDateUtc.HasValue && member.LastPasswordChangeDate.ToUniversalTime() != identityUser.LastPasswordChangeDateUtc.Value) + //{ + // anythingChanged = true; + // member.LastPasswordChangeDate = identityUser.LastPasswordChangeDateUtc.Value.ToLocalTime(); + //} + + //if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.EmailConfirmed)) + // || (member.EmailConfirmedDate.HasValue && member.EmailConfirmedDate.Value != default(DateTime) && identityUser.EmailConfirmed == false) + // || ((member.EmailConfirmedDate.HasValue == false || member.EmailConfirmedDate.Value == default(DateTime)) && identityUser.EmailConfirmed)) + //{ + // anythingChanged = true; + // member.EmailConfirmedDate = identityUser.EmailConfirmed ? (DateTime?)DateTime.Now : null; + //} + + if ( + //memberIdentityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Name)) && + member.Name != memberIdentityUser.Name && memberIdentityUser.Name.IsNullOrWhiteSpace() == false) + { + anythingChanged = true; + member.Name = memberIdentityUser.Name; + } + if ( + //memberIdentityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Email)) && + member.Email != memberIdentityUser.Email && memberIdentityUser.Email.IsNullOrWhiteSpace() == false) + { + anythingChanged = true; + member.Email = memberIdentityUser.Email; + } + + //TODO: AccessFailedCount + //if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.AccessFailedCount)) + // && member.FailedPasswordAttempts != identityUser.AccessFailedCount) + //{ + // anythingChanged = true; + // member.FailedPasswordAttempts = identityUser.AccessFailedCount; + //} + + if (member.IsLockedOut != memberIdentityUser.IsLockedOut) + { + anythingChanged = true; + member.IsLockedOut = memberIdentityUser.IsLockedOut; + + if (member.IsLockedOut) + { + //need to set the last lockout date + member.LastLockoutDate = DateTime.Now; + } + } + if ( + //memberIdentityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.UserName)) && + member.Username != memberIdentityUser.UserName && memberIdentityUser.UserName.IsNullOrWhiteSpace() == false) + { + anythingChanged = true; + member.Username = memberIdentityUser.UserName; + } + + //TODO: PasswordHash and PasswordConfig + //if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.PasswordHash)) + // && member.RawPasswordValue != identityUser.PasswordHash && identityUser.PasswordHash.IsNullOrWhiteSpace() == false) + //{ + // anythingChanged = true; + // member.RawPasswordValue = identityUser.PasswordHash; + // member.PasswordConfiguration = identityUser.PasswordConfig; + //} + + //TODO: SecurityStamp + //if (member.SecurityStamp != identityUser.SecurityStamp) + //{ + // anythingChanged = true; + // member.SecurityStamp = identityUser.SecurityStamp; + //} + + // TODO: Roles + // [Comment] Same comment as per BackOfficeUserStore: Fix this for Groups too + //if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Roles)) || identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Groups))) + //{ + // var userGroupAliases = member.Groups.Select(x => x.Alias).ToArray(); + + // var identityUserRoles = identityUser.Roles.Select(x => x.RoleId).ToArray(); + // var identityUserGroups = identityUser.Groups.Select(x => x.Alias).ToArray(); + + // var combinedAliases = identityUserRoles.Union(identityUserGroups).ToArray(); + + // if (userGroupAliases.ContainsAll(combinedAliases) == false + // || combinedAliases.ContainsAll(userGroupAliases) == false) + // { + // anythingChanged = true; + + // //clear out the current groups (need to ToArray since we are modifying the iterator) + // member.ClearGroups(); + + // //go lookup all these groups + // var groups = _userService.GetUserGroupsByAlias(combinedAliases).Select(x => x.ToReadOnlyGroup()).ToArray(); + + // //use all of the ones assigned and add them + // foreach (var group in groups) + // { + // member.AddGroup(group); + // } + + // //re-assign + // identityUser.Groups = groups; + // } + //} + + //TODO: reset all changes + //memberIdentityUser.ResetDirtyProperties(false); + + return anythingChanged; } - public Task CreateAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } public Task DeleteAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) { throw new NotImplementedException(); } - public Task FindByEmailAsync(string normalizedEmail, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - public Task FindByIdAsync(string userId, CancellationToken cancellationToken) { throw new NotImplementedException(); } - public Task FindByLoginAsync(string loginProvider, string providerKey, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - public Task FindByNameAsync(string normalizedUserName, CancellationToken cancellationToken) { throw new NotImplementedException(); } - public Task GetAccessFailedCountAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task GetEmailAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task GetEmailConfirmedAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task GetLockoutEnabledAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task GetLockoutEndDateAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task> GetLoginsAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task GetNormalizedEmailAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - public Task GetNormalizedUserNameAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) { throw new NotImplementedException(); } - public Task GetPasswordHashAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task> GetRolesAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task GetSecurityStampAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task GetTwoFactorEnabledAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - public Task GetUserIdAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) { throw new NotImplementedException(); @@ -138,86 +276,11 @@ namespace Umbraco.Infrastructure.Members throw new NotImplementedException(); } - public Task> GetUsersInRoleAsync(string roleName, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task HasPasswordAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task IncrementAccessFailedCountAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task IsInRoleAsync(UmbracoMembersIdentityUser user, string roleName, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task RemoveFromRoleAsync(UmbracoMembersIdentityUser user, string roleName, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task RemoveLoginAsync(UmbracoMembersIdentityUser user, string loginProvider, string providerKey, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task ResetAccessFailedCountAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task SetEmailAsync(UmbracoMembersIdentityUser user, string email, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task SetEmailConfirmedAsync(UmbracoMembersIdentityUser user, bool confirmed, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task SetLockoutEnabledAsync(UmbracoMembersIdentityUser user, bool enabled, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task SetLockoutEndDateAsync(UmbracoMembersIdentityUser user, DateTimeOffset? lockoutEnd, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task SetNormalizedEmailAsync(UmbracoMembersIdentityUser user, string normalizedEmail, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - public Task SetNormalizedUserNameAsync(UmbracoMembersIdentityUser user, string normalizedName, CancellationToken cancellationToken) { throw new NotImplementedException(); } - public Task SetPasswordHashAsync(UmbracoMembersIdentityUser user, string passwordHash, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task SetSecurityStampAsync(UmbracoMembersIdentityUser user, string stamp, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public Task SetTwoFactorEnabledAsync(UmbracoMembersIdentityUser user, bool enabled, CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - public Task SetUserNameAsync(UmbracoMembersIdentityUser user, string userName, CancellationToken cancellationToken) { throw new NotImplementedException(); @@ -228,9 +291,10 @@ namespace Umbraco.Infrastructure.Members throw new NotImplementedException(); } - public Task ValidateSessionIdAsync(string userId, string sessionId) + private void ThrowIfDisposed() { - throw new NotImplementedException(); + if (_disposed) + throw new ObjectDisposedException(GetType().Name); } } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserStoreTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserStoreTests.cs new file mode 100644 index 0000000000..335c5ef213 --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserStoreTests.cs @@ -0,0 +1,75 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; +using Moq; +using NUnit.Framework; +using Umbraco.Core.Members; +using Umbraco.Core.Models; +using Umbraco.Core.Services; +using Umbraco.Infrastructure.Members; +using Umbraco.Tests.UnitTests.Umbraco.Core.ShortStringHelper; + +namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Members +{ + [TestFixture] + public class UmbracoMemberIdentityUserStoreTests + { + private Mock _mockMemberService; + + public UmbracoMembersUserStore CreateSut() + { + _mockMemberService = new Mock(); + return new UmbracoMembersUserStore(_mockMemberService.Object); + } + + [Test] + public void GivenICreateUser_AndTheUserIsNull_ThenIShouldGetAFailedResultAsync() + { + //arrange + UmbracoMembersUserStore sut = CreateSut(); + CancellationToken fakeCancellationToken = new CancellationToken(){}; + + //act + Action actual = () => sut.CreateAsync(null, fakeCancellationToken); + + //assert + Assert.That(actual, Throws.ArgumentNullException); + } + + + [Test] + public async Task GivenICreateUser_AndTheUserIsPopulatedCorrectly_ThenIShouldGetASuccessResult() + { + //arrange + UmbracoMembersUserStore sut = CreateSut(); + UmbracoMembersIdentityUser fakeUser = new UmbracoMembersIdentityUser() + { + + }; + + CancellationToken fakeCancellationToken = new CancellationToken() { }; + + IMemberType fakeMemberType = new MemberType(new MockShortStringHelper(), 77); + + var mockMember = Mock.Of(m => + m.Name == "fakeName" && + m.Email == "fakeemail@umbraco.com" && + m.Username == "fakeUsername" && + m.RawPasswordValue == "fakePassword" && + m.ContentTypeAlias == fakeMemberType.Alias && + m.HasIdentity == true); + + bool raiseEvents = false; + + _mockMemberService.Setup(x => x.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).Returns(mockMember); + _mockMemberService.Setup(x => x.Save(mockMember, raiseEvents)); + + //act + IdentityResult identityResult = await sut.CreateAsync(fakeUser, fakeCancellationToken); + + //assert + Assert.IsTrue(identityResult.Succeeded); + } + } +}