diff --git a/src/Umbraco.Core/Models/UmbracoProperty.cs b/src/Umbraco.Core/Models/MemberPropertyModel.cs similarity index 91% rename from src/Umbraco.Core/Models/UmbracoProperty.cs rename to src/Umbraco.Core/Models/MemberPropertyModel.cs index bffcd43fb3..780ad9ca99 100644 --- a/src/Umbraco.Core/Models/UmbracoProperty.cs +++ b/src/Umbraco.Core/Models/MemberPropertyModel.cs @@ -1,12 +1,12 @@ -using System.ComponentModel; +using System.ComponentModel; using System.ComponentModel.DataAnnotations; namespace Umbraco.Cms.Core.Models { /// - /// A simple representation of an Umbraco property + /// A simple representation of an Umbraco member property /// - public class UmbracoProperty + public class MemberPropertyModel { [Editable(false)] public string Alias { get; set; } diff --git a/src/Umbraco.Core/Models/Security/LoginModel.cs b/src/Umbraco.Core/Models/Security/LoginModel.cs deleted file mode 100644 index 3a639603eb..0000000000 --- a/src/Umbraco.Core/Models/Security/LoginModel.cs +++ /dev/null @@ -1,18 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using System.Runtime.Serialization; - -namespace Umbraco.Cms.Core.Models.Security -{ - public class LoginModel : PostRedirectModel - { - [Required] - [DataMember(Name = "username", IsRequired = true)] - public string Username { get; set; } - - [Required] - [DataMember(Name = "password", IsRequired = true)] - [StringLength(maximumLength: 256)] - public string Password { get; set; } - - } -} diff --git a/src/Umbraco.Core/Models/Security/LoginStatusModel.cs b/src/Umbraco.Core/Models/Security/LoginStatusModel.cs deleted file mode 100644 index 040c72187d..0000000000 --- a/src/Umbraco.Core/Models/Security/LoginStatusModel.cs +++ /dev/null @@ -1,41 +0,0 @@ -using System.ComponentModel.DataAnnotations; - -namespace Umbraco.Cms.Core.Models.Security -{ - /// - /// The model representing the status of a logged in member. - /// - public class LoginStatusModel - { - /// - /// Creates a new empty LoginStatusModel. - /// - /// - public static LoginStatusModel CreateModel() - { - return new LoginStatusModel(); - } - - /// - /// The name of the member - /// - [Required] - public string Name { get; set; } - - /// - /// The username of the member - /// - public string Username { get; set; } - - /// - /// The email of the member - /// - [Required] - public string Email { get; set; } - - /// - /// True, if the member is currently logged in - /// - public bool IsLoggedIn { get; set; } - } -} diff --git a/src/Umbraco.Core/PublishedCache/PublishedMember.cs b/src/Umbraco.Core/PublishedCache/PublishedMember.cs index f5926cf639..3fb0c80f6a 100644 --- a/src/Umbraco.Core/PublishedCache/PublishedMember.cs +++ b/src/Umbraco.Core/PublishedCache/PublishedMember.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Umbraco.Cms.Core.Models; @@ -67,8 +67,6 @@ namespace Umbraco.Cms.Core.PublishedCache public DateTime LastLoginDate => _membershipUser.LastLoginDate; - public DateTime LastActivityDate => _membershipUser.LastLoginDate; - public DateTime LastPasswordChangeDate => _membershipUser.LastPasswordChangeDate; #endregion diff --git a/src/Umbraco.Core/Security/IUmbracoWebsiteSecurity.cs b/src/Umbraco.Core/Security/IUmbracoWebsiteSecurity.cs deleted file mode 100644 index 8cdb1d61c7..0000000000 --- a/src/Umbraco.Core/Security/IUmbracoWebsiteSecurity.cs +++ /dev/null @@ -1,58 +0,0 @@ -using System.Collections.Generic; -using System.Threading.Tasks; -using Umbraco.Cms.Core.Models.Security; - -namespace Umbraco.Cms.Core.Security -{ - // TODO: I think we can kill this whole thing, the logic can just be in the controllers - public interface IUmbracoWebsiteSecurity - { - /// - /// Creates a model to use for registering new members with custom member properties - /// - /// Alias of member type for created member (default used if not provided). - /// Instance of - RegisterModel CreateRegistrationModel(string memberTypeAlias = null); - - /// - /// Creates a new profile model filled in with the current members details if they are logged in which allows for editing - /// profile properties. - /// - /// Instance of - Task GetCurrentMemberProfileModelAsync(); - - /// - /// Updates the currently logged in member's profile. - /// - /// Update member profile model. - /// Result of update profile operation. - Task UpdateMemberProfileAsync(ProfileModel model); - - // TODO: Kill this, we will just use the MemberManager / MemberSignInManager - Task LoginAsync(string username, string password); - - // TODO: Kill this, we will just use the MemberManager - bool IsLoggedIn(); - - /// - /// Returns the login status model of the currently logged in member. - /// - /// Instance of - Task GetCurrentLoginStatusAsync(); - - // TODO: Kill this, we will just use the MemberManager - Task LogOutAsync(); - - /// - /// Checks if the current member is authorized based on the parameters provided. - /// - /// Allowed types. - /// Allowed groups. - /// Allowed individual members. - /// True or false if the currently logged in member is authorized - bool IsMemberAuthorized( - IEnumerable allowTypes = null, - IEnumerable allowGroups = null, - IEnumerable allowMembers = null); - } -} diff --git a/src/Umbraco.Core/Security/IUmbracoWebsiteSecurityAccessor.cs b/src/Umbraco.Core/Security/IUmbracoWebsiteSecurityAccessor.cs deleted file mode 100644 index 3d20a7656f..0000000000 --- a/src/Umbraco.Core/Security/IUmbracoWebsiteSecurityAccessor.cs +++ /dev/null @@ -1,7 +0,0 @@ -namespace Umbraco.Cms.Core.Security -{ - public interface IUmbracoWebsiteSecurityAccessor - { - IUmbracoWebsiteSecurity WebsiteSecurity { get; } - } -} diff --git a/src/Umbraco.Infrastructure/Security/BackOfficeIdentityBuilder.cs b/src/Umbraco.Infrastructure/Security/BackOfficeIdentityBuilder.cs index e6b214ea8a..5287316e36 100644 --- a/src/Umbraco.Infrastructure/Security/BackOfficeIdentityBuilder.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficeIdentityBuilder.cs @@ -2,9 +2,13 @@ using System; using System.Reflection; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Umbraco.Cms.Core.Net; +using Umbraco.Cms.Core.Serialization; namespace Umbraco.Cms.Core.Security { + public class BackOfficeIdentityBuilder : IdentityBuilder { /// @@ -12,17 +16,49 @@ namespace Umbraco.Cms.Core.Security /// public BackOfficeIdentityBuilder(IServiceCollection services) : base(typeof(BackOfficeIdentityUser), services) - { - } + => InitializeServices(services); /// /// Initializes a new instance of the class. /// public BackOfficeIdentityBuilder(Type role, IServiceCollection services) : base(typeof(BackOfficeIdentityUser), role, services) + => InitializeServices(services); + + private void InitializeServices(IServiceCollection services) { + // We need to manually register some identity services here because we cannot rely on normal + // AddIdentity calls for back office users + // For example: https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Extensions.Core/src/IdentityServiceCollectionExtensions.cs#L33 + // The reason we need our own is because the Identity system doesn't cater easily for multiple identity systems and particularly being + // able to configure IdentityOptions to a specific provider since there is no named options. So we have strongly typed options + // and strongly typed ILookupNormalizer and IdentityErrorDescriber since those are 'global' and we need to be unintrusive. + + // Services used by identity + services.TryAddScoped, UserValidator>(); + services.TryAddScoped, PasswordValidator>(); + services.TryAddScoped>( + services => new BackOfficePasswordHasher( + new LegacyPasswordSecurity(), + services.GetRequiredService())); + services.TryAddScoped, DefaultUserConfirmation>(); } + public override IdentityBuilder AddErrorDescriber() + { + if (!typeof(BackOfficeIdentityErrorDescriber).IsAssignableFrom(typeof(TDescriber))) + { + throw new InvalidOperationException($"The type {typeof(TDescriber)} does not inherit from {typeof(BackOfficeIdentityErrorDescriber)}"); + } + + // Add as itself, by default identity only wants a single IdentityErrorDescriber + Services.AddScoped(); + return this; + } + + //public override IdentityBuilder AddClaimsPrincipalFactory() where TFactory : class + // => AddScoped(typeof(IUserClaimsPrincipalFactory<>).MakeGenericType(UserType), typeof(TFactory)); + /// /// Adds a token provider for the . /// @@ -40,5 +76,11 @@ namespace Umbraco.Cms.Core.Security Services.AddTransient(provider); return this; } + + private IdentityBuilder AddScoped(Type serviceType, Type concreteType) + { + Services.AddScoped(serviceType, concreteType); + return this; + } } } diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficePasswordHasher.cs b/src/Umbraco.Infrastructure/Security/BackOfficePasswordHasher.cs similarity index 97% rename from src/Umbraco.Web.BackOffice/Security/BackOfficePasswordHasher.cs rename to src/Umbraco.Infrastructure/Security/BackOfficePasswordHasher.cs index 2d2eb548a6..2132752866 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackOfficePasswordHasher.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficePasswordHasher.cs @@ -6,7 +6,7 @@ using Umbraco.Cms.Core.Serialization; using Umbraco.Extensions; using Constants = Umbraco.Cms.Core.Constants; -namespace Umbraco.Cms.Web.BackOffice.Security +namespace Umbraco.Cms.Core.Security { /// /// A password hasher for back office users @@ -72,7 +72,8 @@ namespace Umbraco.Cms.Web.BackOffice.Security return base.VerifyHashedPassword(user, hashedPassword, providedPassword); case Constants.Security.AspNetCoreV2PasswordHashAlgorithmName: var legacyResult = _aspnetV2PasswordHasher.VerifyHashedPassword(user, hashedPassword, providedPassword); - if (legacyResult == PasswordVerificationResult.Success) return PasswordVerificationResult.SuccessRehashNeeded; + if (legacyResult == PasswordVerificationResult.Success) + return PasswordVerificationResult.SuccessRehashNeeded; return legacyResult; } } diff --git a/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs b/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs index 8205a380ef..99d9d35cea 100644 --- a/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs @@ -46,7 +46,7 @@ namespace Umbraco.Cms.Core.Security IExternalLoginService externalLoginService, IOptions globalSettings, UmbracoMapper mapper, - IdentityErrorDescriber describer, + BackOfficeIdentityErrorDescriber describer, AppCaches appCaches) : base(describer) { diff --git a/src/Umbraco.Infrastructure/Security/IMemberManager.cs b/src/Umbraco.Infrastructure/Security/IMemberManager.cs index b310e9434f..a9ada29162 100644 --- a/src/Umbraco.Infrastructure/Security/IMemberManager.cs +++ b/src/Umbraco.Infrastructure/Security/IMemberManager.cs @@ -1,3 +1,5 @@ +using System.Collections.Generic; + namespace Umbraco.Cms.Core.Security { /// @@ -5,5 +7,16 @@ namespace Umbraco.Cms.Core.Security /// public interface IMemberManager : IUmbracoUserManager { + /// + /// Checks if the current member is authorized based on the parameters provided. + /// + /// Allowed types. + /// Allowed groups. + /// Allowed individual members. + /// True or false if the currently logged in member is authorized + bool IsMemberAuthorized( + IEnumerable allowTypes = null, + IEnumerable allowGroups = null, + IEnumerable allowMembers = null); } } diff --git a/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs b/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs index 0cf724ec20..70574181e0 100644 --- a/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs +++ b/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs @@ -114,6 +114,11 @@ namespace Umbraco.Cms.Core.Security target.IsApproved = source.IsApproved; target.SecurityStamp = source.SecurityStamp; target.LockoutEnd = source.IsLockedOut ? DateTime.MaxValue.ToUniversalTime() : (DateTime?)null; + target.Comments = source.Comments; + target.LastLockoutDateUtc = source.LastLockoutDate == DateTime.MinValue ? null : source.LastLockoutDate.ToUniversalTime(); + target.CreatedDateUtc = source.CreateDate.ToUniversalTime(); + target.Key = source.Key; + target.MemberTypeAlias = source.ContentTypeAlias; // NB: same comments re AutoMapper as per BackOfficeUser } diff --git a/src/Umbraco.Infrastructure/Security/MemberIdentityBuilder.cs b/src/Umbraco.Infrastructure/Security/MemberIdentityBuilder.cs deleted file mode 100644 index 4e2e4a39b1..0000000000 --- a/src/Umbraco.Infrastructure/Security/MemberIdentityBuilder.cs +++ /dev/null @@ -1,38 +0,0 @@ -using System; -using System.Reflection; -using Microsoft.AspNetCore.Identity; -using Microsoft.Extensions.DependencyInjection; - -namespace Umbraco.Cms.Core.Security -{ - public class MemberIdentityBuilder : IdentityBuilder - { - public MemberIdentityBuilder(IServiceCollection services) : base(typeof(MemberIdentityUser), services) - { - } - - public MemberIdentityBuilder(Type role, IServiceCollection services) : base(typeof(MemberIdentityUser), role, services) - { - } - - /// - /// Adds a token provider for the . - /// - /// The name of the provider to add. - /// The type of the to add. - /// The current instance. - public override IdentityBuilder AddTokenProvider(string providerName, Type provider) - { - if (!typeof(IUserTwoFactorTokenProvider<>).MakeGenericType(UserType).GetTypeInfo().IsAssignableFrom(provider.GetTypeInfo())) - { - throw new InvalidOperationException($"Invalid Type for TokenProvider: {provider.FullName}"); - } - Services.Configure(options => - { - options.Tokens.ProviderMap[providerName] = new TokenProviderDescriptor(provider); - }); - Services.AddTransient(provider); - return this; - } - } -} diff --git a/src/Umbraco.Infrastructure/Security/MemberIdentityUser.cs b/src/Umbraco.Infrastructure/Security/MemberIdentityUser.cs index dfd2602a7d..b9165e18af 100644 --- a/src/Umbraco.Infrastructure/Security/MemberIdentityUser.cs +++ b/src/Umbraco.Infrastructure/Security/MemberIdentityUser.cs @@ -14,6 +14,7 @@ namespace Umbraco.Cms.Core.Security public class MemberIdentityUser : UmbracoIdentityUser { private string _name; + private string _comments; private string _passwordConfig; private IReadOnlyCollection _groups; @@ -66,6 +67,24 @@ namespace Umbraco.Cms.Core.Security set => BeingDirty.SetPropertyValueAndDetectChanges(value, ref _name, nameof(Name)); } + /// + /// Gets or sets the member's comments + /// + public string Comments + { + get => _comments; + set => BeingDirty.SetPropertyValueAndDetectChanges(value, ref _comments, nameof(Comments)); + } + + // No change tracking because the persisted value is only set with the IsLockedOut flag + public DateTime? LastLockoutDateUtc { get; set; } + + // No change tracking because the persisted value is readonly + public DateTime CreatedDateUtc { get; set; } + + // No change tracking because the persisted value is readonly + public Guid Key { get; set; } + /// /// Gets or sets the password config /// diff --git a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs index 7e36081e73..de4bbca050 100644 --- a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs @@ -646,6 +646,13 @@ namespace Umbraco.Cms.Core.Security member.LastPasswordChangeDate = identityUserMember.LastPasswordChangeDateUtc.Value.ToLocalTime(); } + if (identityUserMember.IsPropertyDirty(nameof(MemberIdentityUser.Comments)) + && member.Comments != identityUserMember.Comments && identityUserMember.Comments.IsNullOrWhiteSpace() == false) + { + anythingChanged = true; + member.Comments = identityUserMember.Comments; + } + if (identityUserMember.IsPropertyDirty(nameof(MemberIdentityUser.EmailConfirmed)) || (member.EmailConfirmedDate.HasValue && member.EmailConfirmedDate.Value != default && identityUserMember.EmailConfirmed == false) || ((member.EmailConfirmedDate.HasValue == false || member.EmailConfirmedDate.Value == default) && identityUserMember.EmailConfirmed)) diff --git a/src/Umbraco.PublishedCache.NuCache/PublishedMember.cs b/src/Umbraco.PublishedCache.NuCache/PublishedMember.cs index dfd347168a..f717b90da2 100644 --- a/src/Umbraco.PublishedCache.NuCache/PublishedMember.cs +++ b/src/Umbraco.PublishedCache.NuCache/PublishedMember.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Umbraco.Cms.Core.Models; @@ -117,8 +117,6 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache public DateTime LastLoginDate => _member.LastLoginDate; - public DateTime LastActivityDate => _member.LastLoginDate; - public DateTime LastPasswordChangedDate => _member.LastPasswordChangeDate; #endregion diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs index e3f936bcac..135aa397c8 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs @@ -29,12 +29,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security private Mock> _mockIdentityOptions; private Mock> _mockPasswordHasher; private Mock _mockMemberService; - private Mock> _mockUserValidators; - private Mock>> _mockPasswordValidators; - private Mock _mockNormalizer; - private IdentityErrorDescriber _mockErrorDescriber; private Mock _mockServiceProviders; - private Mock>> _mockLogger; private Mock> _mockPasswordConfiguration; public MemberManager CreateSut() @@ -52,15 +47,12 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security _mockPasswordHasher = new Mock>(); var userValidators = new List>(); - _mockUserValidators = new Mock>(); var validator = new Mock>(); userValidators.Add(validator.Object); - _mockPasswordValidators = new Mock>>(); - _mockNormalizer = new Mock(); - _mockErrorDescriber = new IdentityErrorDescriber(); + //_mockNormalizer = new Mock(); + //_mockErrorDescriber = new IdentityErrorDescriber(); _mockServiceProviders = new Mock(); - _mockLogger = new Mock>>(); _mockPasswordConfiguration = new Mock>(); _mockPasswordConfiguration.Setup(x => x.Value).Returns(() => new MemberPasswordConfigurationSettings() diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs index 7f80ff9382..1029658d56 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs @@ -168,6 +168,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security m.Email == "fakeemail@umbraco.com" && m.Username == "fakeUsername" && m.RawPasswordValue == "fakePassword" && + m.Comments == "hello" && m.ContentTypeAlias == fakeMemberType.Alias && m.HasIdentity == true); @@ -186,6 +187,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security _mockMemberService.Verify(x => x.Save(mockMember, It.IsAny())); } + // TODO: Test updating! + [Test] public async Task GivenIDeleteUser_AndTheUserIsNotPresent_ThenIShouldGetAFailedResultAsync() diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgeryTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgeryTests.cs deleted file mode 100644 index f3abcabe61..0000000000 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgeryTests.cs +++ /dev/null @@ -1,79 +0,0 @@ -// Copyright (c) Umbraco. -// See LICENSE for more details. - -using System; -using System.Linq; -using System.Security.Claims; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Antiforgery; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; -using Microsoft.Net.Http.Headers; -using NUnit.Framework; -using Umbraco.Cms.Core; -using Umbraco.Cms.Web.BackOffice.Security; -using Umbraco.Extensions; - -namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Security -{ - [TestFixture] - public class BackOfficeAntiforgeryTests - { - private HttpContext GetHttpContext() - { - var identity = new ClaimsIdentity(); - identity.AddRequiredClaims( - Constants.Security.SuperUserIdAsString, - "test", - "test", - Enumerable.Empty(), - Enumerable.Empty(), - "en-US", - Guid.NewGuid().ToString(), - Enumerable.Empty(), - Enumerable.Empty()); - - var httpContext = new DefaultHttpContext() - { - User = new ClaimsPrincipal(identity) - }; - httpContext.Request.IsHttps = true; - return httpContext; - } - - [Test] - public async Task Validate_Tokens() - { - // This is the only way to get the DefaultAntiforgery service from aspnet - var container = new ServiceCollection(); - container.AddLogging(); - container.AddAntiforgery(); - ServiceProvider services = container.BuildServiceProvider(); - - IAntiforgery antiforgery = services.GetRequiredService(); - IOptions options = services.GetRequiredService>(); - - HttpContext httpContext = GetHttpContext(); - - var backofficeAntiforgery = new BackOfficeAntiforgery(antiforgery, options); - backofficeAntiforgery.GetTokens(httpContext, out var cookieToken, out var headerToken); - Assert.IsFalse(cookieToken.IsNullOrWhiteSpace()); - Assert.IsFalse(headerToken.IsNullOrWhiteSpace()); - - // the same context cannot validate since it's already created tokens - httpContext = GetHttpContext(); - - Attempt result = await backofficeAntiforgery.ValidateRequestAsync(httpContext); - - Assert.IsFalse(result.Success); // missing token - - // add cookie and header - httpContext.Request.Headers[HeaderNames.Cookie] = new CookieHeaderValue(Constants.Web.CsrfValidationCookieName, cookieToken).ToString(); - httpContext.Request.Headers[Constants.Web.AngularHeadername] = headerToken; - - result = await backofficeAntiforgery.ValidateRequestAsync(httpContext); - Assert.IsTrue(result.Success); // missing token - } - } -} diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs index bdcc5ec75b..b1317acb95 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs @@ -20,10 +20,10 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security public class MemberSignInManagerTests { private Mock>> _mockLogger; - private readonly Mock> _memberManager = MockUserManager(); + private readonly Mock _memberManager = MockUserManager(); - public MemberClaimsPrincipalFactory CreateClaimsFactory(UserManager userMgr) - => new MemberClaimsPrincipalFactory(userMgr, Options.Create(new MemberIdentityOptions())); + public UserClaimsPrincipalFactory CreateClaimsFactory(MemberManager userMgr) + => new UserClaimsPrincipalFactory(userMgr, Options.Create(new MemberIdentityOptions())); public MemberSignInManager CreateSut() { @@ -54,10 +54,10 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security Mock.Of(), Mock.Of>()); } - private static Mock> MockUserManager() + private static Mock MockUserManager() { var store = new Mock>(); - var mgr = new Mock>(store.Object, null, null, null, null, null, null, null, null); + var mgr = new Mock(store.Object, null, null, null, null, null, null, null, null); return mgr; } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Security/UmbracoWebsiteSecurityTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Security/UmbracoWebsiteSecurityTests.cs deleted file mode 100644 index badcaae8b5..0000000000 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Security/UmbracoWebsiteSecurityTests.cs +++ /dev/null @@ -1,102 +0,0 @@ -// Copyright (c) Umbraco. -// See LICENSE for more details. - -using System.Linq; -using System.Security.Claims; -using System.Security.Principal; -using Microsoft.AspNetCore.Http; -using Moq; -using NUnit.Framework; -using Umbraco.Cms.Core.Models; -using Umbraco.Cms.Core.Models.Security; -using Umbraco.Cms.Core.Security; -using Umbraco.Cms.Core.Services; -using Umbraco.Cms.Core.Strings; -using Umbraco.Cms.Tests.Common.Builders; -using Umbraco.Cms.Web.Common.Security; -using CoreConstants = Umbraco.Cms.Core.Constants; - -namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Website.Security -{ - [TestFixture] - public class UmbracoWebsiteSecurityTests - { - [Test] - public void Can_Create_Registration_Model_With_Default_Member_Type() - { - var sut = CreateUmbracoWebsiteSecurity(); - - var result = sut.CreateRegistrationModel(); - AssertRegisterModel(result); - } - - [Test] - public void Can_Create_Registration_Model_With_Custom_Member_Type() - { - const string Alias = "testAlias"; - var sut = CreateUmbracoWebsiteSecurity(Alias); - - var result = sut.CreateRegistrationModel(Alias); - AssertRegisterModel(result, Alias); - } - - [Test] - public void Can_Detected_Logged_In_User() - { - var sut = CreateUmbracoWebsiteSecurity(); - - var result = sut.IsLoggedIn(); - Assert.IsTrue(result); - } - - [Test] - public void Can_Detected_Anonymous_User() - { - var sut = CreateUmbracoWebsiteSecurity(isUserAuthenticated: false); - - var result = sut.IsLoggedIn(); - Assert.IsFalse(result); - } - - private static void AssertRegisterModel(RegisterModel result, string memberTypeAlias = CoreConstants.Conventions.MemberTypes.DefaultAlias) - { - Assert.AreEqual(memberTypeAlias, result.MemberTypeAlias); - Assert.AreEqual(1, result.MemberProperties.Count); - - var firstProperty = result.MemberProperties.First(); - Assert.AreEqual("title", firstProperty.Alias); - Assert.AreEqual("Title", firstProperty.Name); - Assert.AreEqual(string.Empty, firstProperty.Value); - } - - private IUmbracoWebsiteSecurity CreateUmbracoWebsiteSecurity(string memberTypeAlias = CoreConstants.Conventions.MemberTypes.DefaultAlias, bool isUserAuthenticated = true) - { - var mockHttpContextAccessor = new Mock(); - var mockHttpContext = new Mock(); - var mockIdentity = new Mock(); - mockIdentity.SetupGet(x => x.IsAuthenticated).Returns(isUserAuthenticated); - var mockPrincipal = new Mock(); - mockPrincipal.Setup(x => x.Identity).Returns(mockIdentity.Object); - mockHttpContext.Setup(m => m.User).Returns(mockPrincipal.Object); - mockHttpContextAccessor.SetupGet(x => x.HttpContext).Returns(mockHttpContext.Object); - - var mockMemberService = new Mock(); - - var mockMemberTypeService = new Mock(); - mockMemberTypeService - .Setup(x => x.Get(It.Is(y => y == memberTypeAlias))) - .Returns(CreateSimpleMemberType(memberTypeAlias)); - - var mockShortStringHelper = new Mock(); - - return new UmbracoWebsiteSecurity(mockHttpContextAccessor.Object, mockMemberService.Object, mockMemberTypeService.Object, mockShortStringHelper.Object); - } - - private IMemberType CreateSimpleMemberType(string alias) - { - var memberType = MemberTypeBuilder.CreateSimpleMemberType(alias); - memberType.SetMemberCanEditProperty("title", true); - return memberType; - } - } -} diff --git a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs index f53a4cab6b..5740c8c1a6 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs @@ -18,7 +18,6 @@ using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.Membership; -using Umbraco.Cms.Core.Models.Security; using Umbraco.Cms.Core.Net; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; @@ -30,6 +29,7 @@ using Umbraco.Cms.Web.Common.Attributes; using Umbraco.Cms.Web.Common.Authorization; using Umbraco.Cms.Web.Common.Controllers; using Umbraco.Cms.Web.Common.Filters; +using Umbraco.Cms.Web.Common.Models; using Umbraco.Extensions; using Constants = Umbraco.Cms.Core.Constants; using SignInResult = Microsoft.AspNetCore.Identity.SignInResult; @@ -347,6 +347,10 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers return verifyResponse; } + // TODO: We can check for these and respond differently if we think it's important + // result.IsLockedOut + // result.IsNotAllowed + // return BadRequest (400), we don't want to return a 401 because that get's intercepted // by our angular helper because it thinks that we need to re-perform the request once we are // authorized and we don't want to return a 403 because angular will show a warning message indicating diff --git a/src/Umbraco.Web.BackOffice/DependencyInjection/ServiceCollectionExtensions.cs b/src/Umbraco.Web.BackOffice/DependencyInjection/ServiceCollectionExtensions.cs index 9e887d6a86..f59f2635e5 100644 --- a/src/Umbraco.Web.BackOffice/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.BackOffice/DependencyInjection/ServiceCollectionExtensions.cs @@ -29,7 +29,8 @@ namespace Umbraco.Extensions .AddUserStore() .AddUserManager() .AddSignInManager() - .AddClaimsPrincipalFactory(); + .AddClaimsPrincipalFactory() + .AddErrorDescriber(); // Configure the options specifically for the UmbracoBackOfficeIdentityOptions instance services.ConfigureOptions(); @@ -38,38 +39,10 @@ namespace Umbraco.Extensions private static BackOfficeIdentityBuilder BuildUmbracoBackOfficeIdentity(this IServiceCollection services) { - // Borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Extensions.Core/src/IdentityServiceCollectionExtensions.cs#L33 - // The reason we need our own is because the Identity system doesn't cater easily for multiple identity systems and particularly being - // able to configure IdentityOptions to a specific provider since there is no named options. So we have strongly typed options - // and strongly typed ILookupNormalizer and IdentityErrorDescriber since those are 'global' and we need to be unintrusive. - - // TODO: Could move all of this to BackOfficeComposer? - - // Services used by identity - services.TryAddScoped, UserValidator>(); - services.TryAddScoped, PasswordValidator>(); - services.TryAddScoped>( - services => new BackOfficePasswordHasher( - new LegacyPasswordSecurity(), - services.GetRequiredService())); - services.TryAddScoped, DefaultUserConfirmation>(); - services.TryAddScoped, UserClaimsPrincipalFactory>(); - - // CUSTOM: - services.TryAddScoped(); - services.TryAddScoped(); services.TryAddScoped(); services.TryAddSingleton(); services.TryAddSingleton(); - - /* - * IdentityBuilderExtensions.AddUserManager adds UserManager to service collection - * To validate the container the following registrations are required (dependencies of UserManager) - * Perhaps we shouldn't be registering UserManager at all and only registering/depending the UmbracoBackOffice prefixed types. - */ - services.TryAddScoped(); - services.TryAddScoped(); - + return new BackOfficeIdentityBuilder(services); } diff --git a/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs index 21098ffabf..422b8a87ff 100644 --- a/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs @@ -62,8 +62,6 @@ namespace Umbraco.Extensions /// public static IUmbracoBuilder AddBackOfficeAuthentication(this IUmbracoBuilder builder) { - builder.Services.AddAntiforgery(); - builder.Services // This just creates a builder, nothing more diff --git a/src/Umbraco.Web.BackOffice/Filters/CheckIfUserTicketDataIsStaleAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/CheckIfUserTicketDataIsStaleAttribute.cs index d8f3374b5f..36dd11e9e5 100644 --- a/src/Umbraco.Web.BackOffice/Filters/CheckIfUserTicketDataIsStaleAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/CheckIfUserTicketDataIsStaleAttribute.cs @@ -88,10 +88,9 @@ namespace Umbraco.Cms.Web.BackOffice.Filters private async Task UpdateTokensAndAppendCustomHeaders(ActionExecutingContext actionContext) { - var tokenFilter = - new SetAngularAntiForgeryTokensAttribute.SetAngularAntiForgeryTokensFilter(_backOfficeAntiforgery, - _globalSettings); - await tokenFilter.OnActionExecutionAsync(actionContext, + var tokenFilter = new SetAngularAntiForgeryTokensAttribute.SetAngularAntiForgeryTokensFilter(_backOfficeAntiforgery); + await tokenFilter.OnActionExecutionAsync( + actionContext, () => Task.FromResult(new ActionExecutedContext(actionContext, new List(), null))); // add the header diff --git a/src/Umbraco.Web.BackOffice/Filters/SetAngularAntiForgeryTokensAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/SetAngularAntiForgeryTokensAttribute.cs index 543c6204a8..d869c93108 100644 --- a/src/Umbraco.Web.BackOffice/Filters/SetAngularAntiForgeryTokensAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/SetAngularAntiForgeryTokensAttribute.cs @@ -1,11 +1,8 @@ -using System.Net; +using System.Net; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; -using Microsoft.Extensions.Options; -using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Web.BackOffice.Security; -using Constants = Umbraco.Cms.Core.Constants; namespace Umbraco.Cms.Web.BackOffice.Filters { @@ -21,69 +18,21 @@ namespace Umbraco.Cms.Web.BackOffice.Filters internal class SetAngularAntiForgeryTokensFilter : IAsyncActionFilter { private readonly IBackOfficeAntiforgery _antiforgery; - private readonly GlobalSettings _globalSettings; - public SetAngularAntiForgeryTokensFilter(IBackOfficeAntiforgery antiforgery, IOptions globalSettings) - { - _antiforgery = antiforgery; - _globalSettings = globalSettings.Value; - } + public SetAngularAntiForgeryTokensFilter(IBackOfficeAntiforgery antiforgery) + => _antiforgery = antiforgery; public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) { - if (context.HttpContext.Response != null) - { - //DO not set the token cookies if the request has failed!! - if (context.HttpContext.Response.StatusCode == (int)HttpStatusCode.OK) - { - //don't need to set the cookie if they already exist and they are valid - if (context.HttpContext.Request.Cookies.TryGetValue(Constants.Web.AngularCookieName, out var angularCookieVal) - && context.HttpContext.Request.Cookies.TryGetValue(Constants.Web.CsrfValidationCookieName, out var csrfCookieVal)) - { - //if they are not valid for some strange reason - we need to continue setting valid ones - var valResult = await _antiforgery.ValidateRequestAsync(context.HttpContext); - if (valResult.Success) - { - await next(); - return; - } - } - - string cookieToken, headerToken; - _antiforgery.GetTokens(context.HttpContext, out cookieToken, out headerToken); - - //We need to set 2 cookies: one is the cookie value that angular will use to set a header value on each request, - // the 2nd is the validation value generated by the anti-forgery helper that we use to validate the header token against. - - if (!(headerToken is null)) - { - context.HttpContext.Response.Cookies.Append( - Constants.Web.AngularCookieName, headerToken, - new Microsoft.AspNetCore.Http.CookieOptions - { - Path = "/", - //must be js readable - HttpOnly = false, - Secure = _globalSettings.UseHttps - }); - } - - if (!(cookieToken is null)) - { - context.HttpContext.Response.Cookies.Append( - Constants.Web.CsrfValidationCookieName, cookieToken, - new Microsoft.AspNetCore.Http.CookieOptions - { - Path = "/", - HttpOnly = true, - Secure = _globalSettings.UseHttps - }); - } - - } - } - await next(); + + // anti forgery tokens are based on the currently logged + // in user assigned to the HttpContext which will be assigned during signin so + // we can only execute after the action. + if (context.HttpContext.Response?.StatusCode == (int)HttpStatusCode.OK) + { + _antiforgery.GetAndStoreTokens(context.HttpContext); + } } } diff --git a/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs index c4290396b6..31187d8350 100644 --- a/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs @@ -1,27 +1,16 @@ -using System.Linq; using System.Net; using System.Security.Claims; using System.Threading.Tasks; -using Microsoft.AspNetCore.Antiforgery; -using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; -using Microsoft.Extensions.Logging; -using Umbraco.Cms.Core.Web; using Umbraco.Cms.Web.BackOffice.Security; using Umbraco.Extensions; -using Constants = Umbraco.Cms.Core.Constants; namespace Umbraco.Cms.Web.BackOffice.Filters { /// /// An attribute/filter to check for the csrf token based on Angular's standard approach /// - /// - /// Code derived from http://ericpanorel.net/2013/07/28/spa-authentication-and-csrf-mvc4-antiforgery-implementation/ - /// - /// If the authentication type is cookie based, then this filter will execute, otherwise it will be disabled - /// public sealed class ValidateAngularAntiForgeryTokenAttribute : TypeFilterAttribute { public ValidateAngularAntiForgeryTokenAttribute() @@ -31,16 +20,9 @@ namespace Umbraco.Cms.Web.BackOffice.Filters private class ValidateAngularAntiForgeryTokenFilter : IAsyncActionFilter { - private readonly ILogger _logger; private readonly IBackOfficeAntiforgery _antiforgery; - private readonly ICookieManager _cookieManager; - public ValidateAngularAntiForgeryTokenFilter(ILogger logger, IBackOfficeAntiforgery antiforgery, ICookieManager cookieManager) - { - _logger = logger; - _antiforgery = antiforgery; - _cookieManager = cookieManager; - } + public ValidateAngularAntiForgeryTokenFilter(IBackOfficeAntiforgery antiforgery) => _antiforgery = antiforgery; public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) { @@ -54,13 +36,11 @@ namespace Umbraco.Cms.Web.BackOffice.Filters } } - var cookieToken = _cookieManager.GetCookieValue(Constants.Web.CsrfValidationCookieName); var httpContext = context.HttpContext; - - var validateResult = await ValidateHeaders(httpContext, cookieToken); - if (validateResult.Item1 == false) + var validateResult = await _antiforgery.ValidateRequestAsync(httpContext); + if (!validateResult.Success) { - httpContext.SetReasonPhrase(validateResult.Item2); + httpContext.SetReasonPhrase(validateResult.Result); context.Result = new StatusCodeResult((int)HttpStatusCode.ExpectationFailed); return; } @@ -68,50 +48,6 @@ namespace Umbraco.Cms.Web.BackOffice.Filters await next(); } - private async Task<(bool, string)> ValidateHeaders( - HttpContext httpContext, - string cookieToken) - { - var requestHeaders = httpContext.Request.Headers; - if (requestHeaders.Any(z => z.Key.InvariantEquals(Constants.Web.AngularHeadername)) == false) - { - return (false, "Missing token"); - } - - var headerToken = requestHeaders - .Where(z => z.Key.InvariantEquals(Constants.Web.AngularHeadername)) - .Select(z => z.Value) - .SelectMany(z => z) - .FirstOrDefault(); - - // both header and cookie must be there - if (cookieToken == null || headerToken == null) - { - return (false, "Missing token null"); - } - - if (await ValidateTokens(httpContext) == false) - { - return (false, "Invalid token"); - } - - return (true, "Success"); - } - - private async Task ValidateTokens(HttpContext httpContext) - { - // ensure that the cookie matches the header and then ensure it matches the correct value! - try - { - await _antiforgery.ValidateRequestAsync(httpContext); - return true; - } - catch (AntiforgeryValidationException ex) - { - _logger.LogError(ex, "Could not validate XSRF token"); - return false; - } - } } } } diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgery.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgery.cs index 396387f04e..eeeced89a4 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgery.cs +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgery.cs @@ -1,10 +1,13 @@ -using System.Linq; +using System; +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Antiforgery; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Microsoft.Net.Http.Headers; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Configuration.Models; using Constants = Umbraco.Cms.Core.Constants; namespace Umbraco.Cms.Web.BackOffice.Security @@ -20,100 +23,64 @@ namespace Umbraco.Cms.Web.BackOffice.Security /// public class BackOfficeAntiforgery : IBackOfficeAntiforgery { - private readonly IAntiforgery _defaultAntiforgery; - private readonly IOptions _antiforgeryOptions; + private readonly IAntiforgery _internalAntiForgery; + private readonly GlobalSettings _globalSettings; - public BackOfficeAntiforgery( - IAntiforgery defaultAntiforgery, - IOptions antiforgeryOptions) + public BackOfficeAntiforgery(IOptions globalSettings) { - _defaultAntiforgery = defaultAntiforgery; - _antiforgeryOptions = antiforgeryOptions; + // NOTE: This is the only way to create a separate IAntiForgery service :( + // Everything in netcore is internal. I have logged an issue here https://github.com/dotnet/aspnetcore/issues/22217 + // but it will not be handled so we have to revert to this. + var services = new ServiceCollection(); + services.AddLogging(); + services.AddAntiforgery(x => + { + x.HeaderName = Constants.Web.AngularHeadername; + x.Cookie.Name = Constants.Web.CsrfValidationCookieName; + }); + ServiceProvider container = services.BuildServiceProvider(); + _internalAntiForgery = container.GetRequiredService(); + _globalSettings = globalSettings.Value; } /// public async Task> ValidateRequestAsync(HttpContext httpContext) { - if (!httpContext.Request.Headers.TryGetValue(Constants.Web.AngularHeadername, out var headerVals)) + try { - return Attempt.Fail("Missing header"); + await _internalAntiForgery.ValidateRequestAsync(httpContext); + return Attempt.Succeed(); } - if (!httpContext.Request.Cookies.TryGetValue(Constants.Web.CsrfValidationCookieName, out var cookieToken)) + catch (Exception ex) { - return Attempt.Fail("Missing cookie"); + return Attempt.Fail(ex.Message); } - - var headerToken = headerVals.FirstOrDefault(); - - // both header and cookie must be there - if (cookieToken == null || headerToken == null) - { - return Attempt.Fail("Missing token null"); - } - - if (await ValidateTokensAsync(httpContext, cookieToken, headerToken) == false) - { - return Attempt.Fail("Invalid token"); - } - - return Attempt.Succeed(); } /// - public void GetTokens(HttpContext httpContext, out string cookieToken, out string headerToken) + public void GetAndStoreTokens(HttpContext httpContext) { - var set = _defaultAntiforgery.GetTokens(httpContext); + AntiforgeryTokenSet set = _internalAntiForgery.GetAndStoreTokens(httpContext); - cookieToken = set.CookieToken; - headerToken = set.RequestToken; - } - - /// - /// Validates the cookie and header tokens - /// - /// - /// - /// - /// - private async Task ValidateTokensAsync(HttpContext httpContext, string cookieToken, string headerToken) - { - // TODO: see https://github.com/dotnet/aspnetcore/issues/22217 - // An alternative way to doing this would be to create a separate container specifically for antiforgery and add custom options to - // it and resolve services directly from there. Could be worth a shot and could actually be a way for us to deal with these global - // things later on if this problem arises again. - - // We need to do some tricks here, save the initial cookie/header vals, then reset later - var originalCookies = httpContext.Request.Cookies; - var originalCookiesHeader = httpContext.Request.Headers[HeaderNames.Cookie]; - var originalHeader = httpContext.Request.Headers[_antiforgeryOptions.Value.HeaderName]; - - try + if (set.RequestToken == null) { - // swap the cookie anti-forgery cookie value for the one we want to validate - // (this is how you modify request cookies, it's the only way) - var cookieHeaderVals = CookieHeaderValue.ParseList(originalCookiesHeader); - cookieHeaderVals.Add(new CookieHeaderValue(_antiforgeryOptions.Value.Cookie.Name, cookieToken)); - httpContext.Request.Headers[HeaderNames.Cookie] = cookieHeaderVals.Select(c => c.ToString()).ToArray(); - - // swap the anti-forgery header value for the one we want to validate - httpContext.Request.Headers[_antiforgeryOptions.Value.HeaderName] = headerToken; - - // now validate - return await _defaultAntiforgery.IsRequestValidAsync(httpContext); + throw new InvalidOperationException("Could not resolve a request token."); } - finally - { - // reset - // change request cookies back to original - var cookieHeaderVals = CookieHeaderValue.ParseList(originalCookiesHeader); - httpContext.Request.Headers[HeaderNames.Cookie] = cookieHeaderVals.Select(c => c.ToString()).ToArray(); - - // change the header back to normal - httpContext.Request.Headers[_antiforgeryOptions.Value.HeaderName] = originalHeader; - } + // We need to set 2 cookies: + // The cookie value that angular will use to set a header value on each request - we need to manually set this here + // The validation cookie value generated by the anti-forgery helper that we validate the header token against - set above in GetAndStoreTokens + httpContext.Response.Cookies.Append( + Constants.Web.AngularCookieName, + set.RequestToken, + new CookieOptions + { + Path = "/", + //must be js readable + HttpOnly = false, + Secure = _globalSettings.UseHttps + }); } - } } diff --git a/src/Umbraco.Web.BackOffice/Security/IBackOfficeAntiforgery.cs b/src/Umbraco.Web.BackOffice/Security/IBackOfficeAntiforgery.cs index ded1374db2..97200b6a37 100644 --- a/src/Umbraco.Web.BackOffice/Security/IBackOfficeAntiforgery.cs +++ b/src/Umbraco.Web.BackOffice/Security/IBackOfficeAntiforgery.cs @@ -1,4 +1,4 @@ -using System.Threading.Tasks; +using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Umbraco.Cms.Core; @@ -23,6 +23,6 @@ namespace Umbraco.Cms.Web.BackOffice.Security /// /// /// - void GetTokens(HttpContext httpContext, out string cookieToken, out string headerToken); + void GetAndStoreTokens(HttpContext httpContext); } } diff --git a/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs b/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs index b8f524902e..00cff7a808 100644 --- a/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs @@ -63,24 +63,14 @@ namespace Umbraco.Extensions /// /// Adds the services required for using Members Identity /// + public static void AddMembersIdentity(this IServiceCollection services) => - services.BuildMembersIdentity() + services.AddIdentity() .AddDefaultTokenProviders() .AddMemberManager() - .AddClaimsPrincipalFactory() + .AddSignInManager() .AddUserStore() - .AddRoleStore() - .AddRoleValidator>() - .AddRoleManager>(); - - private static MemberIdentityBuilder BuildMembersIdentity(this IServiceCollection services) - { - // Services used by Umbraco members identity - services.TryAddScoped, UserValidator>(); - services.TryAddScoped, PasswordValidator>(); - services.TryAddScoped, PasswordHasher>(); - return new MemberIdentityBuilder(typeof(UmbracoIdentityRole), services); - } + .AddRoleStore(); private static void RemoveIntParamenterIfValueGreatherThen(IDictionary commands, string parameter, int maxValue) { diff --git a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs index 1c99fc77ab..106fce4b59 100644 --- a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs @@ -9,13 +9,12 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ApplicationModels; +using Microsoft.AspNetCore.Mvc.ViewFeatures; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; -using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Primitives; using Serilog; using Smidge; using Smidge.FileProcessors; @@ -64,7 +63,6 @@ using IHostingEnvironment = Umbraco.Cms.Core.Hosting.IHostingEnvironment; namespace Umbraco.Extensions { - // TODO: We could add parameters to configure each of these for flexibility /// @@ -278,7 +276,7 @@ namespace Umbraco.Extensions // Password hasher builder.Services.AddUnique(); - builder.Services.AddUnique(); + builder.Services.AddUnique(); builder.Services.AddTransient(); builder.Services.AddUnique(); @@ -296,7 +294,6 @@ namespace Umbraco.Extensions builder.Services.AddUnique(); builder.Services.AddUnique(); - builder.Services.AddUnique(); var umbracoApiControllerTypes = builder.TypeLoader.GetUmbracoApiControllers().ToList(); builder.WithCollectionBuilder() @@ -318,7 +315,6 @@ namespace Umbraco.Extensions builder.Services.AddScoped(); builder.Services.AddScoped(); - builder.Services.AddScoped(); builder.AddHttpClients(); diff --git a/src/Umbraco.Web.Common/Extensions/IdentityBuilderExtensions.cs b/src/Umbraco.Web.Common/Extensions/IdentityBuilderExtensions.cs index 6940221518..c7c33b80ba 100644 --- a/src/Umbraco.Web.Common/Extensions/IdentityBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/IdentityBuilderExtensions.cs @@ -18,8 +18,24 @@ namespace Umbraco.Extensions public static IdentityBuilder AddMemberManager(this IdentityBuilder identityBuilder) where TUserManager : UserManager, TInterface { + identityBuilder.AddUserManager(); identityBuilder.Services.AddScoped(typeof(TInterface), typeof(TUserManager)); return identityBuilder; } + + /// + /// Adds a implementation for + /// + /// The sign in manager interface + /// The sign in manager type + /// The + /// The current instance. + public static IdentityBuilder AddSignInManager(this IdentityBuilder identityBuilder) + where TSignInManager : SignInManager, TInterface + { + identityBuilder.AddSignInManager(); + identityBuilder.Services.AddScoped(typeof(TInterface), typeof(TSignInManager)); + return identityBuilder; + } } } diff --git a/src/Umbraco.Web.Common/Filters/UmbracoMemberAuthorizeFilter.cs b/src/Umbraco.Web.Common/Filters/UmbracoMemberAuthorizeFilter.cs index d49db02bb9..5ad064cd37 100644 --- a/src/Umbraco.Web.Common/Filters/UmbracoMemberAuthorizeFilter.cs +++ b/src/Umbraco.Web.Common/Filters/UmbracoMemberAuthorizeFilter.cs @@ -1,6 +1,7 @@ -using System.Collections.Generic; +using System.Collections.Generic; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.Security; using Umbraco.Extensions; @@ -12,46 +13,44 @@ namespace Umbraco.Cms.Web.Common.Filters /// public class UmbracoMemberAuthorizeFilter : IAuthorizationFilter { - // TODO: Lets revisit this when we get members done and the front-end working and whether it can be replaced or moved to an authz policy - private readonly IUmbracoWebsiteSecurity _websiteSecurity; - - public UmbracoMemberAuthorizeFilter(IUmbracoWebsiteSecurity websiteSecurity) + public UmbracoMemberAuthorizeFilter() { - _websiteSecurity = websiteSecurity; } - /// - /// Comma delimited list of allowed member types - /// - public string AllowType { get; private set;} - - /// - /// Comma delimited list of allowed member groups - /// - public string AllowGroup { get; private set;} - - /// - /// Comma delimited list of allowed members - /// - public string AllowMembers { get; private set; } - - private UmbracoMemberAuthorizeFilter(string allowType, string allowGroup, string allowMembers) + public UmbracoMemberAuthorizeFilter(string allowType, string allowGroup, string allowMembers) { AllowType = allowType; AllowGroup = allowGroup; AllowMembers = allowMembers; } + /// + /// Comma delimited list of allowed member types + /// + public string AllowType { get; private set; } + + /// + /// Comma delimited list of allowed member groups + /// + public string AllowGroup { get; private set; } + + /// + /// Comma delimited list of allowed members + /// + public string AllowMembers { get; private set; } + public void OnAuthorization(AuthorizationFilterContext context) { - if (!IsAuthorized()) + IMemberManager memberManager = context.HttpContext.RequestServices.GetRequiredService(); + + if (!IsAuthorized(memberManager)) { context.HttpContext.SetReasonPhrase("Resource restricted: either member is not logged on or is not of a permitted type or group."); context.Result = new ForbidResult(); } } - private bool IsAuthorized() + private bool IsAuthorized(IMemberManager memberManager) { if (AllowMembers.IsNullOrWhiteSpace()) { @@ -77,7 +76,7 @@ namespace Umbraco.Cms.Web.Common.Filters } } - return _websiteSecurity.IsMemberAuthorized(AllowType.Split(Core.Constants.CharArrays.Comma), AllowGroup.Split(Core.Constants.CharArrays.Comma), members); + return memberManager.IsMemberAuthorized(AllowType.Split(Core.Constants.CharArrays.Comma), AllowGroup.Split(Core.Constants.CharArrays.Comma), members); } } } diff --git a/src/Umbraco.Web.Common/Macros/PartialViewMacroEngine.cs b/src/Umbraco.Web.Common/Macros/PartialViewMacroEngine.cs index 4cbc2d7648..0b60de97cb 100644 --- a/src/Umbraco.Web.Common/Macros/PartialViewMacroEngine.cs +++ b/src/Umbraco.Web.Common/Macros/PartialViewMacroEngine.cs @@ -29,16 +29,16 @@ namespace Umbraco.Cms.Web.Common.Macros { private readonly IHttpContextAccessor _httpContextAccessor; private readonly IModelMetadataProvider _modelMetadataProvider; - private readonly ITempDataProvider _tempDataProvider; + private readonly ITempDataDictionaryFactory _tempDataDictionaryFactory; public PartialViewMacroEngine( IHttpContextAccessor httpContextAccessor, IModelMetadataProvider modelMetadataProvider, - ITempDataProvider tempDataProvider) + ITempDataDictionaryFactory tempDataDictionaryFactory) { _httpContextAccessor = httpContextAccessor; _modelMetadataProvider = modelMetadataProvider; - _tempDataProvider = tempDataProvider; + _tempDataDictionaryFactory = tempDataDictionaryFactory; } public MacroContent Execute(MacroModel macro, IPublishedContent content) @@ -65,7 +65,7 @@ namespace Umbraco.Cms.Web.Common.Macros // Check if there's proxied ViewData (i.e. returned from a SurfaceController) ProxyViewDataFeature proxyViewDataFeature = httpContext.Features.Get(); ViewDataDictionary viewData = proxyViewDataFeature?.ViewData ?? new ViewDataDictionary(_modelMetadataProvider, new ModelStateDictionary()); - ITempDataDictionary tempData = proxyViewDataFeature?.TempData ?? new TempDataDictionary(httpContext, _tempDataProvider); + ITempDataDictionary tempData = proxyViewDataFeature?.TempData ?? _tempDataDictionaryFactory.GetTempData(httpContext); var viewContext = new ViewContext( new ActionContext(httpContext, currentRouteData, new ControllerActionDescriptor()), diff --git a/src/Umbraco.Web.Common/Models/LoginModel.cs b/src/Umbraco.Web.Common/Models/LoginModel.cs new file mode 100644 index 0000000000..07bcf8b7ce --- /dev/null +++ b/src/Umbraco.Web.Common/Models/LoginModel.cs @@ -0,0 +1,21 @@ +using System.ComponentModel.DataAnnotations; +using System.Runtime.Serialization; + +namespace Umbraco.Cms.Web.Common.Models +{ + public class LoginModel : PostRedirectModel + { + [Required] + [Display(Name = "User name")] + public string Username { get; set; } + + [Required] + [DataType(DataType.Password)] + [Display(Name = "Password")] + [StringLength(maximumLength: 256)] + public string Password { get; set; } + + [Display(Name = "Remember me?")] + public bool RememberMe { get; set; } + } +} diff --git a/src/Umbraco.Core/Models/Security/PostRedirectModel.cs b/src/Umbraco.Web.Common/Models/PostRedirectModel.cs similarity index 92% rename from src/Umbraco.Core/Models/Security/PostRedirectModel.cs rename to src/Umbraco.Web.Common/Models/PostRedirectModel.cs index 179b24ce0c..ac12a12f0b 100644 --- a/src/Umbraco.Core/Models/Security/PostRedirectModel.cs +++ b/src/Umbraco.Web.Common/Models/PostRedirectModel.cs @@ -1,4 +1,4 @@ -namespace Umbraco.Cms.Core.Models.Security +namespace Umbraco.Cms.Web.Common.Models { /// /// A base model containing a value to indicate to Umbraco where to redirect to after Posting if diff --git a/src/Umbraco.Web.Common/Security/IMemberSignInManager.cs b/src/Umbraco.Web.Common/Security/IMemberSignInManager.cs new file mode 100644 index 0000000000..cc6b0e88b9 --- /dev/null +++ b/src/Umbraco.Web.Common/Security/IMemberSignInManager.cs @@ -0,0 +1,14 @@ +using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; +using Umbraco.Cms.Core.Security; + +namespace Umbraco.Cms.Web.Common.Security +{ + public interface IMemberSignInManager + { + // TODO: We could have a base interface for these to share with IBackOfficeSignInManager + Task PasswordSignInAsync(string userName, string password, bool isPersistent, bool lockoutOnFailure); + Task SignInAsync(MemberIdentityUser user, bool isPersistent, string authenticationMethod = null); + Task SignOutAsync(); + } +} diff --git a/src/Umbraco.Web.Common/Security/MemberClaimsPrincipalFactory.cs b/src/Umbraco.Web.Common/Security/MemberClaimsPrincipalFactory.cs deleted file mode 100644 index fe9a0eadd4..0000000000 --- a/src/Umbraco.Web.Common/Security/MemberClaimsPrincipalFactory.cs +++ /dev/null @@ -1,15 +0,0 @@ -using Microsoft.AspNetCore.Identity; -using Microsoft.Extensions.Options; -using Umbraco.Cms.Core.Security; - - -namespace Umbraco.Cms.Web.Common.Security -{ - public class MemberClaimsPrincipalFactory : UserClaimsPrincipalFactory - { - public MemberClaimsPrincipalFactory(UserManager userManager, IOptions optionsAccessor) - : base(userManager, optionsAccessor) - { - } - } -} diff --git a/src/Umbraco.Web.Common/Security/MemberManager.cs b/src/Umbraco.Web.Common/Security/MemberManager.cs index f3b80ba4bc..24314a99ec 100644 --- a/src/Umbraco.Web.Common/Security/MemberManager.cs +++ b/src/Umbraco.Web.Common/Security/MemberManager.cs @@ -28,5 +28,8 @@ namespace Umbraco.Cms.Web.Common.Security services, logger, passwordConfiguration) { } + + public bool IsMemberAuthorized(IEnumerable allowTypes = null, IEnumerable allowGroups = null, IEnumerable allowMembers = null) + => true; // TODO: Implement! } } diff --git a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs index eeec3c2899..40cc17667d 100644 --- a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs +++ b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs @@ -10,10 +10,11 @@ using Umbraco.Cms.Core.Security; namespace Umbraco.Cms.Web.Common.Security { + /// /// The sign in manager for members /// - public class MemberSignInManager : UmbracoSignInManager + public class MemberSignInManager : UmbracoSignInManager, IMemberSignInManager { public MemberSignInManager( UserManager memberManager, diff --git a/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurity.cs b/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurity.cs deleted file mode 100644 index b6dce007ed..0000000000 --- a/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurity.cs +++ /dev/null @@ -1,237 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Authentication.Cookies; -using Microsoft.AspNetCore.Http; -using Umbraco.Cms.Core; -using Umbraco.Cms.Core.Models; -using Umbraco.Cms.Core.Models.Security; -using Umbraco.Cms.Core.Security; -using Umbraco.Cms.Core.Services; -using Umbraco.Cms.Core.Strings; - -namespace Umbraco.Cms.Web.Common.Security -{ - public class UmbracoWebsiteSecurity : IUmbracoWebsiteSecurity - { - private readonly IHttpContextAccessor _httpContextAccessor; - private readonly IMemberService _memberService; - private readonly IMemberTypeService _memberTypeService; - private readonly IShortStringHelper _shortStringHelper; - - public UmbracoWebsiteSecurity(IHttpContextAccessor httpContextAccessor, - IMemberService memberService, - IMemberTypeService memberTypeService, - IShortStringHelper shortStringHelper) - { - _httpContextAccessor = httpContextAccessor; - _memberService = memberService; - _memberTypeService = memberTypeService; - _shortStringHelper = shortStringHelper; - } - - /// - public RegisterModel CreateRegistrationModel(string memberTypeAlias = null) - { - var providedOrDefaultMemberTypeAlias = memberTypeAlias ?? Core.Constants.Conventions.MemberTypes.DefaultAlias; - var memberType = _memberTypeService.Get(providedOrDefaultMemberTypeAlias); - if (memberType == null) - { - throw new InvalidOperationException($"Could not find a member type with alias: {providedOrDefaultMemberTypeAlias}."); - } - - var model = RegisterModel.CreateModel(); - model.MemberTypeAlias = providedOrDefaultMemberTypeAlias; - model.MemberProperties = GetMemberPropertiesViewModel(memberType); - return model; - } - - private List GetMemberPropertiesViewModel(IMemberType memberType, IMember member = null) - { - var viewProperties = new List(); - - var builtIns = ConventionsHelper.GetStandardPropertyTypeStubs(_shortStringHelper).Select(x => x.Key).ToArray(); - - var propertyTypes = memberType.PropertyTypes - .Where(x => builtIns.Contains(x.Alias) == false && memberType.MemberCanEditProperty(x.Alias)) - .OrderBy(p => p.SortOrder); - - foreach (var prop in propertyTypes) - { - var value = string.Empty; - if (member != null) - { - var propValue = member.Properties[prop.Alias]; - if (propValue != null && propValue.GetValue() != null) - { - value = propValue.GetValue().ToString(); - } - } - - var viewProperty = new UmbracoProperty - { - Alias = prop.Alias, - Name = prop.Name, - Value = value - }; - - // TODO: Perhaps one day we'll ship with our own EditorTempates but for now developers - // can just render their own. - - ////This is a rudimentary check to see what data template we should render - //// if developers want to change the template they can do so dynamically in their views or controllers - //// for a given property. - ////These are the default built-in MVC template types: “Boolean”, “Decimal”, “EmailAddress”, “HiddenInput”, “HTML”, “Object”, “String”, “Text”, and “Url” - //// by default we'll render a text box since we've defined that metadata on the UmbracoProperty.Value property directly. - //if (prop.DataTypeId == new Guid(Constants.PropertyEditors.TrueFalse)) - //{ - // viewProperty.EditorTemplate = "UmbracoBoolean"; - //} - //else - //{ - // switch (prop.DataTypeDatabaseType) - // { - // case DataTypeDatabaseType.Integer: - // viewProperty.EditorTemplate = "Decimal"; - // break; - // case DataTypeDatabaseType.Ntext: - // viewProperty.EditorTemplate = "Text"; - // break; - // case DataTypeDatabaseType.Date: - // case DataTypeDatabaseType.Nvarchar: - // break; - // } - //} - - viewProperties.Add(viewProperty); - } - - return viewProperties; - } - - /// - public async Task GetCurrentMemberProfileModelAsync() - { - if (IsLoggedIn() == false) - { - return null; - } - - var member = GetCurrentPersistedMember(); - - // This shouldn't happen but will if the member is deleted in the back office while the member is trying - // to use the front-end! - if (member == null) - { - // Log them out since they've been removed - await LogOutAsync(); - - return null; - } - - var model = new ProfileModel - { - Name = member.Name, - MemberTypeAlias = member.ContentTypeAlias, - - // TODO: get ASP.NET Core Identity equiavlant of MemberShipUser in order to get common membership properties such as Email - // and UserName (see MembershipProviderExtensions.GetCurrentUserName()for legacy membership provider implementation). - - //Email = membershipUser.Email, - //UserName = membershipUser.UserName, - //Comment = membershipUser.Comment, - //IsApproved = membershipUser.IsApproved, - //IsLockedOut = membershipUser.IsLockedOut, - //LastLockoutDate = membershipUser.LastLockoutDate, - //CreationDate = membershipUser.CreationDate, - //LastLoginDate = membershipUser.LastLoginDate, - //LastActivityDate = membershipUser.LastActivityDate, - //LastPasswordChangedDate = membershipUser.LastPasswordChangedDate - }; - - var memberType = _memberTypeService.Get(member.ContentTypeId); - - model.MemberProperties = GetMemberPropertiesViewModel(memberType, member); - - return model; - } - - /// - public Task UpdateMemberProfileAsync(ProfileModel model) - { - throw new NotImplementedException(); - } - - /// - public bool IsLoggedIn() - { - var httpContext = _httpContextAccessor.HttpContext; - return httpContext?.User != null && httpContext.User.Identity.IsAuthenticated; - } - - /// - public async Task GetCurrentLoginStatusAsync() - { - var model = LoginStatusModel.CreateModel(); - - if (IsLoggedIn() == false) - { - model.IsLoggedIn = false; - return model; - } - - var member = GetCurrentPersistedMember(); - - // This shouldn't happen but will if the member is deleted in the back office while the member is trying - // to use the front-end! - if (member == null) - { - // Log them out since they've been removed. - await LogOutAsync(); - model.IsLoggedIn = false; - return model; - } - - model.Name = member.Name; - model.Username = member.Username; - model.Email = member.Email; - model.IsLoggedIn = true; - - return model; - } - - /// - /// Returns the currently logged in IMember object - this should never be exposed to the front-end since it's returning a business logic entity! - /// - /// - private IMember GetCurrentPersistedMember() - { - // TODO: get user name from ASP.NET Core Identity (see MembershipProviderExtensions.GetCurrentUserName() - // for legacy membership provider implementation). - var username = ""; - - // The result of this is cached by the MemberRepository - return _memberService.GetByUsername(username); - } - - /// - public Task LoginAsync(string username, string password) - { - throw new NotImplementedException(); - } - - /// - public async Task LogOutAsync() - { - await _httpContextAccessor.HttpContext.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme); - } - - /// - public bool IsMemberAuthorized(IEnumerable allowTypes = null, IEnumerable allowGroups = null, IEnumerable allowMembers = null) - { - throw new NotImplementedException(); - } - } -} diff --git a/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityAccessor.cs b/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityAccessor.cs deleted file mode 100644 index 2b90b5ad14..0000000000 --- a/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityAccessor.cs +++ /dev/null @@ -1,23 +0,0 @@ -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; -using Umbraco.Cms.Core.Security; - -namespace Umbraco.Cms.Web.Common.Security -{ - - public class UmbracoWebsiteSecurityAccessor : IUmbracoWebsiteSecurityAccessor - { - private readonly IHttpContextAccessor _httpContextAccessor; - - /// - /// Initializes a new instance of the class. - /// - public UmbracoWebsiteSecurityAccessor(IHttpContextAccessor httpContextAccessor) => _httpContextAccessor = httpContextAccessor; - - /// - /// Gets or sets the object. - /// - public IUmbracoWebsiteSecurity WebsiteSecurity - => _httpContextAccessor.HttpContext?.RequestServices?.GetService(); - } -} diff --git a/src/Umbraco.Web.Common/Templates/TemplateRenderer.cs b/src/Umbraco.Web.Common/Templates/TemplateRenderer.cs index 81f034918a..df72f44f10 100644 --- a/src/Umbraco.Web.Common/Templates/TemplateRenderer.cs +++ b/src/Umbraco.Web.Common/Templates/TemplateRenderer.cs @@ -39,7 +39,7 @@ namespace Umbraco.Cms.Web.Common.Templates private readonly IHttpContextAccessor _httpContextAccessor; private readonly ICompositeViewEngine _viewEngine; private readonly IModelMetadataProvider _modelMetadataProvider; - private readonly ITempDataProvider _tempDataProvider; + private readonly ITempDataDictionaryFactory _tempDataDictionaryFactory; public TemplateRenderer( IUmbracoContextAccessor umbracoContextAccessor, @@ -50,7 +50,7 @@ namespace Umbraco.Cms.Web.Common.Templates IHttpContextAccessor httpContextAccessor, ICompositeViewEngine viewEngine, IModelMetadataProvider modelMetadataProvider, - ITempDataProvider tempDataProvider) + ITempDataDictionaryFactory tempDataDictionaryFactory) { _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor)); _publishedRouter = publishedRouter ?? throw new ArgumentNullException(nameof(publishedRouter)); @@ -60,7 +60,7 @@ namespace Umbraco.Cms.Web.Common.Templates _httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor)); _viewEngine = viewEngine ?? throw new ArgumentNullException(nameof(viewEngine)); _modelMetadataProvider = modelMetadataProvider; - _tempDataProvider = tempDataProvider; + _tempDataDictionaryFactory = tempDataDictionaryFactory; } public async Task RenderAsync(int pageId, int? altTemplateId, StringWriter writer) @@ -169,7 +169,7 @@ namespace Umbraco.Cms.Web.Common.Templates new ActionContext(httpContext, httpContext.GetRouteData(), new ControllerActionDescriptor()), viewResult.View, viewData, - new TempDataDictionary(httpContext, _tempDataProvider), + _tempDataDictionaryFactory.GetTempData(httpContext), writer, new HtmlHelperOptions() ); diff --git a/src/Umbraco.Web.Common/Views/UmbracoViewPage.cs b/src/Umbraco.Web.Common/Views/UmbracoViewPage.cs index f8627dc2e9..c2608efc41 100644 --- a/src/Umbraco.Web.Common/Views/UmbracoViewPage.cs +++ b/src/Umbraco.Web.Common/Views/UmbracoViewPage.cs @@ -116,7 +116,7 @@ namespace Umbraco.Cms.Web.Common.Views { // filter / add preview banner // ASP.NET default value is text/html - if (Context.Response.ContentType.InvariantContains("text/html")) + if (Context.Response?.ContentType?.InvariantContains("text/html") ?? false) { if ((UmbracoContext.IsDebug || UmbracoContext.InPreviewMode) && tagHelperOutput.TagName != null diff --git a/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/EditProfile.cshtml b/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/EditProfile.cshtml index e88794bcb5..55a2c05918 100644 --- a/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/EditProfile.cshtml +++ b/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/EditProfile.cshtml @@ -1,62 +1,68 @@ -@using Umbraco.Cms.Core.Security -@using Umbraco.Cms.Web.Website.Controllers -@using Umbraco.Extensions @inherits Umbraco.Cms.Web.Common.Macros.PartialViewMacroPage -@inject IUmbracoWebsiteSecurityAccessor UmbracoWebsiteSecurityAccessor +@using Umbraco.Cms.Core +@using Umbraco.Cms.Core.Security +@using Umbraco.Cms.Web.Website.Controllers +@using Umbraco.Cms.Web.Website.Models +@using Umbraco.Extensions +@inject MemberModelBuilderFactory memberModelBuilderFactory; @{ - var websiteSecurity = UmbracoWebsiteSecurityAccessor.WebsiteSecurity; - var profileModel = await websiteSecurity.GetCurrentMemberProfileModelAsync(); + // Build a profile model to edit + var profileModel = await memberModelBuilderFactory + .CreateProfileModel() + // If null or not set, this will redirect to the current page + .WithRedirectUrl(null) + .BuildForCurrentMemberAsync(); - - var success = TempData["ProfileUpdateSuccess"] != null; + var success = TempData["FormSuccess"] != null; } -@if (websiteSecurity.IsLoggedIn() && profileModel != null) +@if(profileModel != null) { if (success) { @* This message will show if profileModel.RedirectUrl is not defined (default) *@ -

Profile updated

+

Profile updated

} - using (Html.BeginUmbracoForm("HandleUpdateProfile")) + using (Html.BeginUmbracoForm("HandleUpdateProfile", new { RedirectUrl = profileModel.RedirectUrl })) { -
- Edit profile +

Update your account.

+
+
+
+ + + +
+
+ + + +
- @Html.ValidationSummary("profileModel", true) + @if (!string.IsNullOrWhiteSpace(profileModel.UserName)) + { +
+ + + +
+ } - @Html.LabelFor(m => profileModel.Name) - @Html.TextBoxFor(m => profileModel.Name) - @Html.ValidationMessageFor(m => profileModel.Name) -
+for (var i = 0; i < profileModel.MemberProperties.Count; i++) + { +
+ + + +
+ } - @Html.LabelFor(m => profileModel.Email) - @Html.TextBoxFor(m => profileModel.Email) - @Html.ValidationMessageFor(m => profileModel.Email) -
- - @for (var i = 0; i < profileModel.MemberProperties.Count; i++) - { - @Html.LabelFor(m => profileModel.MemberProperties[i].Value, profileModel.MemberProperties[i].Name) - @* - By default this will render a textbox but if you want to change the editor template for this property you can - easily change it. For example, if you wanted to render a custom editor for this field called "MyEditor" you would - create a file at ~/Views/Shared/EditorTemplates/MyEditor.cshtml", then you will change the next line of code to - render your specific editor template like: - @Html.EditorFor(m => profileModel.MemberProperties[i].Value, "MyEditor") - *@ - @Html.EditorFor(m => profileModel.MemberProperties[i].Value) - @Html.HiddenFor(m => profileModel.MemberProperties[i].Alias) -
- } - - -
+ } } diff --git a/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/Login.cshtml b/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/Login.cshtml index 404f2a155e..b2245dc6b6 100644 --- a/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/Login.cshtml +++ b/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/Login.cshtml @@ -1,36 +1,45 @@ +@inherits Umbraco.Cms.Web.Common.Macros.PartialViewMacroPage @using Microsoft.AspNetCore.Http.Extensions -@using Umbraco.Cms.Core.Models.Security +@using Umbraco.Cms.Web.Common.Models @using Umbraco.Cms.Web.Website.Controllers @using Umbraco.Extensions -@inherits Umbraco.Cms.Web.Common.Macros.PartialViewMacroPage @{ var loginModel = new LoginModel(); - loginModel.RedirectUrl = Context.Request.GetDisplayUrl(); + // You can modify this to redirect to a different URL instead of the current one + loginModel.RedirectUrl = null; } -@using (Html.BeginUmbracoForm("HandleLogin")) -{ - @Html.HiddenFor(m => loginModel.RedirectUrl) -
- Login +
-} + diff --git a/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/LoginStatus.cshtml b/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/LoginStatus.cshtml index cd64033b48..089e534bf1 100644 --- a/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/LoginStatus.cshtml +++ b/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/LoginStatus.cshtml @@ -1,34 +1,26 @@ -@using Umbraco.Cms.Core.Models.Security -@using Umbraco.Cms.Core.Security +@inherits Umbraco.Cms.Web.Common.Macros.PartialViewMacroPage +@using Microsoft.AspNetCore.Http.Extensions +@using Umbraco.Cms.Web.Common.Models @using Umbraco.Cms.Web.Website.Controllers @using Umbraco.Extensions -@inherits Umbraco.Cms.Web.Common.Macros.PartialViewMacroPage -@inject IUmbracoWebsiteSecurityAccessor UmbracoWebsiteSecurityAccessor @{ - var websiteSecurity = UmbracoWebsiteSecurityAccessor.WebsiteSecurity; - var loginStatusModel = await websiteSecurity.GetCurrentLoginStatusAsync(); + var isLoggedIn = Context.User?.Identity?.IsAuthenticated ?? false; var logoutModel = new PostRedirectModel(); - - @* - Here you can specify a redirect URL for after logging out, by default umbraco will simply - redirect to the current page. Example to redirect to the home page: - - logoutModel.RedirectUrl = "/"; - *@ + // You can modify this to redirect to a different URL instead of the current one + logoutModel.RedirectUrl = null; } -@if (loginStatusModel.IsLoggedIn) +@if (isLoggedIn) { -

You are currently logged in as @loginStatusModel.Name

+ } diff --git a/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/RegisterMember.cshtml b/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/RegisterMember.cshtml index 2c860ca435..ae5703b15c 100644 --- a/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/RegisterMember.cshtml +++ b/src/Umbraco.Web.UI.NetCore/umbraco/PartialViewMacros/Templates/RegisterMember.cshtml @@ -1,36 +1,24 @@ -@using Umbraco.Cms.Core.Security -@using Umbraco.Cms.Web.Website.Controllers -@using Umbraco.Extensions @inherits Umbraco.Cms.Web.Common.Macros.PartialViewMacroPage -@inject IUmbracoWebsiteSecurityAccessor UmbracoWebsiteSecurityAccessor +@using Microsoft.AspNetCore.Http.Extensions +@using Umbraco.Cms.Core +@using Umbraco.Cms.Core.Security +@using Umbraco.Cms.Web.Website.Controllers +@using Umbraco.Cms.Web.Website.Models +@using Umbraco.Extensions +@inject MemberModelBuilderFactory memberModelBuilderFactory; @{ - @* - You can specify a custom member type alias in the constructor, the default is 'Member' - for example, to use 'Custom Member' you'd use this syntax: - - var registerModel = Members.CreateRegistrationModel("Custom Member"); - *@ - - var websiteSecurity = UmbracoWebsiteSecurityAccessor.WebsiteSecurity; - var registerModel = websiteSecurity.CreateRegistrationModel(); - - @* - Configurable here: - - registerModel.RedirectUrl - Optional. What path to redirect to if registration is successful. - By default the member will be redirected to the current umbraco page - unless this is specified. - - registerModel.UsernameIsEmail - the default is true - if you want the username to be different from the email - address, set this to true and add a new Username field in - the form below - - @Html.LabelFor(m => registerModel.Username) - @Html.TextBoxFor(m => registerModel.Username) - @Html.ValidationMessageFor(m => registerModel.Username) - *@ + // Build a registration model with parameters + var registerModel = memberModelBuilderFactory + .CreateRegisterModel() + // Set the member type alias to use for the new member + .WithMemberTypeAlias(Constants.Conventions.MemberTypes.DefaultAlias) + // If null or not set, this will redirect to the current page + .WithRedirectUrl(null) + // Set to true if you want the member editable properties shown. + // It will only displays properties marked as "Member can edit" on the "Info" tab of the Member Type. + .LookupProperties(true) + .Build(); var success = TempData["FormSuccess"] != null; } @@ -42,58 +30,55 @@ @if (success) { @* This message will show if registerModel.RedirectUrl is not defined (default) *@ -

Registration succeeded.

+

Registration succeeded.

} else { - using (Html.BeginUmbracoForm("HandleRegisterMember")) + using (Html.BeginUmbracoForm( + "HandleRegisterMember", + new { + MemberTypeAlias = registerModel.MemberTypeAlias, + UsernameIsEmail = registerModel.UsernameIsEmail, + RedirectUrl = registerModel.RedirectUrl + })) { -
- Register Member +

Create a new account.

+
+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
- @Html.ValidationSummary("registerModel", true) - - @Html.LabelFor(m => registerModel.Name) - @Html.TextBoxFor(m => registerModel.Name) - @Html.ValidationMessageFor(m => registerModel.Name) -
- - @Html.LabelFor(m => registerModel.Email) - @Html.TextBoxFor(m => registerModel.Email) - @Html.ValidationMessageFor(m => registerModel.Email) -
- - @Html.LabelFor(m => registerModel.Password) - @Html.PasswordFor(m => registerModel.Password) - @Html.ValidationMessageFor(m => registerModel.Password) -
- - @if (registerModel.MemberProperties != null) + @if (registerModel.MemberProperties != null) + { + for (var i = 0; i < registerModel.MemberProperties.Count; i++) { - @* - It will only displays properties marked as "Member can edit" on the "Info" tab of the Member Type. - *@ - for (var i = 0; i < registerModel.MemberProperties.Count; i++) - { - @Html.LabelFor(m => registerModel.MemberProperties[i].Value, registerModel.MemberProperties[i].Name) - @* - By default this will render a textbox but if you want to change the editor template for this property you can - easily change it. For example, if you wanted to render a custom editor for this field called "MyEditor" you would - create a file at ~/Views/Shared/EditorTemplates/MyEditor.cshtml", then you will change the next line of code to - render your specific editor template like: - @Html.EditorFor(m => registerModel.MemberProperties[i].Value, "MyEditor") - *@ - @Html.EditorFor(m => registerModel.MemberProperties[i].Value) - @Html.HiddenFor(m => registerModel.MemberProperties[i].Alias) -
- } +
+ + + +
} + } - @Html.HiddenFor(m => registerModel.MemberTypeAlias) - @Html.HiddenFor(m => registerModel.RedirectUrl) - @Html.HiddenFor(m => registerModel.UsernameIsEmail) - - -
+ + } } diff --git a/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs b/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs index 93b1f23b76..6a7b8941be 100644 --- a/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs +++ b/src/Umbraco.Web.Website/Controllers/UmbLoginController.cs @@ -1,22 +1,25 @@ using System; using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Logging; -using Umbraco.Cms.Core.Models.Security; using Umbraco.Cms.Core.Routing; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Web.Common.Filters; +using Umbraco.Cms.Web.Common.Models; +using Umbraco.Cms.Web.Common.Security; using Umbraco.Extensions; +using SignInResult = Microsoft.AspNetCore.Identity.SignInResult; namespace Umbraco.Cms.Web.Website.Controllers { public class UmbLoginController : SurfaceController { - private readonly IUmbracoWebsiteSecurityAccessor _websiteSecurityAccessor; + private readonly IMemberSignInManager _signInManager; public UmbLoginController( IUmbracoContextAccessor umbracoContextAccessor, @@ -25,10 +28,10 @@ namespace Umbraco.Cms.Web.Website.Controllers AppCaches appCaches, IProfilingLogger profilingLogger, IPublishedUrlProvider publishedUrlProvider, - IUmbracoWebsiteSecurityAccessor websiteSecurityAccessor) + IMemberSignInManager signInManager) : base(umbracoContextAccessor, databaseFactory, services, appCaches, profilingLogger, publishedUrlProvider) { - _websiteSecurityAccessor = websiteSecurityAccessor; + _signInManager = signInManager; } [HttpPost] @@ -41,27 +44,55 @@ namespace Umbraco.Cms.Web.Website.Controllers return CurrentUmbracoPage(); } - if (await _websiteSecurityAccessor.WebsiteSecurity.LoginAsync(model.Username, model.Password) == false) + MergeRouteValuesToModel(model); + + // Sign the user in with username/password, this also gives a chance for developers to + // custom verify the credentials and auto-link user accounts with a custom IBackOfficePasswordChecker + SignInResult result = await _signInManager.PasswordSignInAsync( + model.Username, model.Password, isPersistent: model.RememberMe, lockoutOnFailure: true); + + if (result.Succeeded) { - // Don't add a field level error, just model level. - ModelState.AddModelError("loginModel", "Invalid username or password"); - return CurrentUmbracoPage(); + TempData["LoginSuccess"] = true; + + // If there is a specified path to redirect to then use it. + if (model.RedirectUrl.IsNullOrWhiteSpace() == false) + { + // Validate the redirect URL. + // If it's not a local URL we'll redirect to the root of the current site. + return Redirect(Url.IsLocalUrl(model.RedirectUrl) + ? model.RedirectUrl + : CurrentPage.AncestorOrSelf(1).Url(PublishedUrlProvider)); + } + + // Redirect to current page by default. + return RedirectToCurrentUmbracoPage(); } - TempData["LoginSuccess"] = true; - - // If there is a specified path to redirect to then use it. - if (model.RedirectUrl.IsNullOrWhiteSpace() == false) + if (result.RequiresTwoFactor) { - // Validate the redirect URL. - // If it's not a local URL we'll redirect to the root of the current site. - return Redirect(Url.IsLocalUrl(model.RedirectUrl) - ? model.RedirectUrl - : CurrentPage.AncestorOrSelf(1).Url(PublishedUrlProvider)); + throw new NotImplementedException("Two factor support is not supported for Umbraco members yet"); } - // Redirect to current page by default. - return RedirectToCurrentUmbracoPage(); + // TODO: We can check for these and respond differently if we think it's important + // result.IsLockedOut + // result.IsNotAllowed + + // Don't add a field level error, just model level. + ModelState.AddModelError("loginModel", "Invalid username or password"); + return CurrentUmbracoPage(); + } + + /// + /// We pass in values via encrypted route values so they cannot be tampered with and merge them into the model for use + /// + /// + private void MergeRouteValuesToModel(LoginModel model) + { + if (RouteData.Values.TryGetValue(nameof(LoginModel.RedirectUrl), out var redirectUrl) && redirectUrl != null) + { + model.RedirectUrl = redirectUrl.ToString(); + } } } } diff --git a/src/Umbraco.Web.Website/Controllers/UmbLoginStatusController.cs b/src/Umbraco.Web.Website/Controllers/UmbLoginStatusController.cs index 230791bdb9..ab22e6f143 100644 --- a/src/Umbraco.Web.Website/Controllers/UmbLoginStatusController.cs +++ b/src/Umbraco.Web.Website/Controllers/UmbLoginStatusController.cs @@ -1,14 +1,14 @@ -using System.Threading.Tasks; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Logging; -using Umbraco.Cms.Core.Models.Security; using Umbraco.Cms.Core.Routing; -using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Web.Common.Filters; +using Umbraco.Cms.Web.Common.Models; +using Umbraco.Cms.Web.Common.Security; using Umbraco.Extensions; namespace Umbraco.Cms.Web.Website.Controllers @@ -16,15 +16,18 @@ namespace Umbraco.Cms.Web.Website.Controllers [UmbracoMemberAuthorize] public class UmbLoginStatusController : SurfaceController { - private readonly IUmbracoWebsiteSecurityAccessor _websiteSecurityAccessor; + private readonly IMemberSignInManager _signInManager; - public UmbLoginStatusController(IUmbracoContextAccessor umbracoContextAccessor, - IUmbracoDatabaseFactory databaseFactory, ServiceContext services, AppCaches appCaches, - IProfilingLogger profilingLogger, IPublishedUrlProvider publishedUrlProvider, IUmbracoWebsiteSecurityAccessor websiteSecurityAccessor) + public UmbLoginStatusController( + IUmbracoContextAccessor umbracoContextAccessor, + IUmbracoDatabaseFactory databaseFactory, + ServiceContext services, + AppCaches appCaches, + IProfilingLogger profilingLogger, + IPublishedUrlProvider publishedUrlProvider, + IMemberSignInManager signInManager) : base(umbracoContextAccessor, databaseFactory, services, appCaches, profilingLogger, publishedUrlProvider) - { - _websiteSecurityAccessor = websiteSecurityAccessor; - } + => _signInManager = signInManager; [HttpPost] [ValidateAntiForgeryToken] @@ -36,9 +39,11 @@ namespace Umbraco.Cms.Web.Website.Controllers return CurrentUmbracoPage(); } - if (_websiteSecurityAccessor.WebsiteSecurity.IsLoggedIn()) + var isLoggedIn = HttpContext.User?.Identity?.IsAuthenticated ?? false; + + if (isLoggedIn) { - await _websiteSecurityAccessor.WebsiteSecurity.LogOutAsync(); + await _signInManager.SignOutAsync(); } TempData["LogoutSuccess"] = true; diff --git a/src/Umbraco.Web.Website/Controllers/UmbProfileController.cs b/src/Umbraco.Web.Website/Controllers/UmbProfileController.cs index b2b4c0778f..492921444e 100644 --- a/src/Umbraco.Web.Website/Controllers/UmbProfileController.cs +++ b/src/Umbraco.Web.Website/Controllers/UmbProfileController.cs @@ -1,15 +1,18 @@ -using System; +using System; +using System.Linq; using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Logging; -using Umbraco.Cms.Core.Models.Security; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Routing; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Web.Common.Filters; +using Umbraco.Cms.Web.Website.Models; using Umbraco.Extensions; namespace Umbraco.Cms.Web.Website.Controllers @@ -17,14 +20,25 @@ namespace Umbraco.Cms.Web.Website.Controllers [UmbracoMemberAuthorize] public class UmbProfileController : SurfaceController { - private readonly IUmbracoWebsiteSecurityAccessor _websiteSecurityAccessor; + private readonly IMemberManager _memberManager; + private readonly IMemberService _memberService; + private readonly IMemberTypeService _memberTypeService; - public UmbProfileController(IUmbracoContextAccessor umbracoContextAccessor, IUmbracoDatabaseFactory databaseFactory, - ServiceContext services, AppCaches appCaches, IProfilingLogger profilingLogger, - IPublishedUrlProvider publishedUrlProvider, IUmbracoWebsiteSecurityAccessor websiteSecurityAccessor) + public UmbProfileController( + IUmbracoContextAccessor umbracoContextAccessor, + IUmbracoDatabaseFactory databaseFactory, + ServiceContext services, + AppCaches appCaches, + IProfilingLogger profilingLogger, + IPublishedUrlProvider publishedUrlProvider, + IMemberManager memberManager, + IMemberService memberService, + IMemberTypeService memberTypeService) : base(umbracoContextAccessor, databaseFactory, services, appCaches, profilingLogger, publishedUrlProvider) { - _websiteSecurityAccessor = websiteSecurityAccessor; + _memberManager = memberManager; + _memberService = memberService; + _memberTypeService = memberTypeService; } [HttpPost] @@ -37,20 +51,23 @@ namespace Umbraco.Cms.Web.Website.Controllers return CurrentUmbracoPage(); } - var result = await _websiteSecurityAccessor.WebsiteSecurity.UpdateMemberProfileAsync(model); - switch (result.Status) + MergeRouteValuesToModel(model); + + MemberIdentityUser currentMember = await _memberManager.GetUserAsync(HttpContext.User); + if (currentMember == null) { - case UpdateMemberProfileStatus.Success: - break; - case UpdateMemberProfileStatus.Error: - // Don't add a field level error, just model level. - ModelState.AddModelError("profileModel", result.ErrorMessage); - return CurrentUmbracoPage(); - default: - throw new ArgumentOutOfRangeException(); + // this shouldn't happen, we also don't want to return an error so just redirect to where we came from + return RedirectToCurrentUmbracoPage(); } - TempData["ProfileUpdateSuccess"] = true; + IdentityResult result = await UpdateMemberAsync(model, currentMember); + if (!result.Succeeded) + { + AddErrors(result); + return CurrentUmbracoPage(); + } + + TempData["FormSuccess"] = true; // If there is a specified path to redirect to then use it. if (model.RedirectUrl.IsNullOrWhiteSpace() == false) @@ -61,5 +78,68 @@ namespace Umbraco.Cms.Web.Website.Controllers // Redirect to current page by default. return RedirectToCurrentUmbracoPage(); } + + /// + /// We pass in values via encrypted route values so they cannot be tampered with and merge them into the model for use + /// + /// + private void MergeRouteValuesToModel(ProfileModel model) + { + if (RouteData.Values.TryGetValue(nameof(ProfileModel.RedirectUrl), out var redirectUrl) && redirectUrl != null) + { + model.RedirectUrl = redirectUrl.ToString(); + } + } + + private void AddErrors(IdentityResult result) + { + foreach (var error in result.Errors) + { + ModelState.AddModelError("profileModel", error.Description); + } + } + + private async Task UpdateMemberAsync(ProfileModel model, MemberIdentityUser currentMember) + { + currentMember.Email = model.Email; + currentMember.Name = model.Name; + currentMember.UserName = model.UserName; + currentMember.Comments = model.Comments; + + IdentityResult saveResult = await _memberManager.UpdateAsync(currentMember); + if (!saveResult.Succeeded) + { + return saveResult; + } + + // now we can update the custom properties + // TODO: Ideally we could do this all through our MemberIdentityUser + + IMember member = _memberService.GetByKey(currentMember.Key); + if (member == null) + { + // should never happen + throw new InvalidOperationException($"Could not find a member with key: {member.Key}."); + } + + IMemberType memberType = _memberTypeService.Get(member.ContentTypeId); + + if (model.MemberProperties != null) + { + foreach (MemberPropertyModel property in model.MemberProperties + //ensure the property they are posting exists + .Where(p => memberType.PropertyTypeExists(p.Alias)) + .Where(property => member.Properties.Contains(property.Alias)) + //needs to be editable + .Where(p => memberType.MemberCanEditProperty(p.Alias))) + { + member.Properties[property.Alias].SetValue(property.Value); + } + } + + _memberService.Save(member); + + return saveResult; + } } } diff --git a/src/Umbraco.Web.Website/Controllers/UmbRegisterController.cs b/src/Umbraco.Web.Website/Controllers/UmbRegisterController.cs index 50a57fbfba..411668cf53 100644 --- a/src/Umbraco.Web.Website/Controllers/UmbRegisterController.cs +++ b/src/Umbraco.Web.Website/Controllers/UmbRegisterController.cs @@ -6,7 +6,6 @@ using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Logging; using Umbraco.Cms.Core.Models; -using Umbraco.Cms.Core.Models.Security; using Umbraco.Cms.Core.Routing; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; @@ -14,28 +13,32 @@ using Umbraco.Cms.Core.Web; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Web.Common.Filters; using Umbraco.Cms.Web.Common.Security; +using Umbraco.Cms.Web.Website.Models; using Umbraco.Extensions; namespace Umbraco.Cms.Web.Website.Controllers { public class UmbRegisterController : SurfaceController { - private readonly MemberManager _memberManager; + private readonly IMemberManager _memberManager; private readonly IMemberService _memberService; + private readonly IMemberSignInManager _memberSignInManager; public UmbRegisterController( - MemberManager memberManager, + IMemberManager memberManager, IMemberService memberService, IUmbracoContextAccessor umbracoContextAccessor, IUmbracoDatabaseFactory databaseFactory, ServiceContext services, AppCaches appCaches, IProfilingLogger profilingLogger, - IPublishedUrlProvider publishedUrlProvider) + IPublishedUrlProvider publishedUrlProvider, + IMemberSignInManager memberSignInManager) : base(umbracoContextAccessor, databaseFactory, services, appCaches, profilingLogger, publishedUrlProvider) { _memberManager = memberManager; _memberService = memberService; + _memberSignInManager = memberSignInManager; } [HttpPost] @@ -48,6 +51,8 @@ namespace Umbraco.Cms.Web.Website.Controllers return CurrentUmbracoPage(); } + MergeRouteValuesToModel(model); + // U4-10762 Server error with "Register Member" snippet (Cannot save member with empty name) // If name field is empty, add the email address instead. if (string.IsNullOrEmpty(model.Name) && string.IsNullOrEmpty(model.Email) == false) @@ -55,7 +60,7 @@ namespace Umbraco.Cms.Web.Website.Controllers model.Name = model.Email; } - IdentityResult result = await RegisterMemberAsync(model, model.LoginOnSuccess); + IdentityResult result = await RegisterMemberAsync(model, true); if (result.Succeeded) { TempData["FormSuccess"] = true; @@ -71,16 +76,38 @@ namespace Umbraco.Cms.Web.Website.Controllers } else { - AddModelErrors(result, "registerModel"); + AddErrors(result); return CurrentUmbracoPage(); } } - private void AddModelErrors(IdentityResult result, string prefix = "") + /// + /// We pass in values via encrypted route values so they cannot be tampered with and merge them into the model for use + /// + /// + private void MergeRouteValuesToModel(RegisterModel model) { - foreach (IdentityError error in result.Errors) + if (RouteData.Values.TryGetValue(nameof(RegisterModel.RedirectUrl), out var redirectUrl) && redirectUrl != null) { - ModelState.AddModelError(prefix, error.Description); + model.RedirectUrl = redirectUrl.ToString(); + } + + if (RouteData.Values.TryGetValue(nameof(RegisterModel.MemberTypeAlias), out var memberTypeAlias) && memberTypeAlias != null) + { + model.MemberTypeAlias = memberTypeAlias.ToString(); + } + + if (RouteData.Values.TryGetValue(nameof(RegisterModel.UsernameIsEmail), out var usernameIsEmail) && usernameIsEmail != null) + { + model.UsernameIsEmail = usernameIsEmail.ToString() == "True"; + } + } + + private void AddErrors(IdentityResult result) + { + foreach (var error in result.Errors) + { + ModelState.AddModelError("registerModel", error.Description); } } @@ -94,7 +121,7 @@ namespace Umbraco.Cms.Web.Website.Controllers { model.Username = (model.UsernameIsEmail || model.Username == null) ? model.Email : model.Username; - var identityUser = MembersIdentityUser.CreateNew(model.Username, model.Email, model.MemberTypeAlias, model.Name); + var identityUser = MemberIdentityUser.CreateNew(model.Username, model.Email, model.MemberTypeAlias, model.Name); IdentityResult identityResult = await _memberManager.CreateAsync( identityUser, model.Password); @@ -103,10 +130,15 @@ namespace Umbraco.Cms.Web.Website.Controllers { // Update the custom properties // TODO: See TODO in MembersIdentityUser, Should we support custom member properties for persistence/retrieval? - IMember member = _memberService.GetByUsername(identityUser.UserName); + IMember member = _memberService.GetByKey(identityUser.Key); + if (member == null) + { + // should never happen + throw new InvalidOperationException($"Could not find a member with key: {member.Key}."); + } if (model.MemberProperties != null) { - foreach (UmbracoProperty property in model.MemberProperties.Where(p => p.Value != null) + foreach (MemberPropertyModel property in model.MemberProperties.Where(p => p.Value != null) .Where(property => member.Properties.Contains(property.Alias))) { member.Properties[property.Alias].SetValue(property.Value); @@ -116,8 +148,7 @@ namespace Umbraco.Cms.Web.Website.Controllers if (logMemberIn) { - // TODO: Log them in - throw new NotImplementedException("Implement MemberSignInManager"); + await _memberSignInManager.SignInAsync(identityUser, false); } } diff --git a/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs index 58074ac9a5..1bdad575c0 100644 --- a/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs @@ -8,6 +8,7 @@ using Umbraco.Cms.Web.Common.Routing; using Umbraco.Cms.Web.Common.Security; using Umbraco.Cms.Web.Website.Collections; using Umbraco.Cms.Web.Website.Controllers; +using Umbraco.Cms.Web.Website.Models; using Umbraco.Cms.Web.Website.Routing; using Umbraco.Cms.Web.Website.ViewEngines; @@ -35,6 +36,7 @@ namespace Umbraco.Extensions // TODO figure out if we need more to work on load balanced setups builder.Services.AddDataProtection(); + builder.Services.AddAntiforgery(); builder.Services.AddScoped(); builder.Services.AddSingleton(); @@ -42,7 +44,9 @@ namespace Umbraco.Extensions builder.Services.AddSingleton(); builder.Services.AddSingleton(); - builder.Services.AddSingleton(); + builder.Services.AddSingleton(); + + builder.Services.AddSingleton(); builder .AddDistributedCache() diff --git a/src/Umbraco.Web.Website/Models/MemberModelBuilderBase.cs b/src/Umbraco.Web.Website/Models/MemberModelBuilderBase.cs new file mode 100644 index 0000000000..6bfeeb8f73 --- /dev/null +++ b/src/Umbraco.Web.Website/Models/MemberModelBuilderBase.cs @@ -0,0 +1,86 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Strings; + +namespace Umbraco.Cms.Web.Website.Models +{ + public abstract class MemberModelBuilderBase + { + private readonly IShortStringHelper _shortStringHelper; + + public MemberModelBuilderBase(IMemberTypeService memberTypeService, IShortStringHelper shortStringHelper) + { + MemberTypeService = memberTypeService; + _shortStringHelper = shortStringHelper; + } + + public IMemberTypeService MemberTypeService { get; } + + protected List GetMemberPropertiesViewModel(IMemberType memberType, IMember member = null) + { + var viewProperties = new List(); + + var builtIns = ConventionsHelper.GetStandardPropertyTypeStubs(_shortStringHelper).Select(x => x.Key).ToArray(); + + IOrderedEnumerable propertyTypes = memberType.PropertyTypes + .Where(x => builtIns.Contains(x.Alias) == false && memberType.MemberCanEditProperty(x.Alias)) + .OrderBy(p => p.SortOrder); + + foreach (IPropertyType prop in propertyTypes) + { + var value = string.Empty; + if (member != null) + { + IProperty propValue = member.Properties[prop.Alias]; + if (propValue != null && propValue.GetValue() != null) + { + value = propValue.GetValue().ToString(); + } + } + + var viewProperty = new MemberPropertyModel + { + Alias = prop.Alias, + Name = prop.Name, + Value = value + }; + + // TODO: Perhaps one day we'll ship with our own EditorTempates but for now developers + // can just render their own. + + ////This is a rudimentary check to see what data template we should render + //// if developers want to change the template they can do so dynamically in their views or controllers + //// for a given property. + ////These are the default built-in MVC template types: “Boolean”, “Decimal”, “EmailAddress”, “HiddenInput”, “HTML”, “Object”, “String”, “Text”, and “Url” + //// by default we'll render a text box since we've defined that metadata on the UmbracoProperty.Value property directly. + //if (prop.DataTypeId == new Guid(Constants.PropertyEditors.TrueFalse)) + //{ + // viewProperty.EditorTemplate = "UmbracoBoolean"; + //} + //else + //{ + // switch (prop.DataTypeDatabaseType) + // { + // case DataTypeDatabaseType.Integer: + // viewProperty.EditorTemplate = "Decimal"; + // break; + // case DataTypeDatabaseType.Ntext: + // viewProperty.EditorTemplate = "Text"; + // break; + // case DataTypeDatabaseType.Date: + // case DataTypeDatabaseType.Nvarchar: + // break; + // } + //} + + viewProperties.Add(viewProperty); + } + + return viewProperties; + } + } +} diff --git a/src/Umbraco.Web.Website/Models/MemberModelBuilderFactory.cs b/src/Umbraco.Web.Website/Models/MemberModelBuilderFactory.cs new file mode 100644 index 0000000000..df4725cd26 --- /dev/null +++ b/src/Umbraco.Web.Website/Models/MemberModelBuilderFactory.cs @@ -0,0 +1,37 @@ +using Microsoft.AspNetCore.Http; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Strings; + +namespace Umbraco.Cms.Web.Website.Models +{ + /// + /// Service to create model builder instances for working with Members on the front-end + /// + public class MemberModelBuilderFactory + { + private readonly IMemberTypeService _memberTypeService; + private readonly IMemberService _memberService; + private readonly IShortStringHelper _shortStringHelper; + private readonly IHttpContextAccessor _httpContextAccessor; + + public MemberModelBuilderFactory(IMemberTypeService memberTypeService, IMemberService memberService, IShortStringHelper shortStringHelper, IHttpContextAccessor httpContextAccessor) + { + _memberTypeService = memberTypeService; + _memberService = memberService; + _shortStringHelper = shortStringHelper; + _httpContextAccessor = httpContextAccessor; + } + + /// + /// Create a + /// + /// + public RegisterModelBuilder CreateRegisterModel() => new RegisterModelBuilder(_memberTypeService, _shortStringHelper); + + /// + /// Create a + /// + /// + public ProfileModelBuilder CreateProfileModel() => new ProfileModelBuilder(_memberTypeService, _memberService, _shortStringHelper, _httpContextAccessor); + } +} diff --git a/src/Umbraco.Core/Models/Security/ProfileModel.cs b/src/Umbraco.Web.Website/Models/ProfileModel.cs similarity index 53% rename from src/Umbraco.Core/Models/Security/ProfileModel.cs rename to src/Umbraco.Web.Website/Models/ProfileModel.cs index 2a495b1264..a435d22c06 100644 --- a/src/Umbraco.Core/Models/Security/ProfileModel.cs +++ b/src/Umbraco.Web.Website/Models/ProfileModel.cs @@ -1,9 +1,11 @@ -using System; +using System; using System.Collections.Generic; using System.ComponentModel; using System.ComponentModel.DataAnnotations; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Web.Common.Models; -namespace Umbraco.Cms.Core.Models.Security +namespace Umbraco.Cms.Web.Website.Models { /// /// A readonly member profile model @@ -11,8 +13,8 @@ namespace Umbraco.Cms.Core.Models.Security public class ProfileModel : PostRedirectModel { [Required] - [RegularExpression(@"[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?", - ErrorMessage = "Please enter a valid e-mail address")] + [EmailAddress] + [Display(Name = "Email")] public string Email { get; set; } /// @@ -20,17 +22,11 @@ namespace Umbraco.Cms.Core.Models.Security /// public string Name { get; set; } - /// - /// The member's member type alias - /// - [ReadOnly(true)] - public string MemberTypeAlias { get; set; } - [ReadOnly(true)] public string UserName { get; set; } [ReadOnly(true)] - public string Comment { get; set; } + public string Comments { get; set; } [ReadOnly(true)] public bool IsApproved { get; set; } @@ -39,19 +35,16 @@ namespace Umbraco.Cms.Core.Models.Security public bool IsLockedOut { get; set; } [ReadOnly(true)] - public DateTime LastLockoutDate { get; set; } + public DateTime? LastLockoutDate { get; set; } [ReadOnly(true)] - public DateTime CreationDate { get; set; } + public DateTime CreatedDate { get; set; } [ReadOnly(true)] - public DateTime LastLoginDate { get; set; } + public DateTime? LastLoginDate { get; set; } [ReadOnly(true)] - public DateTime LastActivityDate { get; set; } - - [ReadOnly(true)] - public DateTime LastPasswordChangedDate { get; set; } + public DateTime? LastPasswordChangedDate { get; set; } /// /// The list of member properties @@ -59,6 +52,6 @@ namespace Umbraco.Cms.Core.Models.Security /// /// Adding items to this list on the front-end will not add properties to the member in the database. /// - public List MemberProperties { get; set; } = new List(); + public List MemberProperties { get; set; } = new List(); } } diff --git a/src/Umbraco.Web.Website/Models/ProfileModelBuilder.cs b/src/Umbraco.Web.Website/Models/ProfileModelBuilder.cs new file mode 100644 index 0000000000..359d32704f --- /dev/null +++ b/src/Umbraco.Web.Website/Models/ProfileModelBuilder.cs @@ -0,0 +1,86 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Strings; +using Umbraco.Cms.Web.Common.Security; + +namespace Umbraco.Cms.Web.Website.Models +{ + public class ProfileModelBuilder : MemberModelBuilderBase + { + private readonly IMemberService _memberService; + private readonly IHttpContextAccessor _httpContextAccessor; + private string _redirectUrl; + + public ProfileModelBuilder( + IMemberTypeService memberTypeService, + IMemberService memberService, + IShortStringHelper shortStringHelper, + IHttpContextAccessor httpContextAccessor) + : base(memberTypeService, shortStringHelper) + { + _memberService = memberService; + _httpContextAccessor = httpContextAccessor; + } + + public ProfileModelBuilder WithRedirectUrl(string redirectUrl) + { + _redirectUrl = redirectUrl; + return this; + } + + public async Task BuildForCurrentMemberAsync() + { + IMemberManager memberManager = _httpContextAccessor.HttpContext?.RequestServices.GetRequiredService(); + + if (memberManager == null) + { + return null; + } + + MemberIdentityUser member = await memberManager.GetUserAsync(_httpContextAccessor.HttpContext.User); + + if (member == null) + { + return null; + } + + var model = new ProfileModel + { + Name = member.Name, + Email = member.Email, + UserName = member.UserName, + Comments = member.Comments, + IsApproved = member.IsApproved, + IsLockedOut = member.IsLockedOut, + LastLockoutDate = member.LastLockoutDateUtc?.ToLocalTime(), + CreatedDate = member.CreatedDateUtc.ToLocalTime(), + LastLoginDate = member.LastLoginDateUtc?.ToLocalTime(), + LastPasswordChangedDate = member.LastPasswordChangeDateUtc?.ToLocalTime(), + RedirectUrl = _redirectUrl + }; + + IMemberType memberType = MemberTypeService.Get(member.MemberTypeAlias); + if (memberType == null) + { + throw new InvalidOperationException($"Could not find a member type with alias: {member.MemberTypeAlias}."); + } + + // TODO: This wouldn't be required if we support exposing custom member properties on the MemberIdentityUser at the ASP.NET Identity level. + IMember persistedMember = _memberService.GetByKey(member.Key); + if (persistedMember == null) + { + // should never happen + throw new InvalidOperationException($"Could not find a member with key: {member.Key}."); + } + + model.MemberProperties = GetMemberPropertiesViewModel(memberType, persistedMember); + + return model; + } + } +} diff --git a/src/Umbraco.Core/Models/Security/RegisterModel.cs b/src/Umbraco.Web.Website/Models/RegisterModel.cs similarity index 51% rename from src/Umbraco.Core/Models/Security/RegisterModel.cs rename to src/Umbraco.Web.Website/Models/RegisterModel.cs index 0cfb249fe0..d949660539 100644 --- a/src/Umbraco.Core/Models/Security/RegisterModel.cs +++ b/src/Umbraco.Web.Website/Models/RegisterModel.cs @@ -1,37 +1,30 @@ -using System.Collections.Generic; +using System.Collections.Generic; using System.ComponentModel.DataAnnotations; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Web.Common.Models; -namespace Umbraco.Cms.Core.Models.Security +namespace Umbraco.Cms.Web.Website.Models { + public class RegisterModel : PostRedirectModel { - /// - /// Creates a new empty RegisterModel. - /// - /// - public static RegisterModel CreateModel() - { - return new RegisterModel(); - } - - private RegisterModel() + public RegisterModel() { MemberTypeAlias = Constants.Conventions.MemberTypes.DefaultAlias; UsernameIsEmail = true; - MemberProperties = new List(); - LoginOnSuccess = true; - CreatePersistentLoginCookie = true; + MemberProperties = new List(); } [Required] - [RegularExpression(@"[A-Za-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[A-Za-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?\.)+[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?", - ErrorMessage = "Please enter a valid e-mail address")] + [EmailAddress] + [Display(Name = "Email")] public string Email { get; set; } /// /// Returns the member properties /// - public List MemberProperties { get; set; } + public List MemberProperties { get; set; } /// /// The member type alias to use to register the member @@ -48,8 +41,16 @@ namespace Umbraco.Cms.Core.Models.Security /// The members password /// [Required] + [StringLength(256)] + [DataType(System.ComponentModel.DataAnnotations.DataType.Password)] + [Display(Name = "Password")] public string Password { get; set; } + [DataType(System.ComponentModel.DataAnnotations.DataType.Password)] + [Display(Name = "Confirm password")] + [Compare("Password", ErrorMessage = "The password and confirmation password do not match.")] + public string ConfirmPassword { get; set; } + /// /// The username of the model, if UsernameIsEmail is true then this is ignored. /// @@ -59,15 +60,5 @@ namespace Umbraco.Cms.Core.Models.Security /// Flag to determine if the username should be the email address, if true then the Username property is ignored /// public bool UsernameIsEmail { get; set; } - - /// - /// Specifies if the member should be logged in if they are successfully created - /// - public bool LoginOnSuccess { get; set; } - - /// - /// Default is true to create a persistent cookie if LoginOnSuccess is true - /// - public bool CreatePersistentLoginCookie { get; set; } } } diff --git a/src/Umbraco.Web.Website/Models/RegisterModelBuilder.cs b/src/Umbraco.Web.Website/Models/RegisterModelBuilder.cs new file mode 100644 index 0000000000..fbba8dd533 --- /dev/null +++ b/src/Umbraco.Web.Website/Models/RegisterModelBuilder.cs @@ -0,0 +1,67 @@ +using System; +using System.Linq; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Strings; + +namespace Umbraco.Cms.Web.Website.Models +{ + + /// + /// Builds a for use on the front-end + /// + public class RegisterModelBuilder : MemberModelBuilderBase + { + private string _memberTypeAlias; + private bool _lookupProperties; + private bool _usernameIsEmail; + private string _redirectUrl; + + public RegisterModelBuilder(IMemberTypeService memberTypeService, IShortStringHelper shortStringHelper) + : base(memberTypeService, shortStringHelper) + { + } + + public RegisterModelBuilder WithRedirectUrl(string redirectUrl) + { + _redirectUrl = redirectUrl; + return this; + } + + public RegisterModelBuilder UsernameIsEmail(bool usernameIsEmail = true) + { + _usernameIsEmail = usernameIsEmail; + return this; + } + + public RegisterModelBuilder WithMemberTypeAlias(string memberTypeAlias) + { + _memberTypeAlias = memberTypeAlias; + return this; + } + + public RegisterModelBuilder LookupProperties(bool lookupProperties) + { + _lookupProperties = lookupProperties; + return this; + } + + public RegisterModel Build() + { + var providedOrDefaultMemberTypeAlias = _memberTypeAlias ?? Core.Constants.Conventions.MemberTypes.DefaultAlias; + IMemberType memberType = MemberTypeService.Get(providedOrDefaultMemberTypeAlias); + if (memberType == null) + { + throw new InvalidOperationException($"Could not find a member type with alias: {providedOrDefaultMemberTypeAlias}."); + } + + var model = new RegisterModel + { + MemberTypeAlias = providedOrDefaultMemberTypeAlias, + UsernameIsEmail = _usernameIsEmail, + MemberProperties = _lookupProperties ? GetMemberPropertiesViewModel(memberType) : Enumerable.Empty().ToList() + }; + return model; + } + } +} diff --git a/src/Umbraco.Web/Models/Membership/UmbracoMembershipMember.cs b/src/Umbraco.Web/Models/Membership/UmbracoMembershipMember.cs index e85ac39b86..0d5e169cec 100644 --- a/src/Umbraco.Web/Models/Membership/UmbracoMembershipMember.cs +++ b/src/Umbraco.Web/Models/Membership/UmbracoMembershipMember.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Web.Security; using Umbraco.Cms.Core.Models.Membership; @@ -19,7 +19,6 @@ namespace Umbraco.Web.Models.Membership private string _email; private string _comment; private bool _isApproved; - private DateTime _lastActivityDate; //NOTE: We are only overriding the properties that matter, we don't override things like IsOnline since that is handled with the sub-class and the membership providers. @@ -41,8 +40,6 @@ namespace Umbraco.Web.Models.Membership _isLockedOut = member.IsLockedOut; _creationDate = member.CreateDate.ToUniversalTime(); _lastLoginDate = member.LastLoginDate.ToUniversalTime(); - // TODO: We currently don't really have any place to store this data!! - _lastActivityDate = member.LastLoginDate.ToUniversalTime(); _lastPasswordChangedDate = member.LastPasswordChangeDate.ToUniversalTime(); _lastLockoutDate = member.LastLockoutDate.ToUniversalTime(); } @@ -103,12 +100,6 @@ namespace Umbraco.Web.Models.Membership set { _lastLoginDate = value; } } - public override DateTime LastActivityDate - { - get { return _lastActivityDate; } - set { _lastActivityDate = value; } - } - public override DateTime LastPasswordChangedDate { get { return _lastPasswordChangedDate; } diff --git a/src/Umbraco.Web/Security/MembershipHelper.cs b/src/Umbraco.Web/Security/MembershipHelper.cs index 4f48b5cc80..e93fcbdfd0 100644 --- a/src/Umbraco.Web/Security/MembershipHelper.cs +++ b/src/Umbraco.Web/Security/MembershipHelper.cs @@ -8,13 +8,11 @@ using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.PublishedContent; -using Umbraco.Cms.Core.Models.Security; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Strings; using Umbraco.Extensions; using Umbraco.Web.Security.Providers; -using Constants = Umbraco.Cms.Core.Constants; namespace Umbraco.Web.Security { @@ -167,175 +165,8 @@ namespace Umbraco.Web.Security return result; } - /// - /// Returns true if the current membership provider is the Umbraco built-in one. - /// - /// - public bool IsUmbracoMembershipProviderActive() - { - var provider = _membershipProvider; - return provider.IsUmbracoMembershipProvider(); - } - - /// - /// Updates the currently logged in members profile - /// - /// - /// - /// The updated MembershipUser object - /// - public virtual Attempt UpdateMemberProfile(ProfileModel model) - { - if (IsLoggedIn() == false) - { - throw new NotSupportedException("No member is currently logged in"); - } - - //get the current membership user - 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); - - try - { - //check if the email needs to change - if (model.Email.InvariantEquals(membershipUser.Email) == false) - { - //Use the membership provider to change the email since that is configured to do the checks to check for unique emails if that is configured. - var requiresUpdating = UpdateMember(membershipUser, provider, model.Email); - membershipUser = requiresUpdating.Result; - } - } - catch (Exception ex) - { - //This will occur if an email already exists! - return Attempt.Fail(ex); - } - - var member = GetCurrentPersistedMember(); - - //NOTE: If changing the username is a requirement, than that needs to be done via the IMember directly since MembershipProvider's natively do - // not support changing a username! - if (model.Name != null && member.Name != model.Name) - { - member.Name = model.Name; - } - - var memberType = _memberTypeService.Get(member.ContentTypeId); - - if (model.MemberProperties != null) - { - foreach (var property in model.MemberProperties - //ensure the property they are posting exists - .Where(p => memberType.PropertyTypeExists(p.Alias)) - .Where(property => member.Properties.Contains(property.Alias)) - //needs to be editable - .Where(p => memberType.MemberCanEditProperty(p.Alias))) - { - member.Properties[property.Alias].SetValue(property.Value); - } - } - - _memberService.Save(member); - - //reset the FormsAuth cookie since the username might have changed - FormsAuthentication.SetAuthCookie(member.Username, true); - - return Attempt.Succeed(membershipUser); - } - - /// - /// Registers a new member - /// - /// - /// - /// - /// true to log the member in upon successful registration - /// - /// - public virtual MembershipUser RegisterMember(RegisterModel model, out MembershipCreateStatus status, bool logMemberIn = true) - { - model.Username = (model.UsernameIsEmail || model.Username == null) ? model.Email : model.Username; - - MembershipUser membershipUser; - var provider = _membershipProvider; - membershipUser = ((UmbracoMembershipProviderBase)provider).CreateUser( - model.MemberTypeAlias, - model.Username, model.Password, model.Email, - // TODO: Support q/a http://issues.umbraco.org/issue/U4-3213 - null, null, - true, null, out status); - - if (status != MembershipCreateStatus.Success) - return null; - - var member = _memberService.GetByUsername(membershipUser.UserName); - member.Name = model.Name; - - if (model.MemberProperties != null) - { - foreach (var property in model.MemberProperties.Where(p => p.Value != null) - .Where(property => member.Properties.Contains(property.Alias))) - { - member.Properties[property.Alias].SetValue(property.Value); - } - } - - _memberService.Save(member); - - if (logMemberIn) - { - //Set member online - provider.GetUser(model.Username, true); - - //Log them in - FormsAuthentication.SetAuthCookie(membershipUser.UserName, model.CreatePersistentLoginCookie); - } - - return membershipUser; - } - - /// - /// A helper method to perform the validation and logging in of a member - this is simply wrapping standard membership provider and asp.net forms auth logic. - /// - /// - /// - /// - public virtual bool Login(string username, string password) - { - var provider = _membershipProvider; - //Validate credentials - if (provider.ValidateUser(username, password) == false) - { - return false; - } - // Get the member, do not set to online - this is done implicitly as part of ValidateUser which is consistent with - // how the .NET framework SqlMembershipProvider works. Passing in true will just cause more unnecessary SQL queries/locks. - var member = provider.GetUser(username, false); - if (member == null) - { - //this should not happen - _logger.LogWarning("The member validated but then no member was returned with the username {Username}", username); - return false; - } - //Log them in - FormsAuthentication.SetAuthCookie(member.UserName, true); - return true; - } #region Querying for front-end - - /// - /// Logs out the current member - /// - public virtual void Logout() - { - FormsAuthentication.SignOut(); - } - - public virtual IPublishedContent GetByProviderKey(object key) { return MemberCache.GetByProviderKey(key); @@ -411,150 +242,6 @@ namespace Umbraco.Web.Security return result == null ? null : MemberCache.GetByMember(result); } - /// - /// Returns the currently logged in member id, -1 if they are not logged in - /// - /// - public int GetCurrentMemberId() - { - if (IsLoggedIn() == false) - { - return -1; - } - var result = GetCurrentMember(); - return result?.Id ?? -1; - } - - #endregion - - #region Model Creation methods for member data editing on the front-end - /// - /// Creates a new profile model filled in with the current members details if they are logged in which allows for editing - /// profile properties - /// - /// - public virtual ProfileModel GetCurrentMemberProfileModel() - { - if (IsLoggedIn() == false) - { - return null; - } - - var provider = _membershipProvider; - - var membershipUser = provider.GetCurrentUserOnline(); - var member = GetCurrentPersistedMember(); - //this shouldn't happen but will if the member is deleted in the back office while the member is trying - // to use the front-end! - if (member == null) - { - //log them out since they've been removed - FormsAuthentication.SignOut(); - - return null; - } - - var model = new ProfileModel(); - model.Name = member.Name; - model.MemberTypeAlias = member.ContentTypeAlias; - - model.Email = membershipUser.Email; - model.UserName = membershipUser.UserName; - model.Comment = membershipUser.Comment; - model.IsApproved = membershipUser.IsApproved; - model.IsLockedOut = membershipUser.IsLockedOut; - model.LastLockoutDate = membershipUser.LastLockoutDate; - model.CreationDate = membershipUser.CreationDate; - model.LastLoginDate = membershipUser.LastLoginDate; - model.LastActivityDate = membershipUser.LastActivityDate; - model.LastPasswordChangedDate = membershipUser.LastPasswordChangedDate; - - var memberType = _memberTypeService.Get(member.ContentTypeId); - - var builtIns = ConventionsHelper.GetStandardPropertyTypeStubs(_shortStringHelper).Select(x => x.Key).ToArray(); - - model.MemberProperties = GetMemberPropertiesViewModel(memberType, builtIns, member).ToList(); - - return model; - } - - /// - /// Creates a model to use for registering new members with custom member properties - /// - /// - /// - public virtual RegisterModel CreateRegistrationModel(string memberTypeAlias = null) - { - var provider = _membershipProvider; - memberTypeAlias = memberTypeAlias ?? Constants.Conventions.MemberTypes.DefaultAlias; - var memberType = _memberTypeService.Get(memberTypeAlias); - if (memberType == null) - throw new InvalidOperationException("Could not find a member type with alias " + memberTypeAlias); - - var builtIns = ConventionsHelper.GetStandardPropertyTypeStubs(_shortStringHelper).Select(x => x.Key).ToArray(); - var model = RegisterModel.CreateModel(); - model.MemberTypeAlias = memberTypeAlias; - model.MemberProperties = GetMemberPropertiesViewModel(memberType, builtIns).ToList(); - return model; - } - - private IEnumerable GetMemberPropertiesViewModel(IMemberType memberType, IEnumerable builtIns, IMember member = null) - { - var viewProperties = new List(); - - foreach (var prop in memberType.PropertyTypes - .Where(x => builtIns.Contains(x.Alias) == false && memberType.MemberCanEditProperty(x.Alias)) - .OrderBy(p => p.SortOrder)) - { - var value = string.Empty; - if (member != null) - { - var propValue = member.Properties[prop.Alias]; - if (propValue != null && propValue.GetValue() != null) - { - value = propValue.GetValue().ToString(); - } - } - - var viewProperty = new UmbracoProperty - { - Alias = prop.Alias, - Name = prop.Name, - Value = value - }; - - // TODO: Perhaps one day we'll ship with our own EditorTempates but for now developers - // can just render their own. - - ////This is a rudimentary check to see what data template we should render - //// if developers want to change the template they can do so dynamically in their views or controllers - //// for a given property. - ////These are the default built-in MVC template types: “Boolean”, “Decimal”, “EmailAddress”, “HiddenInput”, “HTML”, “Object”, “String”, “Text”, and “Url” - //// by default we'll render a text box since we've defined that metadata on the UmbracoProperty.Value property directly. - //if (prop.DataTypeId == new Guid(Constants.PropertyEditors.TrueFalse)) - //{ - // viewProperty.EditorTemplate = "UmbracoBoolean"; - //} - //else - //{ - // switch (prop.DataTypeDatabaseType) - // { - // case DataTypeDatabaseType.Integer: - // viewProperty.EditorTemplate = "Decimal"; - // break; - // case DataTypeDatabaseType.Ntext: - // viewProperty.EditorTemplate = "Text"; - // break; - // case DataTypeDatabaseType.Date: - // case DataTypeDatabaseType.Nvarchar: - // break; - // } - //} - - viewProperties.Add(viewProperty); - } - return viewProperties; - } #endregion /// @@ -575,40 +262,6 @@ namespace Umbraco.Web.Security return _appCaches.RequestCache.GetCacheItem(key, () => Roles.Provider.GetRolesForUser(userName)); } - /// - /// Returns the login status model of the currently logged in member. - /// - /// - public virtual LoginStatusModel GetCurrentLoginStatus() - { - var model = LoginStatusModel.CreateModel(); - - if (IsLoggedIn() == false) - { - model.IsLoggedIn = false; - return model; - } - - var provider = _membershipProvider; - - var member = GetCurrentPersistedMember(); - //this shouldn't happen but will if the member is deleted in the back office while the member is trying - // to use the front-end! - if (member == null) - { - //log them out since they've been removed - FormsAuthentication.SignOut(); - model.IsLoggedIn = false; - return model; - } - model.Name = member.Name; - model.Username = member.Username; - model.Email = member.Email; - - model.IsLoggedIn = true; - return model; - } - /// /// Check if a member is logged in /// @@ -691,66 +344,6 @@ namespace Umbraco.Web.Security return allowAction; } - /// - /// Updates a membership user with all of it's writable properties - /// - /// - /// - /// - /// - /// - /// - /// - /// - /// Returns successful if the membership user required updating, otherwise returns failed if it didn't require updating. - /// - internal Attempt UpdateMember(MembershipUser member, MembershipProvider provider, - string email = null, - bool? isApproved = null, - DateTime? lastLoginDate = null, - DateTime? lastActivityDate = null, - string comment = null) - { - var update = false; - - if (email != null) - { - if (member.Email != email) - update = true; - member.Email = email; - } - if (isApproved.HasValue) - { - if (member.IsApproved != isApproved.Value) - update = true; - member.IsApproved = isApproved.Value; - } - if (lastLoginDate.HasValue) - { - if (member.LastLoginDate != lastLoginDate.Value) - update = true; - member.LastLoginDate = lastLoginDate.Value; - } - if (lastActivityDate.HasValue) - { - if (member.LastActivityDate != lastActivityDate.Value) - update = true; - member.LastActivityDate = lastActivityDate.Value; - } - if (comment != null) - { - if (member.Comment != comment) - update = true; - member.Comment = comment; - } - - if (update == false) - return Attempt.Fail(member); - - provider.UpdateUser(member); - return Attempt.Succeed(member); - } - /// /// Returns the currently logged in IMember object - this should never be exposed to the front-end since it's returning a business logic entity! /// @@ -765,65 +358,5 @@ namespace Umbraco.Web.Security return member; } - /// - /// Changes password for a member/user given the membership provider and the password change model - /// - /// The username of the user having their password changed - /// - /// - /// - private Attempt ChangePasswordWithMembershipProvider( - string username, - ChangingPasswordModel passwordModel, - MembershipProvider membershipProvider) - { - var umbracoBaseProvider = membershipProvider as MembershipProviderBase; - - // 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)); - var userId = -1; - - - //we're not resetting it so we need to try to change it. - - if (passwordModel.NewPassword.IsNullOrWhiteSpace()) - { - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Cannot set an empty password", new[] { "value" }) }); - } - - if (membershipProvider.EnablePasswordRetrieval) - { - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Membership providers using encrypted passwords and password retrieval are not supported", new[] { "value" }) }); - } - - //without being able to retrieve the original password - if (passwordModel.OldPassword.IsNullOrWhiteSpace()) - { - //if password retrieval is not enabled but there is no old password we cannot continue - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Password cannot be changed without the old password", new[] { "oldPassword" }) }); - } - - //if an old password is supplied try to change it - - try - { - var result = membershipProvider.ChangePassword(username, passwordModel.OldPassword, passwordModel.NewPassword); - - return result == false - ? Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, invalid username or password", new[] { "oldPassword" }) }) - : Attempt.Succeed(new PasswordChangedModel()); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Could not change member password"); - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, error: " + ex.Message + " (see log for full details)", new[] { "value" }) }); - } - - } - } }