diff --git a/src/Umbraco.Infrastructure/Security/MemberRolesUserStore.cs b/src/Umbraco.Infrastructure/Security/MemberRolesUserStore.cs new file mode 100644 index 0000000000..771005b09b --- /dev/null +++ b/src/Umbraco.Infrastructure/Security/MemberRolesUserStore.cs @@ -0,0 +1,57 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Security.Claims; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; +using Umbraco.Core.Scoping; +using Umbraco.Core.Services; + +namespace Umbraco.Infrastructure.Security +{ + /// + /// A custom user store that uses Umbraco member data + /// + public class MemberRolesUserStore : RoleStoreBase, string, IdentityUserRole, IdentityRoleClaim> + { + private readonly IMemberService _memberService; + private readonly IMemberGroupService _memberGroupService; + private readonly IScopeProvider _scopeProvider; + + public MemberRolesUserStore(IMemberService memberService, IMemberGroupService memberGroupService, IScopeProvider scopeProvider, IdentityErrorDescriber describer) + : base(describer) + { + _memberService = memberService ?? throw new ArgumentNullException(nameof(memberService)); + _memberGroupService = memberGroupService ?? throw new ArgumentNullException(nameof(memberGroupService)); + _scopeProvider = scopeProvider ?? throw new ArgumentNullException(nameof(scopeProvider)); + } + + /// + public override IQueryable> Roles { get; } + + /// + public override Task CreateAsync(IdentityRole role, CancellationToken cancellationToken = new CancellationToken()) => throw new System.NotImplementedException(); + + /// + public override Task UpdateAsync(IdentityRole role, CancellationToken cancellationToken = new CancellationToken()) => throw new System.NotImplementedException(); + + /// + public override Task DeleteAsync(IdentityRole role, CancellationToken cancellationToken = new CancellationToken()) => throw new System.NotImplementedException(); + + /// + public override Task> FindByIdAsync(string id, CancellationToken cancellationToken = new CancellationToken()) => throw new System.NotImplementedException(); + + /// + public override Task> FindByNameAsync(string normalizedName, CancellationToken cancellationToken = new CancellationToken()) => throw new System.NotImplementedException(); + + /// + public override Task> GetClaimsAsync(IdentityRole role, CancellationToken cancellationToken = new CancellationToken()) => throw new System.NotImplementedException(); + + /// + public override Task AddClaimAsync(IdentityRole role, Claim claim, CancellationToken cancellationToken = new CancellationToken()) => throw new System.NotImplementedException(); + + /// + public override Task RemoveClaimAsync(IdentityRole role, Claim claim, CancellationToken cancellationToken = new CancellationToken()) => throw new System.NotImplementedException(); + } +} diff --git a/src/Umbraco.Infrastructure/Security/MembersUserStore.cs b/src/Umbraco.Infrastructure/Security/MembersUserStore.cs index ebcd340d7c..d7defd8da5 100644 --- a/src/Umbraco.Infrastructure/Security/MembersUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/MembersUserStore.cs @@ -32,13 +32,12 @@ namespace Umbraco.Infrastructure.Security /// The mapper for properties /// The scope provider /// The error describer - /// public MembersUserStore(IMemberService memberService, UmbracoMapper mapper, IScopeProvider scopeProvider, IdentityErrorDescriber describer) : base(describer) { _memberService = memberService ?? throw new ArgumentNullException(nameof(memberService)); - _mapper = mapper; - _scopeProvider = scopeProvider; + _mapper = mapper ?? throw new ArgumentNullException(nameof(mapper)); + _scopeProvider = scopeProvider ?? throw new ArgumentNullException(nameof(scopeProvider)); } /// @@ -55,7 +54,7 @@ namespace Umbraco.Infrastructure.Security public override Task SetNormalizedUserNameAsync(MembersIdentityUser user, string normalizedName, CancellationToken cancellationToken) => SetUserNameAsync(user, normalizedName, cancellationToken); /// - public override Task CreateAsync(MembersIdentityUser user, CancellationToken cancellationToken) + public override Task CreateAsync(MembersIdentityUser user, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); @@ -338,8 +337,10 @@ namespace Umbraco.Infrastructure.Security cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); + var logins = new List(); + // TODO: external login needed? - var logins = new List(); //_externalLoginService.Find(loginProvider, providerKey).ToList(); + //_externalLoginService.Find(loginProvider, providerKey).ToList(); if (logins.Count == 0) { return Task.FromResult((IdentityUserLogin)null); @@ -379,99 +380,13 @@ namespace Umbraco.Infrastructure.Security if (userRole == null) { + _memberService.AssignRole(user.UserName, role); user.AddRole(role); } return Task.CompletedTask; } - /// - /// Add the specified user to the named roles - /// - /// The user to add to the named roles - /// The name of the roles to add the user to. - /// The cancellation token - /// The Task that represents the asynchronous operation, containing the IdentityResult of the operation - public Task AddToRolesAsync(MembersIdentityUser user, IEnumerable roles, CancellationToken cancellationToken = default) - { - cancellationToken.ThrowIfCancellationRequested(); - ThrowIfDisposed(); - if (user == null) - { - throw new ArgumentNullException(nameof(user)); - } - - if (roles == null) - { - throw new ArgumentNullException(nameof(roles)); - } - - IEnumerable enumerable = roles as string[] ?? roles.ToArray(); - foreach (string role in enumerable) - { - if (string.IsNullOrWhiteSpace(role)) - { - throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(role)); - } - - IdentityUserRole userRole = user.Roles.SingleOrDefault(r => r.RoleId == role); - - if (userRole == null) - { - user.AddRole(role); - } - } - - _memberService.AssignRoles(new[] { user.UserName }, enumerable.ToArray()); - - return Task.CompletedTask; - } - - /// - /// Removes the specified user from the named roles. - /// - /// The user to remove from the named roles. - /// The name of the roles to remove the user from. - /// The cancellation token - /// The Task that represents the asynchronous operation, containing the IdentityResult of the operation. - public Task RemoveFromRolesAsync(MembersIdentityUser user, IEnumerable roles, CancellationToken cancellationToken = default) - { - cancellationToken.ThrowIfCancellationRequested(); - - if (user == null) - { - throw new ArgumentNullException(nameof(user)); - } - - if (roles == null) - { - throw new ArgumentNullException(nameof(roles)); - } - - IEnumerable enumerable = roles as string[] ?? roles.ToArray(); - foreach (string role in enumerable) - { - if (string.IsNullOrWhiteSpace(role)) - { - throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(role)); - } - - //TODO: is the role ID the role string passed in? - IdentityUserRole userRole = user.Roles.SingleOrDefault(r => r.RoleId == role); - - if (userRole != null) - { - user.Roles.Remove(userRole); - } - } - - //TODO: confirm that when updating the identity member, we're also calling the service to update in the DB via repository - _memberService.DissociateRoles(new[] { user.UserName }, enumerable.ToArray()); - - return Task.CompletedTask; - } - - //TODO: should we call the single remove from the multiple remove? or have it only in one place? /// public override Task RemoveFromRoleAsync(MembersIdentityUser user, string role, CancellationToken cancellationToken = default) { @@ -492,13 +407,11 @@ namespace Umbraco.Infrastructure.Security throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(role)); } - //TODO: is the role ID the role string passed in? IdentityUserRole userRole = user.Roles.SingleOrDefault(r => r.RoleId == role); if (userRole != null) { - //TODO: when updating the identity member, we're also calling the service to update in the DB via repository - _memberService.DissociateRole(userRole.UserId, userRole.RoleId); + _memberService.DissociateRole(user.UserName, userRole.RoleId); user.Roles.Remove(userRole); } @@ -517,8 +430,14 @@ namespace Umbraco.Infrastructure.Security throw new ArgumentNullException(nameof(user)); } - //TODO: should we have tests for the store? IEnumerable currentRoles = _memberService.GetAllRoles(user.UserName); + ICollection> roles = currentRoles.Select(role => new IdentityUserRole + { + RoleId = role, + UserId = user.Id + }).ToList(); + + user.Roles = roles; return Task.FromResult((IList)user.Roles.Select(x => x.RoleId).ToList()); } diff --git a/src/Umbraco.Tests.UnitTests/AutoFixture/AutoMoqDataAttribute.cs b/src/Umbraco.Tests.UnitTests/AutoFixture/AutoMoqDataAttribute.cs index 8097cbed92..ca95c73345 100644 --- a/src/Umbraco.Tests.UnitTests/AutoFixture/AutoMoqDataAttribute.cs +++ b/src/Umbraco.Tests.UnitTests/AutoFixture/AutoMoqDataAttribute.cs @@ -54,7 +54,8 @@ namespace Umbraco.Tests.UnitTests.AutoFixture .Customize(new ConstructorCustomization(typeof(PreviewController), new GreedyConstructorQuery())) .Customize(new ConstructorCustomization(typeof(MemberController), new GreedyConstructorQuery())) .Customize(new ConstructorCustomization(typeof(BackOfficeController), new GreedyConstructorQuery())) - .Customize(new ConstructorCustomization(typeof(BackOfficeUserManager), new GreedyConstructorQuery())); + .Customize(new ConstructorCustomization(typeof(BackOfficeUserManager), new GreedyConstructorQuery())) + .Customize(new ConstructorCustomization(typeof(MembersUserManager), new GreedyConstructorQuery())); fixture.Customize(new AutoMoqCustomization()); diff --git a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs index 8208ed9789..25f27f571f 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs @@ -527,7 +527,6 @@ namespace Umbraco.Web.BackOffice.Controllers // 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 = await _memberManager.GetRolesAsync(identityMember); - //IEnumerable currentRoles = _memberService.GetAllRoles(contentItem.PersistedContent.Username); // find the ones to remove and remove them IEnumerable roles = currentRoles.ToList(); @@ -538,7 +537,6 @@ namespace Umbraco.Web.BackOffice.Controllers if (rolesToRemove.Any()) { await _memberManager.RemoveFromRolesAsync(identityMember, rolesToRemove); - //_memberService.DissociateRoles(new[] { contentItem.PersistedContent.Username }, rolesToRemove); } // find the ones to add and add them diff --git a/src/Umbraco.Web.Common/Security/MembersUserManager.cs b/src/Umbraco.Web.Common/Security/MembersUserManager.cs index f1d6c963a9..51206bce61 100644 --- a/src/Umbraco.Web.Common/Security/MembersUserManager.cs +++ b/src/Umbraco.Web.Common/Security/MembersUserManager.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Security.Principal; -using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; diff --git a/src/Umbraco.Web/Security/MembershipHelper.cs b/src/Umbraco.Web/Security/MembershipHelper.cs index 5cee61834e..639b9c396c 100644 --- a/src/Umbraco.Web/Security/MembershipHelper.cs +++ b/src/Umbraco.Web/Security/MembershipHelper.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; @@ -11,7 +11,6 @@ using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Models.Security; using Umbraco.Core.Services; using Umbraco.Core.Strings; -using Umbraco.Web.Editors; using Umbraco.Web.Models; using Umbraco.Web.PublishedCache; using Umbraco.Web.Security.Providers; @@ -19,6 +18,15 @@ using Umbraco.Web.Security.Providers; namespace Umbraco.Web.Security { // MIGRATED TO NETCORE + // TODO: Analyse all - much can be moved/removed since most methods will occur on the manager via identity implementation + + /// + /// Helper class containing logic relating to the built-in Umbraco members macros and controllers for: + /// - Registration + /// - Updating + /// - Logging in + /// - Current status + /// public class MembershipHelper { private readonly MembersMembershipProvider _membershipProvider; @@ -118,7 +126,7 @@ namespace Umbraco.Web.Security var pathsWithAccess = HasAccess(pathsWithProtection, Roles.Provider); var result = new Dictionary(); - foreach(var path in paths) + foreach (var path in paths) { pathsWithAccess.TryGetValue(path, out var hasAccess); // if it's not found it's false anyways @@ -144,7 +152,8 @@ namespace Umbraco.Web.Security string[] userRoles = null; string[] getUserRoles(string username) { - if (userRoles != null) return userRoles; + if (userRoles != null) + return userRoles; userRoles = roleProvider.GetRolesForUser(username).ToArray(); return userRoles; } @@ -185,7 +194,8 @@ namespace Umbraco.Web.Security var provider = _membershipProvider; var membershipUser = provider.GetCurrentUser(); //NOTE: This should never happen since they are logged in - if (membershipUser == null) throw new InvalidOperationException("Could not find member with username " + _httpContextAccessor.GetRequiredHttpContext().User.Identity.Name); + if (membershipUser == null) + throw new InvalidOperationException("Could not find member with username " + _httpContextAccessor.GetRequiredHttpContext().User.Identity.Name); try { @@ -257,7 +267,8 @@ namespace Umbraco.Web.Security null, null, true, null, out status); - if (status != MembershipCreateStatus.Success) return null; + if (status != MembershipCreateStatus.Success) + return null; var member = _memberService.GetByUsername(membershipUser.UserName); member.Name = model.Name; @@ -367,7 +378,8 @@ namespace Umbraco.Web.Security public virtual IPublishedContent Get(Udi udi) { var guidUdi = udi as GuidUdi; - if (guidUdi == null) return null; + if (guidUdi == null) + return null; var umbracoType = UdiEntityTypeHelper.ToUmbracoObjectType(udi.EntityType); @@ -702,27 +714,32 @@ namespace Umbraco.Web.Security if (email != null) { - if (member.Email != email) update = true; + if (member.Email != email) + update = true; member.Email = email; } if (isApproved.HasValue) { - if (member.IsApproved != isApproved.Value) update = true; + if (member.IsApproved != isApproved.Value) + update = true; member.IsApproved = isApproved.Value; } if (lastLoginDate.HasValue) { - if (member.LastLoginDate != lastLoginDate.Value) update = true; + if (member.LastLoginDate != lastLoginDate.Value) + update = true; member.LastLoginDate = lastLoginDate.Value; } if (lastActivityDate.HasValue) { - if (member.LastActivityDate != lastActivityDate.Value) update = true; + if (member.LastActivityDate != lastActivityDate.Value) + update = true; member.LastActivityDate = lastActivityDate.Value; } if (comment != null) { - if (member.Comment != comment) update = true; + if (member.Comment != comment) + update = true; member.Comment = comment; } @@ -741,8 +758,8 @@ namespace Umbraco.Web.Security { var provider = _membershipProvider; - var username = provider.GetCurrentUserName(); - // The result of this is cached by the MemberRepository + var username = provider.GetCurrentUserName(); + // The result of this is cached by the MemberRepository var member = _memberService.GetByUsername(username); return member; } @@ -763,8 +780,10 @@ namespace Umbraco.Web.Security // YES! It is completely insane how many options you have to take into account based on the membership provider. yikes! - if (passwordModel == null) throw new ArgumentNullException(nameof(passwordModel)); - if (membershipProvider == null) throw new ArgumentNullException(nameof(membershipProvider)); + if (passwordModel == null) + throw new ArgumentNullException(nameof(passwordModel)); + if (membershipProvider == null) + throw new ArgumentNullException(nameof(membershipProvider)); var userId = -1; diff --git a/src/Umbraco.Web/Security/MembershipProviderBase.cs b/src/Umbraco.Web/Security/MembershipProviderBase.cs index 669b105775..29f694803b 100644 --- a/src/Umbraco.Web/Security/MembershipProviderBase.cs +++ b/src/Umbraco.Web/Security/MembershipProviderBase.cs @@ -1,25 +1,22 @@ -using System; +using System; using System.Collections.Specialized; using System.ComponentModel.DataAnnotations; using System.Configuration.Provider; using System.Text; using System.Text.RegularExpressions; -using System.Web; -using System.Web.Hosting; -using System.Web.Configuration; using System.Web.Security; using Microsoft.Extensions.Logging; using Umbraco.Core; using Umbraco.Web.Composing; using Umbraco.Core.Hosting; -using Umbraco.Core.Security; namespace Umbraco.Web.Security { + //TODO: Delete - should not be used /// /// A base membership provider class offering much of the underlying functionality for initializing and password encryption/hashing. /// - [Obsolete("Will be replaced by UmbracoMemberUserManager")] + [Obsolete("We are now using ASP.NET Core Identity instead of membership providers")] public abstract class MembershipProviderBase : MembershipProvider { private readonly IHostingEnvironment _hostingEnvironment; diff --git a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs index fa1ddae980..92ea0c42f0 100644 --- a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs @@ -1,4 +1,4 @@ -using System.Collections.Specialized; +using System.Collections.Specialized; using System.Configuration.Provider; using System.Web.Security; using Umbraco.Core; @@ -7,13 +7,14 @@ using Umbraco.Core.Hosting; using Umbraco.Core.Models; using Umbraco.Core.Security; using Umbraco.Core.Services; -using Umbraco.Core.Models.Membership; using Umbraco.Web.Composing; using System; using Umbraco.Net; namespace Umbraco.Web.Security.Providers { + //TODO: Delete: should not be used + [Obsolete("We are now using ASP.NET Core Identity instead of membership providers")] /// /// Custom Membership Provider for Umbraco Members (User authentication for Frontend applications NOT umbraco CMS) /// @@ -120,6 +121,7 @@ namespace Umbraco.Web.Security.Providers public override LegacyPasswordSecurity PasswordSecurity => _passwordSecurity.Value; public IPasswordConfiguration PasswordConfiguration => _passwordConfig.Value; + [Obsolete("We are now using ASP.NET Core Identity instead of membership providers")] private class MembershipProviderPasswordConfiguration : IPasswordConfiguration { public MembershipProviderPasswordConfiguration(int requiredLength, bool requireNonLetterOrDigit, bool requireDigit, bool requireLowercase, bool requireUppercase, bool useLegacyEncoding, string hashAlgorithmType, int maxFailedAccessAttemptsBeforeLockout) diff --git a/src/Umbraco.Web/Security/Providers/MembersRoleProvider.cs b/src/Umbraco.Web/Security/Providers/MembersRoleProvider.cs index 0637b08621..b1b4657d3e 100644 --- a/src/Umbraco.Web/Security/Providers/MembersRoleProvider.cs +++ b/src/Umbraco.Web/Security/Providers/MembersRoleProvider.cs @@ -1,3 +1,4 @@ +using System; using System.Configuration.Provider; using System.Linq; using System.Web.Security; @@ -8,6 +9,8 @@ using Umbraco.Web.Composing; namespace Umbraco.Web.Security.Providers { + //TODO: Delete: should not be used + [Obsolete("We are now using ASP.NET Core Identity instead of membership providers")] public class MembersRoleProvider : RoleProvider { private readonly IMembershipRoleService _roleService; diff --git a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs index 8a92226d7e..51f31b577b 100644 --- a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Specialized; using System.Configuration.Provider; using System.Linq; @@ -17,8 +17,8 @@ using Umbraco.Web.Composing; namespace Umbraco.Web.Security.Providers { - - + //TODO: Delete - should not be used + [Obsolete("We are now using ASP.NET Core Identity instead of membership providers")] /// /// Abstract Membership Provider that users any implementation of IMembershipMemberService{TEntity} service /// @@ -32,7 +32,7 @@ namespace Umbraco.Web.Security.Providers protected IMembershipMemberService MemberService { get; private set; } protected UmbracoMembershipProvider(IMembershipMemberService memberService, IUmbracoVersion umbracoVersion, IHostingEnvironment hostingEnvironment, IIpResolver ipResolver) - :base(hostingEnvironment) + : base(hostingEnvironment) { _umbracoVersion = umbracoVersion; _ipResolver = ipResolver; @@ -55,9 +55,11 @@ namespace Umbraco.Web.Security.Providers /// The name of the provider has a length of zero. public override void Initialize(string name, NameValueCollection config) { - if (config == null) { throw new ArgumentNullException("config"); } + if (config == null) + { throw new ArgumentNullException("config"); } - if (string.IsNullOrEmpty(name)) name = ProviderName; + if (string.IsNullOrEmpty(name)) + name = ProviderName; // Initialize base provider class base.Initialize(name, config); @@ -80,7 +82,8 @@ namespace Umbraco.Web.Security.Providers // in order to support updating passwords from the umbraco core, we can't validate the old password var m = MemberService.GetByUsername(username); - if (m == null) return false; + if (m == null) + return false; string salt; var encodedPassword = PasswordSecurity.HashNewPassword(Membership.HashAlgorithmType, newPassword, out salt); @@ -174,7 +177,8 @@ namespace Umbraco.Web.Security.Providers public override bool DeleteUser(string username, bool deleteAllRelatedData) { var member = MemberService.GetByUsername(username); - if (member == null) return false; + if (member == null) + return false; MemberService.Delete(member); return true; @@ -423,7 +427,8 @@ namespace Umbraco.Web.Security.Providers } // Non need to update - if (member.IsLockedOut == false) return true; + if (member.IsLockedOut == false) + return true; member.IsLockedOut = false; member.FailedPasswordAttempts = 0;