diff --git a/src/Umbraco.Abstractions/Configuration/IMemberPasswordConfiguration.cs b/src/Umbraco.Abstractions/Configuration/IMemberPasswordConfiguration.cs new file mode 100644 index 0000000000..a7c956a337 --- /dev/null +++ b/src/Umbraco.Abstractions/Configuration/IMemberPasswordConfiguration.cs @@ -0,0 +1,9 @@ +namespace Umbraco.Core.Configuration +{ + /// + /// The password configuration for members + /// + public interface IMemberPasswordConfiguration : IPasswordConfiguration + { + } +} diff --git a/src/Umbraco.Abstractions/Configuration/IUserPasswordConfiguration.cs b/src/Umbraco.Abstractions/Configuration/IUserPasswordConfiguration.cs index 04af69c68c..ca9a7f3271 100644 --- a/src/Umbraco.Abstractions/Configuration/IUserPasswordConfiguration.cs +++ b/src/Umbraco.Abstractions/Configuration/IUserPasswordConfiguration.cs @@ -5,6 +5,5 @@ /// public interface IUserPasswordConfiguration : IPasswordConfiguration { - } } diff --git a/src/Umbraco.Abstractions/Security/IPasswordGenerator.cs b/src/Umbraco.Abstractions/Security/IPasswordGenerator.cs deleted file mode 100644 index 3977ec3cf6..0000000000 --- a/src/Umbraco.Abstractions/Security/IPasswordGenerator.cs +++ /dev/null @@ -1,17 +0,0 @@ -using Umbraco.Core.Configuration; - -namespace Umbraco.Core.Security -{ - /// - /// Generates a password - /// - public interface IPasswordGenerator - { - /// - /// Generates a password - /// - /// - /// - string GeneratePassword(IPasswordConfiguration passwordConfiguration); - } -} diff --git a/src/Umbraco.Abstractions/Security/PasswordGenerator.cs b/src/Umbraco.Abstractions/Security/PasswordGenerator.cs index 2dca396384..d8e18a8146 100644 --- a/src/Umbraco.Abstractions/Security/PasswordGenerator.cs +++ b/src/Umbraco.Abstractions/Security/PasswordGenerator.cs @@ -11,28 +11,34 @@ namespace Umbraco.Core.Security /// /// This uses logic copied from the old MembershipProvider.GeneratePassword logic /// - public class PasswordGenerator : IPasswordGenerator + public class PasswordGenerator { - public string GeneratePassword(IPasswordConfiguration passwordConfiguration) + private readonly IPasswordConfiguration _passwordConfiguration; + + public PasswordGenerator(IPasswordConfiguration passwordConfiguration) + { + _passwordConfiguration = passwordConfiguration; + } + public string GeneratePassword() { var password = PasswordStore.GeneratePassword( - passwordConfiguration.RequiredLength, - passwordConfiguration.RequireNonLetterOrDigit ? 2 : 0); + _passwordConfiguration.RequiredLength, + _passwordConfiguration.RequireNonLetterOrDigit ? 2 : 0); var random = new Random(); var passwordChars = password.ToCharArray(); - if (passwordConfiguration.RequireDigit && passwordChars.ContainsAny(Enumerable.Range(48, 58).Select(x => (char)x))) + if (_passwordConfiguration.RequireDigit && passwordChars.ContainsAny(Enumerable.Range(48, 58).Select(x => (char)x))) password += Convert.ToChar(random.Next(48, 58)); // 0-9 - if (passwordConfiguration.RequireLowercase && passwordChars.ContainsAny(Enumerable.Range(97, 123).Select(x => (char)x))) + if (_passwordConfiguration.RequireLowercase && passwordChars.ContainsAny(Enumerable.Range(97, 123).Select(x => (char)x))) password += Convert.ToChar(random.Next(97, 123)); // a-z - if (passwordConfiguration.RequireUppercase && passwordChars.ContainsAny(Enumerable.Range(65, 91).Select(x => (char)x))) + if (_passwordConfiguration.RequireUppercase && passwordChars.ContainsAny(Enumerable.Range(65, 91).Select(x => (char)x))) password += Convert.ToChar(random.Next(65, 91)); // A-Z - if (passwordConfiguration.RequireNonLetterOrDigit && passwordChars.ContainsAny(Enumerable.Range(33, 48).Select(x => (char)x))) + if (_passwordConfiguration.RequireNonLetterOrDigit && passwordChars.ContainsAny(Enumerable.Range(33, 48).Select(x => (char)x))) password += Convert.ToChar(random.Next(33, 48)); // symbols !"#$%&'()*+,-./ return password; diff --git a/src/Umbraco.Configuration/ConfigsFactory.cs b/src/Umbraco.Configuration/ConfigsFactory.cs index cc68549412..ceef2f8f5a 100644 --- a/src/Umbraco.Configuration/ConfigsFactory.cs +++ b/src/Umbraco.Configuration/ConfigsFactory.cs @@ -25,7 +25,8 @@ namespace Umbraco.Core.Configuration configs.Add("umbracoConfiguration/settings"); configs.Add("umbracoConfiguration/HealthChecks"); - configs.Add(() => new DefaultUserPasswordConfig()); + configs.Add(() => new DefaultPasswordConfig()); + configs.Add(() => new DefaultPasswordConfig()); configs.Add(() => new CoreDebug()); configs.Add(() => new ConnectionStrings()); configs.AddCoreConfigs(_ioHelper); @@ -34,9 +35,10 @@ namespace Umbraco.Core.Configuration } // Default/static user password configs - // TODO: Make this configurable somewhere - we've removed membership providers, so could be a section in the umbracosettings.config file? - // keeping in mind that we will also be removing the members membership provider so there will be 2x the same/similar configuration - internal class DefaultUserPasswordConfig : IUserPasswordConfiguration + // TODO: Make this configurable somewhere - we've removed membership providers for users, so could be a section in the umbracosettings.config file? + // keeping in mind that we will also be removing the members membership provider so there will be 2x the same/similar configuration. + // TODO: Currently it doesn't actually seem possible to replace any sub-configuration unless totally replacing the IConfigsFactory?? + internal class DefaultPasswordConfig : IUserPasswordConfiguration, IMemberPasswordConfiguration { public int RequiredLength => 12; public bool RequireNonLetterOrDigit => false; diff --git a/src/Umbraco.Configuration/ConnectionStrings.cs b/src/Umbraco.Configuration/ConnectionStrings.cs index 1842ff6627..707f58c7b7 100644 --- a/src/Umbraco.Configuration/ConnectionStrings.cs +++ b/src/Umbraco.Configuration/ConnectionStrings.cs @@ -13,7 +13,7 @@ namespace Umbraco.Core.Configuration get { var settings = ConfigurationManager.ConnectionStrings[key]; - + if (settings == null) return null; return new ConfigConnectionString(settings.ConnectionString, settings.ProviderName, settings.Name); } } diff --git a/src/Umbraco.Core/Composing/Current.cs b/src/Umbraco.Core/Composing/Current.cs index e1d7c270ce..4eba166020 100644 --- a/src/Umbraco.Core/Composing/Current.cs +++ b/src/Umbraco.Core/Composing/Current.cs @@ -129,9 +129,6 @@ namespace Umbraco.Core.Composing public static IRuntimeState RuntimeState => Factory.GetInstance(); - public static IPasswordGenerator PasswordGenerator - => Factory.GetInstance(); - public static TypeLoader TypeLoader => Factory.GetInstance(); diff --git a/src/Umbraco.Core/Runtime/CoreInitialComposer.cs b/src/Umbraco.Core/Runtime/CoreInitialComposer.cs index 6a2a897999..9373f7b70c 100644 --- a/src/Umbraco.Core/Runtime/CoreInitialComposer.cs +++ b/src/Umbraco.Core/Runtime/CoreInitialComposer.cs @@ -136,7 +136,6 @@ namespace Umbraco.Core.Runtime composition.SetCultureDictionaryFactory(); composition.Register(f => f.GetInstance().CreateDictionary(), Lifetime.Singleton); - composition.RegisterUnique(); } } } diff --git a/src/Umbraco.Core/Security/IUmbracoMemberTypeMembershipProvider.cs b/src/Umbraco.Core/Security/IUmbracoMemberTypeMembershipProvider.cs deleted file mode 100644 index bd165334f1..0000000000 --- a/src/Umbraco.Core/Security/IUmbracoMemberTypeMembershipProvider.cs +++ /dev/null @@ -1,21 +0,0 @@ -using System.Collections.Specialized; - -namespace Umbraco.Core.Security -{ - /// - /// An interface for exposing the content type properties for storing membership data in when - /// a membership provider's data is backed by an Umbraco content type. - /// - public interface IUmbracoMemberTypeMembershipProvider - { - - string LockPropertyTypeAlias { get; } - string LastLockedOutPropertyTypeAlias { get; } - string FailedPasswordAttemptsPropertyTypeAlias { get; } - string ApprovedPropertyTypeAlias { get; } - string CommentPropertyTypeAlias { get; } - string LastLoginPropertyTypeAlias { get; } - string LastPasswordChangedPropertyTypeAlias { get; } - - } -} diff --git a/src/Umbraco.Core/Security/PasswordSecurity.cs b/src/Umbraco.Core/Security/PasswordSecurity.cs index 6ffcff67de..3e5d65dfd7 100644 --- a/src/Umbraco.Core/Security/PasswordSecurity.cs +++ b/src/Umbraco.Core/Security/PasswordSecurity.cs @@ -1,54 +1,96 @@ using System; +using System.Collections.Generic; using System.Security.Cryptography; using System.Text; +using System.Threading.Tasks; using Umbraco.Core.Configuration; namespace Umbraco.Core.Security { + /// + /// Handles password hashing and formatting + /// public class PasswordSecurity { - private readonly IPasswordConfiguration _passwordConfiguration; + public IPasswordConfiguration PasswordConfiguration { get; } + public PasswordGenerator _generator; + public ConfiguredPasswordValidator _validator; + /// + /// Constructor + /// + /// public PasswordSecurity(IPasswordConfiguration passwordConfiguration) { - _passwordConfiguration = passwordConfiguration; + PasswordConfiguration = passwordConfiguration; } + /// + /// Checks if the password passes validation rules + /// + /// + /// + public async Task>> IsValidPasswordAsync(string password) + { + if (_validator == null) + _validator = new ConfiguredPasswordValidator(PasswordConfiguration); + var result = await _validator.ValidateAsync(password); + if (result.Succeeded) + return Attempt>.Succeed(); + + return Attempt>.Fail(result.Errors); + } + + public string GeneratePassword() + { + if (_generator == null) + _generator = new PasswordGenerator(PasswordConfiguration); + return _generator.GeneratePassword(); + } + + /// + /// Returns a hashed password value used to store in a data store + /// + /// + /// public string HashPasswordForStorage(string password) { + if (string.IsNullOrWhiteSpace(password)) + throw new ArgumentException("password cannot be empty", nameof(password)); + string salt; var hashed = HashNewPassword(password, out salt); return FormatPasswordForStorage(hashed, salt); } - public bool VerifyPassword(string password, string hashedPassword) - { - if (string.IsNullOrWhiteSpace(hashedPassword)) throw new ArgumentException("Value cannot be null or whitespace.", "hashedPassword"); - return CheckPassword(password, hashedPassword); - } - /// /// If the password format is a hashed keyed algorithm then we will pre-pend the salt used to hash the password /// to the hashed password itself. /// + /// + /// + /// + public string FormatPasswordForStorage(string hashedPassword, string salt) + { + if (PasswordConfiguration.UseLegacyEncoding) + { + return hashedPassword; + } + + return salt + hashedPassword; + } + + /// + /// Hashes a password with a given salt + /// /// /// /// - public string FormatPasswordForStorage(string pass, string salt) - { - if (_passwordConfiguration.UseLegacyEncoding) - { - return pass; - } - - return salt + pass; - } - public string HashPassword(string pass, string salt) { //if we are doing it the old way - if (_passwordConfiguration.UseLegacyEncoding) + if (PasswordConfiguration.UseLegacyEncoding) { return LegacyEncodePassword(pass); } @@ -103,21 +145,25 @@ namespace Umbraco.Core.Security } /// - /// Checks the password. + /// Verifies if the password matches the expected hash+salt of the stored password string /// /// The password. - /// The dbPassword. + /// The value of the password stored in a data store. /// - public bool CheckPassword(string password, string dbPassword) + public bool VerifyPassword(string password, string dbPassword) { if (string.IsNullOrWhiteSpace(dbPassword)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(dbPassword)); - var storedHashedPass = StoredPassword(dbPassword, out var salt); + + if (dbPassword.StartsWith(Constants.Security.EmptyPasswordPrefix)) + return false; + + var storedHashedPass = ParseStoredHashPassword(dbPassword, out var salt); var hashed = HashPassword(password, salt); return storedHashedPass == hashed; } /// - /// hash a new password with a new salt + /// Create a new password hash and a new salt /// /// /// @@ -129,15 +175,15 @@ namespace Umbraco.Core.Security } /// - /// Returns the hashed password without the salt if it is hashed + /// Parses out the hashed password and the salt from the stored password string value /// /// /// returns the salt /// - public string StoredPassword(string storedString, out string salt) + public string ParseStoredHashPassword(string storedString, out string salt) { if (string.IsNullOrWhiteSpace(storedString)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(storedString)); - if (_passwordConfiguration.UseLegacyEncoding) + if (PasswordConfiguration.UseLegacyEncoding) { salt = string.Empty; return storedString; @@ -155,9 +201,14 @@ namespace Umbraco.Core.Security return Convert.ToBase64String(numArray); } + /// + /// Return the hash algorithm to use + /// + /// + /// public HashAlgorithm GetHashAlgorithm(string password) { - if (_passwordConfiguration.UseLegacyEncoding) + if (PasswordConfiguration.UseLegacyEncoding) { return new HMACSHA1 { @@ -166,12 +217,12 @@ namespace Umbraco.Core.Security }; } - if (_passwordConfiguration.HashAlgorithmType.IsNullOrWhiteSpace()) + if (PasswordConfiguration.HashAlgorithmType.IsNullOrWhiteSpace()) throw new InvalidOperationException("No hash algorithm type specified"); - var alg = HashAlgorithm.Create(_passwordConfiguration.HashAlgorithmType); + var alg = HashAlgorithm.Create(PasswordConfiguration.HashAlgorithmType); if (alg == null) - throw new InvalidOperationException($"The hash algorithm specified {_passwordConfiguration.HashAlgorithmType} cannot be resolved"); + throw new InvalidOperationException($"The hash algorithm specified {PasswordConfiguration.HashAlgorithmType} cannot be resolved"); return alg; } @@ -183,9 +234,8 @@ namespace Umbraco.Core.Security /// The encoded password. private string LegacyEncodePassword(string password) { - string encodedPassword = password; var hashAlgorith = GetHashAlgorithm(password); - encodedPassword = Convert.ToBase64String(hashAlgorith.ComputeHash(Encoding.Unicode.GetBytes(password))); + var encodedPassword = Convert.ToBase64String(hashAlgorith.ComputeHash(Encoding.Unicode.GetBytes(password))); return encodedPassword; } diff --git a/src/Umbraco.Core/Services/Implement/MemberService.cs b/src/Umbraco.Core/Services/Implement/MemberService.cs index a716cf57cc..8295d3ee55 100644 --- a/src/Umbraco.Core/Services/Implement/MemberService.cs +++ b/src/Umbraco.Core/Services/Implement/MemberService.cs @@ -1120,77 +1120,6 @@ namespace Umbraco.Core.Services.Implement #region Membership - /// - /// A helper method that will create a basic/generic member for use with a generic membership provider - /// - /// - internal static IMember CreateGenericMembershipProviderMember(string name, string email, string username, string password) - { - var identity = int.MaxValue; - - var memType = new MemberType(-1); - var propGroup = new PropertyGroup(MemberType.SupportsPublishingConst) - { - Name = "Membership", - Id = --identity - }; - propGroup.PropertyTypes.Add(new PropertyType(Constants.PropertyEditors.Aliases.TextBox, ValueStorageType.Ntext, Constants.Conventions.Member.Comments) - { - Name = Constants.Conventions.Member.CommentsLabel, - SortOrder = 0, - Id = --identity, - Key = identity.ToGuid() - }); - propGroup.PropertyTypes.Add(new PropertyType(Constants.PropertyEditors.Aliases.Boolean, ValueStorageType.Integer, Constants.Conventions.Member.IsApproved) - { - Name = Constants.Conventions.Member.IsApprovedLabel, - SortOrder = 3, - Id = --identity, - Key = identity.ToGuid() - }); - propGroup.PropertyTypes.Add(new PropertyType(Constants.PropertyEditors.Aliases.Boolean, ValueStorageType.Integer, Constants.Conventions.Member.IsLockedOut) - { - Name = Constants.Conventions.Member.IsLockedOutLabel, - SortOrder = 4, - Id = --identity, - Key = identity.ToGuid() - }); - propGroup.PropertyTypes.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Date, Constants.Conventions.Member.LastLockoutDate) - { - Name = Constants.Conventions.Member.LastLockoutDateLabel, - SortOrder = 5, - Id = --identity, - Key = identity.ToGuid() - }); - propGroup.PropertyTypes.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Date, Constants.Conventions.Member.LastLoginDate) - { - Name = Constants.Conventions.Member.LastLoginDateLabel, - SortOrder = 6, - Id = --identity, - Key = identity.ToGuid() - }); - propGroup.PropertyTypes.Add(new PropertyType(Constants.PropertyEditors.Aliases.Label, ValueStorageType.Date, Constants.Conventions.Member.LastPasswordChangeDate) - { - Name = Constants.Conventions.Member.LastPasswordChangeDateLabel, - SortOrder = 7, - Id = --identity, - Key = identity.ToGuid() - }); - - memType.PropertyGroups.Add(propGroup); - - // should we "create member"? - var member = new Member(name, email, username, password, memType); - - //we've assigned ids to the property types and groups but we also need to assign fake ids to the properties themselves. - foreach (var property in member.Properties) - { - property.Id = --identity; - } - - return member; - } - /// /// Exports a member. /// diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index b870a6a54e..4d3bf9ebc0 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -735,7 +735,6 @@ - diff --git a/src/Umbraco.Tests/Membership/MembershipProviderBaseTests.cs b/src/Umbraco.Tests/Membership/MembershipProviderBaseTests.cs index 7005bab984..1dc261714c 100644 --- a/src/Umbraco.Tests/Membership/MembershipProviderBaseTests.cs +++ b/src/Umbraco.Tests/Membership/MembershipProviderBaseTests.cs @@ -17,28 +17,6 @@ namespace Umbraco.Tests.Membership [UmbracoTest(WithApplication = true)] public class MembershipProviderBaseTests : UmbracoTestBase { - [Test] - public void Change_Password_Without_AllowManuallyChangingPassword_And_No_Pass_Validation() - { - var providerMock = new Mock(TestHelper.GetHostingEnvironment()) { CallBase = true }; - providerMock.Setup(@base => @base.AllowManuallyChangingPassword).Returns(false); - var provider = providerMock.Object; - - Assert.Throws(() => provider.ChangePassword("test", "", "test")); - } - - [Test] - public void Change_Password_With_AllowManuallyChangingPassword_And_Invalid_Creds() - { - var providerMock = new Mock(TestHelper.GetHostingEnvironment()) { CallBase = true }; - providerMock.Setup(@base => @base.AllowManuallyChangingPassword).Returns(false); - providerMock.Setup(@base => @base.ValidateUser("test", "test")).Returns(false); - var provider = providerMock.Object; - - Assert.IsFalse(provider.ChangePassword("test", "test", "test")); - - } - [Test] public void ChangePasswordQuestionAndAnswer_Without_RequiresQuestionAndAnswer() { @@ -49,18 +27,6 @@ namespace Umbraco.Tests.Membership Assert.Throws(() => provider.ChangePasswordQuestionAndAnswer("test", "test", "test", "test")); } - [Test] - public void ChangePasswordQuestionAndAnswer_Without_AllowManuallyChangingPassword_And_Invalid_Creds() - { - var providerMock = new Mock(TestHelper.GetHostingEnvironment()) { CallBase = true }; - providerMock.Setup(@base => @base.RequiresQuestionAndAnswer).Returns(true); - providerMock.Setup(@base => @base.AllowManuallyChangingPassword).Returns(false); - providerMock.Setup(@base => @base.ValidateUser("test", "test")).Returns(false); - var provider = providerMock.Object; - - Assert.IsFalse(provider.ChangePasswordQuestionAndAnswer("test", "test", "test", "test")); - } - [Test] public void CreateUser_Not_Whitespace() { diff --git a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs index 3759bc50aa..85f57e3103 100644 --- a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs +++ b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs @@ -106,7 +106,7 @@ namespace Umbraco.Tests.Membership Assert.AreNotEqual("test", createdMember.RawPasswordValue); string salt; - var storedPassword = provider.PasswordSecurity.StoredPassword(createdMember.RawPasswordValue, out salt); + var storedPassword = provider.PasswordSecurity.ParseStoredHashPassword(createdMember.RawPasswordValue, out salt); var hashedPassword = provider.PasswordSecurity.HashPassword("testtest$1", salt); Assert.AreEqual(hashedPassword, storedPassword); } diff --git a/src/Umbraco.Tests/Routing/RenderRouteHandlerTests.cs b/src/Umbraco.Tests/Routing/RenderRouteHandlerTests.cs index 63506b0004..06c4b5be6d 100644 --- a/src/Umbraco.Tests/Routing/RenderRouteHandlerTests.cs +++ b/src/Umbraco.Tests/Routing/RenderRouteHandlerTests.cs @@ -148,7 +148,7 @@ namespace Umbraco.Tests.Routing var handler = new RenderRouteHandler(umbracoContext, new TestControllerFactory(umbracoContextAccessor, Mock.Of(), context => { var membershipHelper = new MembershipHelper( - umbracoContext.HttpContext, Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), AppCaches.Disabled, Mock.Of()); + umbracoContext.HttpContext, Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), AppCaches.Disabled, Mock.Of()); return new CustomDocumentController(Factory.GetInstance(), umbracoContextAccessor, Factory.GetInstance(), diff --git a/src/Umbraco.Tests/Security/PasswordSecurityTests.cs b/src/Umbraco.Tests/Security/PasswordSecurityTests.cs index 6c4d700f74..9ed130a62b 100644 --- a/src/Umbraco.Tests/Security/PasswordSecurityTests.cs +++ b/src/Umbraco.Tests/Security/PasswordSecurityTests.cs @@ -40,7 +40,7 @@ namespace Umbraco.Tests.Security var hashed = passwordSecurity.HashNewPassword(pass, out salt); var storedPassword = passwordSecurity.FormatPasswordForStorage(hashed, salt); - var result = passwordSecurity.CheckPassword("ThisIsAHashedPassword", storedPassword); + var result = passwordSecurity.VerifyPassword("ThisIsAHashedPassword", storedPassword); Assert.IsTrue(result); } @@ -55,7 +55,7 @@ namespace Umbraco.Tests.Security var hashed = passwordSecurity.HashNewPassword(pass, out salt); var storedPassword = passwordSecurity.FormatPasswordForStorage(hashed, salt); - var result = passwordSecurity.CheckPassword("ThisIsAHashedPassword", storedPassword); + var result = passwordSecurity.VerifyPassword("ThisIsAHashedPassword", storedPassword); Assert.IsTrue(result); } @@ -82,7 +82,7 @@ namespace Umbraco.Tests.Security var stored = salt + "ThisIsAHashedPassword"; string initSalt; - var result = passwordSecurity.StoredPassword(stored, out initSalt); + var result = passwordSecurity.ParseStoredHashPassword(stored, out initSalt); Assert.AreEqual("ThisIsAHashedPassword", result); } diff --git a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs index 3dc3cefeef..1b48e83ab9 100644 --- a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs +++ b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs @@ -151,7 +151,7 @@ namespace Umbraco.Tests.TestHelpers.ControllerTesting urlHelper.Setup(provider => provider.GetUrl(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(UrlInfo.Url("/hello/world/1234")); - var membershipHelper = new MembershipHelper(umbCtx.HttpContext, Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), AppCaches.Disabled, Mock.Of()); + var membershipHelper = new MembershipHelper(umbCtx.HttpContext, Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), AppCaches.Disabled, Mock.Of()); var umbHelper = new UmbracoHelper(Mock.Of(), Mock.Of(), diff --git a/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs b/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs index 8171bbb175..1b1d3172d3 100644 --- a/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs +++ b/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs @@ -71,7 +71,7 @@ namespace Umbraco.Tests.Testing.TestingTests Mock.Of(), Mock.Of(), Mock.Of(), - new MembershipHelper(umbracoContext.HttpContext, Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), AppCaches.Disabled, Mock.Of())); + new MembershipHelper(umbracoContext.HttpContext, Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), AppCaches.Disabled, Mock.Of())); Assert.Pass(); } @@ -103,7 +103,7 @@ namespace Umbraco.Tests.Testing.TestingTests var memberService = Mock.Of(); var memberTypeService = Mock.Of(); var membershipProvider = new MembersMembershipProvider(memberService, memberTypeService, Mock.Of(), TestHelper.GetHostingEnvironment(), TestHelper.GetIpResolver()); - var membershipHelper = new MembershipHelper(umbracoContext.HttpContext, Mock.Of(), membershipProvider, Mock.Of(), memberService, memberTypeService, Mock.Of(), Mock.Of(), AppCaches.Disabled, logger); + var membershipHelper = new MembershipHelper(umbracoContext.HttpContext, Mock.Of(), membershipProvider, Mock.Of(), memberService, memberTypeService, Mock.Of(), AppCaches.Disabled, logger); var umbracoHelper = new UmbracoHelper(Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), membershipHelper); var umbracoMapper = new UmbracoMapper(new MapDefinitionCollection(new[] { Mock.Of() })); diff --git a/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs index 139b559d7f..4d7815b991 100644 --- a/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/AuthenticationControllerTests.cs @@ -76,7 +76,7 @@ namespace Umbraco.Tests.Web.Controllers } IOHelper.ForceNotHosted = true; var usersController = new AuthenticationController( - new DefaultUserPasswordConfig(), + new DefaultPasswordConfig(), Factory.GetInstance(), umbracoContextAccessor, Factory.GetInstance(), diff --git a/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs b/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs index c5d904fa24..0d8e9693d9 100644 --- a/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs +++ b/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs @@ -121,7 +121,7 @@ namespace Umbraco.Tests.Web.Mvc Mock.Of(), Mock.Of(), Mock.Of(query => query.Content(2) == content.Object), - new MembershipHelper(umbracoContext.HttpContext, Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), AppCaches.Disabled, Mock.Of())); + new MembershipHelper(umbracoContext.HttpContext, Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), AppCaches.Disabled, Mock.Of())); var ctrl = new TestSurfaceController(umbracoContextAccessor, helper); var result = ctrl.GetContent(2) as PublishedContentResult; diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js index c86b32a101..b106f35efb 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js @@ -90,7 +90,7 @@ $location.search('invite', null); }), //get the membership provider config for password policies - authResource.getPasswordConfig().then(function (data) { + authResource.getPasswordConfig(0).then(function (data) { vm.invitedUserPasswordModel.passwordPolicies = data; //localize the text diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/users/changepassword.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/users/changepassword.directive.js index 97eb2bf708..0512abe579 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/users/changepassword.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/users/changepassword.directive.js @@ -15,21 +15,14 @@ var unsubscribe = []; function resetModel(isNew) { - //the model config will contain an object, if it does not we'll create defaults - //NOTE: We will not support doing the password regex on the client side because the regex on the server side - //based on the membership provider cannot always be ported to js from .net directly. + //the model config will contain an object, if it does not we'll create defaults /* { hasPassword: true/false, - requiresQuestionAnswer: true/false, - enableReset: true/false, - enablePasswordRetrieval: true/false, minPasswordLength: 10 } */ - vm.showReset = false; - //set defaults if they are not available if (vm.config.disableToggle === undefined) { vm.config.disableToggle = false; @@ -37,20 +30,6 @@ if (vm.config.hasPassword === undefined) { vm.config.hasPassword = false; } - if (vm.config.enablePasswordRetrieval === undefined) { - vm.config.enablePasswordRetrieval = true; - } - if (vm.config.requiresQuestionAnswer === undefined) { - vm.config.requiresQuestionAnswer = false; - } - //don't enable reset if it is new - that doesn't make sense - if (isNew === "true") { - vm.config.enableReset = false; - } - else if (vm.config.enableReset === undefined) { - vm.config.enableReset = true; - } - if (vm.config.minPasswordLength === undefined) { vm.config.minPasswordLength = 0; } @@ -60,9 +39,7 @@ //if it's not an object then just create a new one vm.passwordValues = { newPassword: null, - oldPassword: null, - reset: null, - answer: null + oldPassword: null }; } else { @@ -73,8 +50,6 @@ vm.passwordValues.newPassword = null; vm.passwordValues.oldPassword = null; } - vm.passwordValues.reset = null; - vm.passwordValues.answer = null; } //the value to compare to match passwords @@ -105,11 +80,7 @@ })); unsubscribe.push($scope.$on("formSubmitting", function () { - //if there was a previously generated password displaying, clear it - if (vm.changing && vm.passwordValues) { - vm.passwordValues.generatedPassword = null; - } - else if (!vm.changing) { + if (!vm.changing) { //we are not changing, so the model needs to be null vm.passwordValues = null; } @@ -130,8 +101,6 @@ function doChange() { resetModel(); vm.changing = true; - //if there was a previously generated password displaying, clear it - vm.passwordValues.generatedPassword = null; vm.passwordValues.confirm = null; }; @@ -143,8 +112,7 @@ function showOldPass() { return vm.config.hasPassword && - !vm.config.allowManuallyChangingPassword && - !vm.config.enablePasswordRetrieval && !vm.showReset; + !vm.config.allowManuallyChangingPassword; }; // TODO: I don't think we need this or the cancel button, this can be up to the editor rendering this component diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/auth.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/auth.resource.js index a088b8fc73..cc9da4dbef 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/auth.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/auth.resource.js @@ -204,13 +204,13 @@ function authResource($q, $http, umbRequestHelper, angularHelper) { * @description * Gets the configuration of the user membership provider which is used to configure the change password form */ - getPasswordConfig: function () { - return umbRequestHelper.resourcePromise( - $http.get( - umbRequestHelper.getApiUrl( - "authenticationApiBaseUrl", - "GetPasswordConfig")), - 'Failed to retrieve membership provider config'); + getPasswordConfig: function (userId) { + return umbRequestHelper.resourcePromise( + $http.get( + umbRequestHelper.getApiUrl( + "authenticationApiBaseUrl", + "GetPasswordConfig", { userId: userId })), + 'Failed to retrieve membership provider config'); }, /** diff --git a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js index a03a71febe..a3017548a4 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js @@ -41,7 +41,7 @@ if (!model) { return null; } - var trimmed = _.omit(model, ["confirm", "generatedPassword"]); + var trimmed = _.omit(model, ["confirm"]); //ensure that the pass value is null if all child properties are null var allNull = true; @@ -304,33 +304,6 @@ } saveModel.memberGroups = selectedGroups; - //turn the dictionary into an array of pairs - var memberProviderPropAliases = _.pairs(displayModel.fieldConfig); - _.each(displayModel.tabs, function (tab) { - _.each(tab.properties, function (prop) { - var foundAlias = _.find(memberProviderPropAliases, function (item) { - return prop.alias === item[1]; - }); - if (foundAlias) { - //we know the current property matches an alias, now we need to determine which membership provider property it was for - // by looking at the key - switch (foundAlias[0]) { - case "umbracoMemberLockedOut": - saveModel.isLockedOut = Object.toBoolean(prop.value); - break; - case "umbracoMemberApproved": - saveModel.isApproved = Object.toBoolean(prop.value); - break; - case "umbracoMemberComments": - saveModel.comments = prop.value; - break; - } - } - }); - }); - - - return saveModel; }, diff --git a/src/Umbraco.Web.UI.Client/src/views/common/overlays/user/user.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/overlays/user/user.controller.js index d098d3dd7c..060ef77a5d 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/overlays/user/user.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/overlays/user/user.controller.js @@ -87,6 +87,17 @@ angular.module("umbraco") } } }); + + //go get the config for the membership provider and add it to the model + authResource.getPasswordConfig(user.id).then(function (data) { + $scope.changePasswordModel.config = data; + //ensure the hasPassword config option is set to true (the user of course has a password already assigned) + //this will ensure the oldPassword is shown so they can change it + // disable reset password functionality beacuse it does not make sense inside the backoffice + $scope.changePasswordModel.config.hasPassword = true; + $scope.changePasswordModel.config.disableToggle = true; + }); + } }); } @@ -103,6 +114,12 @@ angular.module("umbraco") }); } + //create the initial model for change password + $scope.changePasswordModel = { + config: {}, + value: {} + }; + updateUserInfo(); //remove all event handlers @@ -113,25 +130,6 @@ angular.module("umbraco") }); - /* ---------- UPDATE PASSWORD ---------- */ - - //create the initial model for change password - $scope.changePasswordModel = { - config: {}, - value: {} - }; - - //go get the config for the membership provider and add it to the model - authResource.getPasswordConfig().then(function(data) { - $scope.changePasswordModel.config = data; - //ensure the hasPassword config option is set to true (the user of course has a password already assigned) - //this will ensure the oldPassword is shown so they can change it - // disable reset password functionality beacuse it does not make sense inside the backoffice - $scope.changePasswordModel.config.hasPassword = true; - $scope.changePasswordModel.config.disableToggle = true; - $scope.changePasswordModel.config.enableReset = false; - }); - $scope.changePassword = function() { if (formHelper.submitForm({ scope: $scope })) { @@ -143,11 +141,6 @@ angular.module("umbraco") //reset old data clearPasswordFields(); - //if the password has been reset, then update our model - if (data.value) { - $scope.changePasswordModel.value.generatedPassword = data.value; - } - formHelper.resetForm({ scope: $scope }); $scope.changePasswordButtonState = "success"; diff --git a/src/Umbraco.Web.UI.Client/src/views/components/users/change-password.html b/src/Umbraco.Web.UI.Client/src/views/components/users/change-password.html index b7b8ec12c7..6ef4378106 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/users/change-password.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/users/change-password.html @@ -1,9 +1,4 @@ - - Password has been reset to: - - {{vm.passwordValues.generatedPassword}} - @@ -13,15 +8,7 @@ - - - - {{changePasswordForm.resetPassword.errorMsg}} - - - - + - + - + /// [WebApi.UmbracoAuthorize(requireApproval: false)] - public IDictionary GetPasswordConfig() + public IDictionary GetPasswordConfig(int userId) { - return _passwordConfiguration.GetConfiguration(); + return _passwordConfiguration.GetConfiguration(userId != UmbracoContext.Security.CurrentUser.Id); } /// diff --git a/src/Umbraco.Web/Editors/Binders/MemberBinder.cs b/src/Umbraco.Web/Editors/Binders/MemberBinder.cs index 8cc34896ba..33e37adb2b 100644 --- a/src/Umbraco.Web/Editors/Binders/MemberBinder.cs +++ b/src/Umbraco.Web/Editors/Binders/MemberBinder.cs @@ -2,17 +2,12 @@ using System.Collections.Generic; using System.Web.Http.Controllers; using System.Web.Http.ModelBinding; -using System.Web.Security; using Umbraco.Core; using Umbraco.Core.Models; -using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; using System.Linq; -using Umbraco.Core.Models.Membership; -using Umbraco.Core.Services.Implement; using Umbraco.Web.Composing; -using Umbraco.Web.Security; namespace Umbraco.Web.Editors.Binders { diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 18450f41ae..fe490e8699 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -8,7 +8,6 @@ using System.Text; using System.Web.Http; using System.Web.Http.Controllers; using System.Web.Http.ModelBinding; -using System.Web.Security; using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Configuration; diff --git a/src/Umbraco.Web/Editors/CurrentUserController.cs b/src/Umbraco.Web/Editors/CurrentUserController.cs index 46151f2a9e..0aa0dd029a 100644 --- a/src/Umbraco.Web/Editors/CurrentUserController.cs +++ b/src/Umbraco.Web/Editors/CurrentUserController.cs @@ -167,20 +167,13 @@ namespace Umbraco.Web.Editors /// public async Task> PostChangePassword(ChangingPasswordModel data) { - var passwordChanger = new PasswordChanger(Logger, Services.UserService, UmbracoContext.HttpContext); + var passwordChanger = new PasswordChanger(Logger); var passwordChangeResult = await passwordChanger.ChangePasswordWithIdentityAsync(Security.CurrentUser, Security.CurrentUser, data, UserManager); if (passwordChangeResult.Success) { var userMgr = this.TryGetOwinContext().Result.GetBackOfficeUserManager(); - //raise the reset event - // TODO: I don't think this is required anymore since from 7.7 we no longer display the reset password checkbox since that didn't make sense. - if (data.Reset.HasValue && data.Reset.Value) - { - userMgr.RaisePasswordResetEvent(Security.CurrentUser.Id); - } - //even if we weren't resetting this, it is the correct value (null), otherwise if we were resetting then it will contain the new pword var result = new ModelWithNotifications(passwordChangeResult.Result.ResetPassword); result.AddSuccessNotification(Services.TextService.Localize("user/password"), Services.TextService.Localize("user/passwordChanged")); diff --git a/src/Umbraco.Web/Editors/Filters/MemberSaveModelValidator.cs b/src/Umbraco.Web/Editors/Filters/MemberSaveModelValidator.cs index 75f497ec6f..f0d9b1f1a9 100644 --- a/src/Umbraco.Web/Editors/Filters/MemberSaveModelValidator.cs +++ b/src/Umbraco.Web/Editors/Filters/MemberSaveModelValidator.cs @@ -5,7 +5,6 @@ using System.Net; using System.Net.Http; using System.Web.Http.Controllers; using System.Web.Http.ModelBinding; -using System.Web.Security; using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Models; @@ -21,11 +20,18 @@ namespace Umbraco.Web.Editors.Filters internal class MemberSaveModelValidator : ContentModelValidator> { private readonly IMemberTypeService _memberTypeService; + private readonly IMemberService _memberService; - public MemberSaveModelValidator(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, ILocalizedTextService textService, IMemberTypeService memberTypeService) + public MemberSaveModelValidator( + ILogger logger, + IUmbracoContextAccessor umbracoContextAccessor, + ILocalizedTextService textService, + IMemberTypeService memberTypeService, + IMemberService memberService) : base(logger, umbracoContextAccessor, textService) { _memberTypeService = memberTypeService ?? throw new ArgumentNullException(nameof(memberTypeService)); + _memberService = memberService ?? throw new ArgumentNullException(nameof(memberService)); } /// @@ -52,10 +58,7 @@ namespace Umbraco.Web.Editors.Filters $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}email"); } - //default provider! - var membershipProvider = MembershipProviderExtensions.GetMembersMembershipProvider(); - - var validEmail = ValidateUniqueEmail(model, membershipProvider); + var validEmail = ValidateUniqueEmail(model); if (validEmail == false) { modelState.AddPropertyError( @@ -63,7 +66,7 @@ namespace Umbraco.Web.Editors.Filters $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}email"); } - var validLogin = ValidateUniqueLogin(model, membershipProvider); + var validLogin = ValidateUniqueLogin(model); if (validLogin == false) { modelState.AddPropertyError( @@ -121,13 +124,11 @@ namespace Umbraco.Web.Editors.Filters return ValidateProperties(propertiesToValidate, model.PersistedContent.Properties.ToList(), actionContext); } - internal bool ValidateUniqueLogin(MemberSave model, MembershipProvider membershipProvider) + internal bool ValidateUniqueLogin(MemberSave model) { if (model == null) throw new ArgumentNullException(nameof(model)); - if (membershipProvider == null) throw new ArgumentNullException(nameof(membershipProvider)); - int totalRecs; - var existingByName = membershipProvider.FindUsersByName(model.Username.Trim(), 0, int.MaxValue, out totalRecs); + var existingByName = _memberService.GetByUsername(model.Username.Trim()); switch (model.Action) { case ContentSaveAction.Save: @@ -136,8 +137,7 @@ namespace Umbraco.Web.Editors.Filters if (model.PersistedContent.Username.InvariantEquals(model.Username.Trim()) == false) { //they are changing their login name - if (existingByName.Cast().Select(x => x.UserName) - .Any(x => x == model.Username.Trim())) + if (existingByName != null && existingByName.Username == model.Username.Trim()) { //the user cannot use this login return false; @@ -146,8 +146,7 @@ namespace Umbraco.Web.Editors.Filters break; case ContentSaveAction.SaveNew: //check if the user's login already exists - if (existingByName.Cast().Select(x => x.UserName) - .Any(x => x == model.Username.Trim())) + if (existingByName != null && existingByName.Username == model.Username.Trim()) { //the user cannot use this login return false; @@ -161,18 +160,11 @@ namespace Umbraco.Web.Editors.Filters return true; } - internal bool ValidateUniqueEmail(MemberSave model, MembershipProvider membershipProvider) + internal bool ValidateUniqueEmail(MemberSave model) { if (model == null) throw new ArgumentNullException(nameof(model)); - if (membershipProvider == null) throw new ArgumentNullException(nameof(membershipProvider)); - if (membershipProvider.RequiresUniqueEmail == false) - { - return true; - } - - int totalRecs; - var existingByEmail = membershipProvider.FindUsersByEmail(model.Email.Trim(), 0, int.MaxValue, out totalRecs); + var existingByEmail = _memberService.GetByEmail(model.Email.Trim()); switch (model.Action) { case ContentSaveAction.Save: @@ -180,8 +172,7 @@ namespace Umbraco.Web.Editors.Filters if (model.PersistedContent.Email.InvariantEquals(model.Email.Trim()) == false) { //they are changing their email - if (existingByEmail.Cast().Select(x => x.Email) - .Any(x => x.InvariantEquals(model.Email.Trim()))) + if (existingByEmail != null && existingByEmail.Email.InvariantEquals(model.Email.Trim())) { //the user cannot use this email return false; @@ -190,8 +181,7 @@ namespace Umbraco.Web.Editors.Filters break; case ContentSaveAction.SaveNew: //check if the user's email already exists - if (existingByEmail.Cast().Select(x => x.Email) - .Any(x => x.InvariantEquals(model.Email.Trim()))) + if (existingByEmail != null && existingByEmail.Email.InvariantEquals(model.Email.Trim())) { //the user cannot use this email return false; diff --git a/src/Umbraco.Web/Editors/Filters/MemberSaveValidationAttribute.cs b/src/Umbraco.Web/Editors/Filters/MemberSaveValidationAttribute.cs index 7e2c204596..629a5a4eec 100644 --- a/src/Umbraco.Web/Editors/Filters/MemberSaveValidationAttribute.cs +++ b/src/Umbraco.Web/Editors/Filters/MemberSaveValidationAttribute.cs @@ -17,23 +17,25 @@ namespace Umbraco.Web.Editors.Filters private readonly IUmbracoContextAccessor _umbracoContextAccessor; private readonly ILocalizedTextService _textService; private readonly IMemberTypeService _memberTypeService; + private readonly IMemberService _memberService; public MemberSaveValidationAttribute() - : this(Current.Logger, Current.UmbracoContextAccessor, Current.Services.TextService, Current.Services.MemberTypeService) + : this(Current.Logger, Current.UmbracoContextAccessor, Current.Services.TextService, Current.Services.MemberTypeService, Current.Services.MemberService) { } - public MemberSaveValidationAttribute(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, ILocalizedTextService textService, IMemberTypeService memberTypeService) + public MemberSaveValidationAttribute(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, ILocalizedTextService textService, IMemberTypeService memberTypeService, IMemberService memberService) { _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor)); _textService = textService ?? throw new ArgumentNullException(nameof(textService)); _memberTypeService = memberTypeService ?? throw new ArgumentNullException(nameof(memberTypeService)); + _memberService = memberService ?? throw new ArgumentNullException(nameof(memberService));; } public override void OnActionExecuting(HttpActionContext actionContext) { var model = (MemberSave)actionContext.ActionArguments["contentItem"]; - var contentItemValidator = new MemberSaveModelValidator(_logger, _umbracoContextAccessor, _textService, _memberTypeService); + var contentItemValidator = new MemberSaveModelValidator(_logger, _umbracoContextAccessor,_textService, _memberTypeService, _memberService); //now do each validation step if (contentItemValidator.ValidateExistingContent(model, actionContext)) if (contentItemValidator.ValidateProperties(model, model, actionContext)) diff --git a/src/Umbraco.Web/Editors/MemberController.cs b/src/Umbraco.Web/Editors/MemberController.cs index 67234b3de2..a270460075 100644 --- a/src/Umbraco.Web/Editors/MemberController.cs +++ b/src/Umbraco.Web/Editors/MemberController.cs @@ -7,13 +7,10 @@ using System.Net.Http.Formatting; using System.Net.Http.Headers; using System.Web.Http; using System.Web.Http.ModelBinding; -using System.Web.Security; using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; -using Umbraco.Core.Persistence.DatabaseModelDefinitions; -using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Core.Services.Implement; using Umbraco.Web.WebApi; @@ -32,6 +29,8 @@ using Umbraco.Web.Editors.Filters; using Constants = Umbraco.Core.Constants; using Umbraco.Core.Dictionary; using Umbraco.Web.Security; +using Umbraco.Core.Security; +using System.Threading.Tasks; namespace Umbraco.Web.Editors { @@ -44,15 +43,17 @@ namespace Umbraco.Web.Editors [OutgoingNoHyphenGuidFormat] public class MemberController : ContentControllerBase { - public MemberController(ICultureDictionary cultureDictionary, PropertyEditorCollection propertyEditors, IGlobalSettings globalSettings, IUmbracoContextAccessor umbracoContextAccessor, ISqlContext sqlContext, ServiceContext services, AppCaches appCaches, IProfilingLogger logger, IRuntimeState runtimeState, UmbracoHelper umbracoHelper) + public MemberController(IMemberPasswordConfiguration passwordConfig, ICultureDictionary cultureDictionary, PropertyEditorCollection propertyEditors, IGlobalSettings globalSettings, IUmbracoContextAccessor umbracoContextAccessor, ISqlContext sqlContext, ServiceContext services, AppCaches appCaches, IProfilingLogger logger, IRuntimeState runtimeState, UmbracoHelper umbracoHelper) : base(cultureDictionary, globalSettings, umbracoContextAccessor, sqlContext, services, appCaches, logger, runtimeState, umbracoHelper) { + _passwordConfig = passwordConfig ?? throw new ArgumentNullException(nameof(passwordConfig)); _propertyEditors = propertyEditors ?? throw new ArgumentNullException(nameof(propertyEditors)); } - private readonly MembershipProvider _provider = MembershipProviderExtensions.GetMembersMembershipProvider(); + private readonly IMemberPasswordConfiguration _passwordConfig; private readonly PropertyEditorCollection _propertyEditors; - + private PasswordSecurity _passwordSecurity; + private PasswordSecurity PasswordSecurity => _passwordSecurity ?? (_passwordSecurity = new PasswordSecurity(_passwordConfig)); public PagedResult GetPagedResults( int pageNumber = 1, @@ -149,10 +150,10 @@ namespace Umbraco.Web.Editors throw new HttpResponseException(HttpStatusCode.NotFound); } - var provider = MembershipProviderExtensions.GetMembersMembershipProvider(); + var passwordGenerator = new PasswordGenerator(_passwordConfig); emptyContent = new Member(contentType); - emptyContent.AdditionalData["NewPassword"] = Membership.GeneratePassword(provider.MinRequiredPasswordLength, provider.MinRequiredNonAlphanumericCharacters); + emptyContent.AdditionalData["NewPassword"] = passwordGenerator.GeneratePassword(); return Mapper.Map(emptyContent); } @@ -163,7 +164,7 @@ namespace Umbraco.Web.Editors [FileUploadCleanupFilter] [OutgoingEditorModelEvent] [MemberSaveValidation] - public MemberDisplay PostSave( + public async Task PostSave( [ModelBinder(typeof(MemberBinder))] MemberSave contentItem) { @@ -178,6 +179,8 @@ namespace Umbraco.Web.Editors //map the properties to the persisted entity MapPropertyValues(contentItem); + await ValidateMemberDataAsync(contentItem); + //Unlike content/media - if there are errors for a member, we do NOT proceed to save them, we cannot so return the errors if (ModelState.IsValid == false) { @@ -186,53 +189,31 @@ namespace Umbraco.Web.Editors throw new HttpResponseException(Request.CreateValidationErrorResponse(forDisplay)); } - // TODO: WE need to support this! - requires UI updates, etc... - if (_provider.RequiresQuestionAndAnswer) - { - throw new NotSupportedException("Currently the member editor does not support providers that have RequiresQuestionAndAnswer specified"); - } - //We're gonna look up the current roles now because the below code can cause // events to be raised and developers could be manually adding roles to members in // their handlers. If we don't look this up now there's a chance we'll just end up // removing the roles they've assigned. - var currRoles = Roles.GetRolesForUser(contentItem.PersistedContent.Username); + var currRoles = Services.MemberService.GetAllRoles(contentItem.PersistedContent.Username); //find the ones to remove and remove them var rolesToRemove = currRoles.Except(contentItem.Groups).ToArray(); - string generatedPassword = null; //Depending on the action we need to first do a create or update using the membership provider // this ensures that passwords are formatted correctly and also performs the validation on the provider itself. switch (contentItem.Action) { case ContentSaveAction.Save: - generatedPassword = UpdateWithMembershipProvider(contentItem); + UpdateMemberData(contentItem); break; case ContentSaveAction.SaveNew: - MembershipCreateStatus status; - CreateWithMembershipProvider(contentItem, out status); - - // save the ID of the creator - contentItem.PersistedContent.CreatorId = Security.CurrentUser.Id; + contentItem.PersistedContent = CreateMemberData(contentItem); break; default: //we don't support anything else for members throw new HttpResponseException(HttpStatusCode.NotFound); } - //If we've had problems creating/updating the user with the provider then return the error - if (ModelState.IsValid == false) - { - var forDisplay = Mapper.Map(contentItem.PersistedContent); - forDisplay.Errors = ModelState.ToErrorDictionary(); - throw new HttpResponseException(Request.CreateValidationErrorResponse(forDisplay)); - } - - //save the IMember - - //save the item - //NOTE: We are setting the password to NULL - this indicates to the system to not actually save the password - // so it will not get overwritten! - contentItem.PersistedContent.RawPasswordValue = null; + //TODO: There's 3 things saved here and we should do this all in one transaction, which we can do here by wrapping in a scope + // but it would be nicer to have this taken care of within the Save method itself //create/save the IMember Services.MemberService.Save(contentItem.PersistedContent); @@ -241,21 +222,16 @@ namespace Umbraco.Web.Editors // if we are changing the username, it must be persisted before looking up the member roles). if (rolesToRemove.Any()) { - Roles.RemoveUserFromRoles(contentItem.PersistedContent.Username, rolesToRemove); + Services.MemberService.DissociateRoles(new[] { contentItem.PersistedContent.Username }, rolesToRemove); } //find the ones to add and add them var toAdd = contentItem.Groups.Except(currRoles).ToArray(); if (toAdd.Any()) { //add the ones submitted - Roles.AddUserToRoles(contentItem.PersistedContent.Username, toAdd); + Services.MemberService.AssignRoles(new[] { contentItem.PersistedContent.Username }, toAdd); } - //set the generated password (if there was one) - in order to do this we'll chuck the generated password into the - // additional data of the IUmbracoEntity of the persisted item - then we can retrieve this in the model mapper and set - // the value to be given to the UI. Hooray for AdditionalData :) - contentItem.PersistedContent.AdditionalData["GeneratedPassword"] = generatedPassword; - //return the updated model var display = Mapper.Map(contentItem.PersistedContent); @@ -296,32 +272,38 @@ namespace Umbraco.Web.Editors null); // member are all invariant } + private IMember CreateMemberData(MemberSave contentItem) + { + var memberType = Services.MemberTypeService.Get(contentItem.ContentTypeAlias); + if (memberType == null) + throw new InvalidOperationException($"No member type found with alias {contentItem.ContentTypeAlias}"); + var member = new Member(contentItem.Name, contentItem.Email, contentItem.Username, memberType, true) + { + CreatorId = Security.CurrentUser.Id, + RawPasswordValue = PasswordSecurity.HashPasswordForStorage(contentItem.Password.NewPassword), + Comments = contentItem.Comments, + IsApproved = contentItem.IsApproved + }; + + return member; + } + /// - /// Update the membership user using the membership provider (for things like email, etc...) - /// If a password change is detected then we'll try that too. + /// Update the member security data /// /// /// /// If the password has been reset then this method will return the reset/generated password, otherwise will return null. /// - private string UpdateWithMembershipProvider(MemberSave contentItem) + private void UpdateMemberData(MemberSave contentItem) { - //Get the member from the provider + contentItem.PersistedContent.WriterId = Security.CurrentUser.Id; - var membershipUser = _provider.GetUser(contentItem.PersistedContent.Key, false); - if (membershipUser == null) - { - //This should never happen! so we'll let it YSOD if it does. - throw new InvalidOperationException("Could not get member from membership provider " + _provider.Name + " with key " + contentItem.PersistedContent.Key); - } - - var shouldReFetchMember = false; - var providedUserName = contentItem.PersistedContent.Username; - - //if the user doesn't have access to sensitive values, then we need to check if any of the built in member property types - //have been marked as sensitive. If that is the case we cannot change these persisted values no matter what value has been posted. - //There's only 3 special ones we need to deal with that are part of the MemberSave instance - if (Security.CurrentUser.HasAccessToSensitiveData() == false) + // If the user doesn't have access to sensitive values, then we need to check if any of the built in member property types + // have been marked as sensitive. If that is the case we cannot change these persisted values no matter what value has been posted. + // There's only 3 special ones we need to deal with that are part of the MemberSave instance: Comments, IsApproved, IsLockedOut + // but we will take care of this in a generic way below so that it works for all props. + if (!Security.CurrentUser.HasAccessToSensitiveData()) { var memberType = Services.MemberTypeService.Get(contentItem.PersistedContent.ContentTypeId); var sensitiveProperties = memberType @@ -330,161 +312,37 @@ namespace Umbraco.Web.Editors foreach (var sensitiveProperty in sensitiveProperties) { - //if found, change the value of the contentItem model to the persisted value so it remains unchanged - switch (sensitiveProperty.Alias) + var destProp = contentItem.Properties.FirstOrDefault(x => x.Alias == sensitiveProperty.Alias); + if (destProp != null) { - case Constants.Conventions.Member.Comments: - contentItem.Comments = contentItem.PersistedContent.Comments; - break; - case Constants.Conventions.Member.IsApproved: - contentItem.IsApproved = contentItem.PersistedContent.IsApproved; - break; - case Constants.Conventions.Member.IsLockedOut: - contentItem.IsLockedOut = contentItem.PersistedContent.IsLockedOut; - break; + //if found, change the value of the contentItem model to the persisted value so it remains unchanged + var origValue = contentItem.PersistedContent.GetValue(sensitiveProperty.Alias); + destProp.Value = origValue; } } } - //Update the membership user if it has changed - try - { - var requiredUpdating = Members.UpdateMember(membershipUser, _provider, - contentItem.Email.Trim(), - contentItem.IsApproved, - comment: contentItem.Comments); - - if (requiredUpdating.Success) - { - //re-map these values - shouldReFetchMember = true; - } - } - catch (Exception ex) - { - Logger.Warn(ex, "Could not update member, the provider returned an error"); - ModelState.AddPropertyError( - //specify 'default' just so that it shows up as a notification - is not assigned to a property - new ValidationResult("Could not update member, the provider returned an error: " + ex.Message + " (see log for full details)"), "default"); - } + var isLockedOut = contentItem.IsLockedOut; //if they were locked but now they are trying to be unlocked - if (membershipUser.IsLockedOut && contentItem.IsLockedOut == false) + if (contentItem.PersistedContent.IsLockedOut && isLockedOut == false) { - try - { - var result = _provider.UnlockUser(membershipUser.UserName); - if (result == false) - { - //it wasn't successful - but it won't really tell us why. - ModelState.AddModelError("custom", "Could not unlock the user"); - } - else - { - shouldReFetchMember = true; - } - } - catch (Exception ex) - { - ModelState.AddModelError("custom", ex); - } + contentItem.PersistedContent.IsLockedOut = false; + contentItem.PersistedContent.FailedPasswordAttempts = 0; } - else if (membershipUser.IsLockedOut == false && contentItem.IsLockedOut) + else if (!contentItem.PersistedContent.IsLockedOut && isLockedOut) { //NOTE: This should not ever happen unless someone is mucking around with the request data. //An admin cannot simply lock a user, they get locked out by password attempts, but an admin can un-approve them ModelState.AddModelError("custom", "An admin cannot lock a user"); } - //password changes ? + //no password changes then exit ? if (contentItem.Password == null) - { - //If the provider has changed some values, these values need to be reflected in the member object - //that will get mapped to the display object - if (shouldReFetchMember) - { - RefetchMemberData(contentItem, LookupType.ByKey); - RestoreProvidedUserName(contentItem, providedUserName); - } + return; - return null; - } - - var passwordChangeResult = Members.ChangePassword(membershipUser.UserName, contentItem.Password, _provider); - if (passwordChangeResult.Success) - { - //If the provider has changed some values, these values need to be reflected in the member object - //that will get mapped to the display object - if (shouldReFetchMember) - { - RefetchMemberData(contentItem, LookupType.ByKey); - RestoreProvidedUserName(contentItem, providedUserName); - } - - //even if we weren't resetting this, it is the correct value (null), otherwise if we were resetting then it will contain the new pword - return passwordChangeResult.Result.ResetPassword; - } - - //it wasn't successful, so add the change error to the model state - ModelState.AddPropertyError( - passwordChangeResult.Result.ChangeError, - string.Format("{0}password", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); - - return null; - } - - private enum LookupType - { - ByKey, - ByUserName - } - - /// - /// Re-fetches the database data to map to the PersistedContent object and re-assigns the already mapped the posted properties so that the display object is up-to-date - /// - /// - /// - /// - /// This is done during an update if the membership provider has changed some underlying data - we need to ensure that our model is consistent with that data - /// - private void RefetchMemberData(MemberSave contentItem, LookupType lookup) - { - var currProps = contentItem.PersistedContent.Properties.ToArray(); - - switch (lookup) - { - case LookupType.ByKey: - //Go and re-fetch the persisted item - contentItem.PersistedContent = Services.MemberService.GetByKey(contentItem.Key); - break; - case LookupType.ByUserName: - contentItem.PersistedContent = Services.MemberService.GetByUsername(contentItem.Username.Trim()); - break; - } - - UpdateName(contentItem); - - // re-assign the mapped values that are not part of the membership provider properties. - var builtInAliases = ConventionsHelper.GetStandardPropertyTypeStubs().Select(x => x.Key).ToArray(); - foreach (var p in contentItem.PersistedContent.Properties) - { - var valueMapped = currProps.FirstOrDefault(x => x.Alias == p.Alias); - if (builtInAliases.Contains(p.Alias) == false && valueMapped != null) - { - p.SetValue(valueMapped.GetValue()); - } - } - } - - /// - /// Following a refresh of member data called during an update if the membership provider has changed some underlying data, - /// we don't want to lose the provided, and potentially changed, username - /// - /// - /// - private static void RestoreProvidedUserName(MemberSave contentItem, string providedUserName) - { - contentItem.PersistedContent.Username = providedUserName; + // set the password + contentItem.PersistedContent.RawPasswordValue = PasswordSecurity.HashPasswordForStorage(contentItem.Password.NewPassword); } private static void UpdateName(MemberSave memberSave) @@ -496,111 +354,48 @@ namespace Umbraco.Web.Editors } } - /// - /// This is going to create the user with the membership provider and check for validation - /// - /// - /// - /// - /// - /// Depending on if the Umbraco membership provider is active or not, the process differs slightly: - /// - /// * If the umbraco membership provider is used - we create the membership user first with the membership provider, since - /// it's the umbraco membership provider, this writes to the umbraco tables. When that is complete we re-fetch the IMember - /// model data from the db. In this case we don't care what the provider user key is. - /// * If we're using a non-umbraco membership provider - we check if there is a 'Member' member type - if so - /// we create an empty IMember instance first (of type 'Member'), this gives us a unique ID (GUID) - /// that we then use to create the member in the custom membership provider. This acts as the link between Umbraco data and - /// the custom membership provider data. This gives us the ability to eventually have custom membership properties but still use - /// a custom membership provider. If there is no 'Member' member type, then we will simply just create the membership provider member - /// with no link to our data. - /// - /// If this is successful, it will go and re-fetch the IMember from the db because it will now have an ID because the Umbraco provider - /// uses the umbraco data store - then of course we need to re-map it to the saved property values. - /// - private MembershipUser CreateWithMembershipProvider(MemberSave contentItem, out MembershipCreateStatus status) + // TODO: This logic should be pulled into the service layer + private async Task ValidateMemberDataAsync(MemberSave contentItem) { - MembershipUser membershipUser; - - //We are using the umbraco membership provider, create the member using the membership provider first. - var umbracoMembershipProvider = (UmbracoMembershipProviderBase)_provider; - // TODO: We are not supporting q/a - passing in empty here - membershipUser = umbracoMembershipProvider.CreateUser( - contentItem.ContentTypeAlias, contentItem.Username, - contentItem.Password.NewPassword, - contentItem.Email, "", "", - contentItem.IsApproved, - Guid.NewGuid(), //since it's the umbraco provider, the user key here doesn't make any difference - out status); - - // TODO: Localize these! - switch (status) + if (contentItem.Name.IsNullOrWhiteSpace()) { - case MembershipCreateStatus.Success: - - //map the key back - contentItem.Key = membershipUser.ProviderUserKey.TryConvertTo().Result; - contentItem.PersistedContent.Key = contentItem.Key; - - //if the comments are there then we need to save them - if (contentItem.Comments.IsNullOrWhiteSpace() == false) - { - membershipUser.Comment = contentItem.Comments; - _provider.UpdateUser(membershipUser); - } - - RefetchMemberData(contentItem, LookupType.ByUserName); - - break; - case MembershipCreateStatus.InvalidUserName: - ModelState.AddPropertyError( + ModelState.AddPropertyError( new ValidationResult("Invalid user name", new[] { "value" }), string.Format("{0}login", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); - break; - case MembershipCreateStatus.InvalidPassword: - ModelState.AddPropertyError( - new ValidationResult("Invalid password", new[] { "value" }), - string.Format("{0}password", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); - break; - case MembershipCreateStatus.InvalidQuestion: - case MembershipCreateStatus.InvalidAnswer: - throw new NotSupportedException("Currently the member editor does not support providers that have RequiresQuestionAndAnswer specified"); - case MembershipCreateStatus.InvalidEmail: - ModelState.AddPropertyError( - new ValidationResult("Invalid email", new[] { "value" }), - string.Format("{0}email", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); - break; - case MembershipCreateStatus.DuplicateUserName: - ModelState.AddPropertyError( - new ValidationResult("Username is already in use", new[] { "value" }), - string.Format("{0}login", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); - break; - case MembershipCreateStatus.DuplicateEmail: - ModelState.AddPropertyError( - new ValidationResult("Email address is already in use", new[] { "value" }), - string.Format("{0}email", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); - break; - case MembershipCreateStatus.InvalidProviderUserKey: - ModelState.AddPropertyError( - //specify 'default' just so that it shows up as a notification - is not assigned to a property - new ValidationResult("Invalid provider user key"), "default"); - break; - case MembershipCreateStatus.DuplicateProviderUserKey: - ModelState.AddPropertyError( - //specify 'default' just so that it shows up as a notification - is not assigned to a property - new ValidationResult("Duplicate provider user key"), "default"); - break; - case MembershipCreateStatus.ProviderError: - case MembershipCreateStatus.UserRejected: - ModelState.AddPropertyError( - //specify 'default' just so that it shows up as a notification - is not assigned to a property - new ValidationResult("User could not be created (rejected by provider)"), "default"); - break; - default: - throw new ArgumentOutOfRangeException(); + return false; } - return membershipUser; + if (contentItem.Password != null && !contentItem.Password.NewPassword.IsNullOrWhiteSpace()) + { + var validPassword = await PasswordSecurity.IsValidPasswordAsync(contentItem.Password.NewPassword); + if (!validPassword) + { + ModelState.AddPropertyError( + new ValidationResult("Invalid password: " + string.Join(", ", validPassword.Result), new[] { "value" }), + string.Format("{0}password", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); + return false; + } + } + + var byUsername = Services.MemberService.GetByUsername(contentItem.Username); + if (byUsername != null && byUsername.Key != contentItem.Key) + { + ModelState.AddPropertyError( + new ValidationResult("Username is already in use", new[] { "value" }), + string.Format("{0}login", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); + return false; + } + + var byEmail = Services.MemberService.GetByEmail(contentItem.Email); + if (byEmail != null && byEmail.Key != contentItem.Key) + { + ModelState.AddPropertyError( + new ValidationResult("Email address is already in use", new[] { "value" }), + string.Format("{0}email", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); + return false; + } + + return true; } /// diff --git a/src/Umbraco.Web/Editors/MemberGroupController.cs b/src/Umbraco.Web/Editors/MemberGroupController.cs index 89041fb512..d06c45aad6 100644 --- a/src/Umbraco.Web/Editors/MemberGroupController.cs +++ b/src/Umbraco.Web/Editors/MemberGroupController.cs @@ -3,14 +3,10 @@ using System.Linq; using System.Net; using System.Net.Http; using System.Web.Http; -using System.Web.Security; -using Umbraco.Core; using Umbraco.Core.Models; -using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Mvc; -using Umbraco.Web.Security; using Umbraco.Web.WebApi.Filters; using Constants = Umbraco.Core.Constants; diff --git a/src/Umbraco.Web/Editors/MemberTypeController.cs b/src/Umbraco.Web/Editors/MemberTypeController.cs index 9600b96f92..fd34eaf300 100644 --- a/src/Umbraco.Web/Editors/MemberTypeController.cs +++ b/src/Umbraco.Web/Editors/MemberTypeController.cs @@ -4,7 +4,6 @@ using System.Linq; using System.Net; using System.Net.Http; using System.Web.Http; -using System.Web.Security; using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Configuration; @@ -12,11 +11,9 @@ using Umbraco.Core.Dictionary; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Persistence; -using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Mvc; -using Umbraco.Web.Security; using Umbraco.Web.WebApi.Filters; using Constants = Umbraco.Core.Constants; diff --git a/src/Umbraco.Web/Editors/PasswordChanger.cs b/src/Umbraco.Web/Editors/PasswordChanger.cs index e1546cae51..6da4e364e1 100644 --- a/src/Umbraco.Web/Editors/PasswordChanger.cs +++ b/src/Umbraco.Web/Editors/PasswordChanger.cs @@ -1,14 +1,11 @@ using System; using System.ComponentModel.DataAnnotations; using System.Threading.Tasks; -using System.Web; -using System.Web.Security; using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Identity; using Umbraco.Core.Security; -using Umbraco.Core.Services; using Umbraco.Web.Models; using Umbraco.Web.Security; using IUser = Umbraco.Core.Models.Membership.IUser; @@ -18,14 +15,10 @@ namespace Umbraco.Web.Editors internal class PasswordChanger { private readonly ILogger _logger; - private readonly IUserService _userService; - private readonly HttpContextBase _httpContext; - public PasswordChanger(ILogger logger, IUserService userService, HttpContextBase httpContext) + public PasswordChanger(ILogger logger) { _logger = logger; - _userService = userService; - _httpContext = httpContext; } /// @@ -45,55 +38,43 @@ namespace Umbraco.Web.Editors if (passwordModel == null) throw new ArgumentNullException(nameof(passwordModel)); if (userMgr == null) throw new ArgumentNullException(nameof(userMgr)); - //Are we resetting the password? - //This flag indicates that either an admin user is changing another user's password without knowing the original password - // or that the password needs to be reset to an auto-generated one. - if (passwordModel.Reset.HasValue && passwordModel.Reset.Value) - { - //if it's the current user, the current user cannot reset their own password - if (currentUser.Username == savingUser.Username) - { - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Password reset is not allowed", new[] { "resetPassword" }) }); - } - - //if the current user has access to reset/manually change the password - if (currentUser.HasSectionAccess(Umbraco.Core.Constants.Applications.Users) == false) - { - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("The current user is not authorized", new[] { "resetPassword" }) }); - } - - //ok, we should be able to reset it - var resetToken = await userMgr.GeneratePasswordResetTokenAsync(savingUser.Id); - var newPass = passwordModel.NewPassword.IsNullOrWhiteSpace() - ? userMgr.GeneratePassword() - : passwordModel.NewPassword; - - var resetResult = await userMgr.ChangePasswordWithResetAsync(savingUser.Id, resetToken, newPass); - - if (resetResult.Succeeded == false) - { - var errors = string.Join(". ", resetResult.Errors); - _logger.Warn("Could not reset user password {PasswordErrors}", errors); - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult(errors, new[] { "resetPassword" }) }); - } - - return Attempt.Succeed(new PasswordChangedModel()); - } - - //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" }) }); } - //we cannot arbitrarily change the password without knowing the old one and no old password was supplied - need to return an error + //Are we just changing another user's 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 it's the current user, the current user cannot reset their own password + if (currentUser.Username == savingUser.Username) + { + return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Password reset is not allowed", new[] { "value" }) }); + } + + //if the current user has access to reset/manually change the password + if (currentUser.HasSectionAccess(Umbraco.Core.Constants.Applications.Users) == false) + { + return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("The current user is not authorized", new[] { "value" }) }); + } + + //ok, we should be able to reset it + var resetToken = await userMgr.GeneratePasswordResetTokenAsync(savingUser.Id); + + var resetResult = await userMgr.ChangePasswordWithResetAsync(savingUser.Id, resetToken, passwordModel.NewPassword); + + if (resetResult.Succeeded == false) + { + var errors = string.Join(". ", resetResult.Errors); + _logger.Warn("Could not reset user password {PasswordErrors}", errors); + return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult(errors, new[] { "value" }) }); + } + + return Attempt.Succeed(new PasswordChangedModel()); } + //we're changing our own password... + //get the user var backOfficeIdentityUser = await userMgr.FindByIdAsync(savingUser.Id); if (backOfficeIdentityUser == null) @@ -120,179 +101,5 @@ namespace Umbraco.Web.Editors return Attempt.Succeed(new PasswordChangedModel()); } - /// - /// Changes password for a member/user given the membership provider and the password change model - /// - /// The username of the user having their password changed - /// - /// - /// - public 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 backofficeUserManager = _httpContext.GetOwinContext().GetBackOfficeUserManager(); - var userId = -1; - - if (backofficeUserManager == null) - { - // should never happen - throw new InvalidOperationException("Could not resolve the back office user manager"); - } - - var profile = _userService.GetProfileByUserName(username); - if (profile != null) - int.TryParse(profile.Id.ToString(), out userId); - - //Are we resetting the password? - //This flag indicates that either an admin user is changing another user's password without knowing the original password - // or that the password needs to be reset to an auto-generated one. - if (passwordModel.Reset.HasValue && passwordModel.Reset.Value) - { - //if a new password is supplied then it's an admin user trying to change another user's password without knowing the original password - //this is only possible when using a membership provider if the membership provider supports AllowManuallyChangingPassword - if (passwordModel.NewPassword.IsNullOrWhiteSpace() == false) - { - if (umbracoBaseProvider != null && umbracoBaseProvider.AllowManuallyChangingPassword) - { - //this provider allows manually changing the password without the old password, so we can just do it - try - { - var result = umbracoBaseProvider.ChangePassword(username, string.Empty, passwordModel.NewPassword); - - if (result && backofficeUserManager != null && userId >= 0) - backofficeUserManager.RaisePasswordChangedEvent(userId); - - return result == false - ? Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, invalid username or password", new[] { "value" }) }) - : Attempt.Succeed(new PasswordChangedModel()); - } - catch (Exception ex) - { - _logger.Warn("Could not change member password", ex); - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, error: " + ex.Message + " (see log for full details)", new[] { "value" }) }); - } - } - else - { - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Provider does not support manually changing passwords", new[] { "value" }) }); - } - } - - //we've made it here which means we need to generate a new password - - var canReset = true; // TODO: Remove this! This is legacy, all of this will be gone when we get membership providers entirely gone - if (canReset == false) - { - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Password reset is not enabled", new[] { "resetPassword" }) }); - } - if (membershipProvider.RequiresQuestionAndAnswer && passwordModel.Answer.IsNullOrWhiteSpace()) - { - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Password reset requires a password answer", new[] { "resetPassword" }) }); - } - - //ok, we should be able to reset it - try - { - var newPass = membershipProvider.ResetPassword( - username, - membershipProvider.RequiresQuestionAndAnswer ? passwordModel.Answer : null); - - if (userId >= 0) - backofficeUserManager.RaisePasswordResetEvent(userId); - - //return the generated pword - return Attempt.Succeed(new PasswordChangedModel { ResetPassword = newPass }); - } - catch (Exception ex) - { - _logger.Warn(ex, "Could not reset member password"); - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not reset password, error: " + ex.Message + " (see log for full details)", new[] { "resetPassword" }) }); - } - } - - //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" }) }); - } - - //without being able to retrieve the original password, - //we cannot arbitrarily change the password without knowing the old one and no old password was supplied - need to return an error - if (passwordModel.OldPassword.IsNullOrWhiteSpace() && membershipProvider.EnablePasswordRetrieval == false) - { - //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 (passwordModel.OldPassword.IsNullOrWhiteSpace() == false) - { - //if an old password is supplied try to change it - - try - { - var result = membershipProvider.ChangePassword(username, passwordModel.OldPassword, passwordModel.NewPassword); - - if (result && backofficeUserManager != null && userId >= 0) - backofficeUserManager.RaisePasswordChangedEvent(userId); - - 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.Warn(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" }) }); - } - } - - if (membershipProvider.EnablePasswordRetrieval == false) - { - //we cannot continue if we cannot get the current password - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Password cannot be changed without the old password", new[] { "oldPassword" }) }); - } - if (membershipProvider.RequiresQuestionAndAnswer && passwordModel.Answer.IsNullOrWhiteSpace()) - { - //if the question answer is required but there isn't one, we cannot continue - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Password cannot be changed without the password answer", new[] { "value" }) }); - } - - //lets try to get the old one so we can change it - try - { - var oldPassword = membershipProvider.GetPassword( - username, - membershipProvider.RequiresQuestionAndAnswer ? passwordModel.Answer : null); - - try - { - var result = membershipProvider.ChangePassword(username, oldPassword, passwordModel.NewPassword); - return result == false - ? Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password", new[] { "value" }) }) - : Attempt.Succeed(new PasswordChangedModel()); - } - catch (Exception ex1) - { - _logger.Warn(ex1, "Could not change member password"); - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, error: " + ex1.Message + " (see log for full details)", new[] { "value" }) }); - } - - } - catch (Exception ex2) - { - _logger.Warn(ex2, "Could not retrieve member password"); - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, error: " + ex2.Message + " (see log for full details)", new[] { "value" }) }); - } - } } } diff --git a/src/Umbraco.Web/Editors/UsersController.cs b/src/Umbraco.Web/Editors/UsersController.cs index 0a18a3ed41..2211813f63 100644 --- a/src/Umbraco.Web/Editors/UsersController.cs +++ b/src/Umbraco.Web/Editors/UsersController.cs @@ -551,7 +551,7 @@ namespace Umbraco.Web.Editors if (userSave.ChangePassword != null) { - var passwordChanger = new PasswordChanger(Logger, Services.UserService, UmbracoContext.HttpContext); + var passwordChanger = new PasswordChanger(Logger); //this will change the password and raise appropriate events var passwordChangeResult = await passwordChanger.ChangePasswordWithIdentityAsync(Security.CurrentUser, found, userSave.ChangePassword, UserManager); diff --git a/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs b/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs index 98f958c844..9bdade968c 100644 --- a/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs +++ b/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs @@ -4,7 +4,6 @@ using System.Net.Http; using System.Text; using System.Threading.Tasks; using System.Web; -using System.Web.Security; using Newtonsoft.Json; using Umbraco.Core; using Umbraco.Core.Composing; @@ -14,7 +13,6 @@ using Umbraco.Core.Models.Identity; using Umbraco.Core.Services; using Umbraco.Web.Install.Models; using Umbraco.Web.Security; -using System.Web; namespace Umbraco.Web.Install.InstallSteps { diff --git a/src/Umbraco.Web/Models/ChangingPasswordModel.cs b/src/Umbraco.Web/Models/ChangingPasswordModel.cs index 355b358049..50f2a1bed7 100644 --- a/src/Umbraco.Web/Models/ChangingPasswordModel.cs +++ b/src/Umbraco.Web/Models/ChangingPasswordModel.cs @@ -19,35 +19,5 @@ namespace Umbraco.Web.Models [DataMember(Name = "oldPassword")] public string OldPassword { get; set; } - /// - /// Set to true if the password is to be reset - /// - /// - /// - /// This operator is different between using ASP.NET Identity APIs and Membership APIs. - /// - /// - /// When using Membership APIs, this is only valid when: EnablePasswordReset = true and it will reset the password to something auto generated. - /// - /// - /// When using ASP.NET Identity APIs this needs to be set if an administrator user that has access to the Users section is changing another users - /// password. This flag is required to indicate that the oldPassword value is not required and that we are in fact performing a password reset and - /// then a password change if the executing user has access to do so. - /// - /// - [DataMember(Name = "reset")] - public bool? Reset { get; set; } - - /// - /// The password answer - required for reset when: RequiresQuestionAndAnswer = true - /// - [DataMember(Name = "answer")] - public string Answer { get; set; } - - /// - /// This is filled in on the server side if the password has been reset/generated - /// - [DataMember(Name = "generatedPassword")] - public string GeneratedPassword { get; set; } } } diff --git a/src/Umbraco.Web/Models/ContentEditing/MemberDisplay.cs b/src/Umbraco.Web/Models/ContentEditing/MemberDisplay.cs index 3391447535..f4200c4963 100644 --- a/src/Umbraco.Web/Models/ContentEditing/MemberDisplay.cs +++ b/src/Umbraco.Web/Models/ContentEditing/MemberDisplay.cs @@ -1,8 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Runtime.Serialization; -using Umbraco.Core.Models; -using Umbraco.Core.Models.Membership; +using System.Runtime.Serialization; namespace Umbraco.Web.Models.ContentEditing { @@ -12,24 +8,11 @@ namespace Umbraco.Web.Models.ContentEditing [DataContract(Name = "content", Namespace = "")] public class MemberDisplay : ListViewAwareContentItemDisplayBase { - public MemberDisplay() - { - MemberProviderFieldMapping = new Dictionary(); - } - [DataMember(Name = "username")] public string Username { get; set; } [DataMember(Name = "email")] public string Email { get; set; } - /// - /// This is used to indicate how to map the membership provider properties to the save model, this mapping - /// will change if a developer has opted to have custom member property aliases specified in their membership provider config, - /// or if we are editing a member that is not an Umbraco member (custom provider) - /// - [DataMember(Name = "fieldConfig")] - public IDictionary MemberProviderFieldMapping { get; set; } - } } diff --git a/src/Umbraco.Web/Models/ContentEditing/MemberSave.cs b/src/Umbraco.Web/Models/ContentEditing/MemberSave.cs index 5132dbc3ec..10774de1a4 100644 --- a/src/Umbraco.Web/Models/ContentEditing/MemberSave.cs +++ b/src/Umbraco.Web/Models/ContentEditing/MemberSave.cs @@ -1,11 +1,14 @@ using System; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.Linq; using System.Runtime.Serialization; using Newtonsoft.Json.Linq; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; using Umbraco.Core.Models.Validation; using Umbraco.Web.WebApi.Filters; +using Umbraco.Core; namespace Umbraco.Web.Models.ContentEditing { @@ -19,6 +22,7 @@ namespace Umbraco.Web.Models.ContentEditing [DataMember(Name = "email", IsRequired = true)] [RequiredForPersistence(AllowEmptyStrings = false, ErrorMessage = "Required")] + [EmailAddress] public string Email { get; set; } [DataMember(Name = "password")] @@ -27,16 +31,27 @@ namespace Umbraco.Web.Models.ContentEditing [DataMember(Name = "memberGroups")] public IEnumerable Groups { get; set; } - [DataMember(Name = "comments")] - public string Comments { get; set; } + /// + /// Returns the value from the Comments property + /// + public string Comments => GetPropertyValue(Constants.Conventions.Member.Comments); - [DataMember(Name = "isLockedOut")] - public bool IsLockedOut { get; set; } + /// + /// Returns the value from the IsLockedOut property + /// + public bool IsLockedOut => GetPropertyValue(Constants.Conventions.Member.IsLockedOut); - [DataMember(Name = "isApproved")] - public bool IsApproved { get; set; } - + /// + /// Returns the value from the IsApproved property + /// + public bool IsApproved => GetPropertyValue(Constants.Conventions.Member.IsApproved); - // TODO: Need to add question / answer support + private T GetPropertyValue(string alias) + { + var prop = Properties.FirstOrDefault(x => x.Alias == alias); + if (prop == null) return default; + var converted = prop.Value.TryConvertTo(); + return converted.ResultOr(default); + } } } diff --git a/src/Umbraco.Web/Models/Mapping/MemberMapDefinition.cs b/src/Umbraco.Web/Models/Mapping/MemberMapDefinition.cs index 435d349aa3..37f86ae61e 100644 --- a/src/Umbraco.Web/Models/Mapping/MemberMapDefinition.cs +++ b/src/Umbraco.Web/Models/Mapping/MemberMapDefinition.cs @@ -1,17 +1,7 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Web.Security; -using Umbraco.Core; +using Umbraco.Core; using Umbraco.Core.Mapping; using Umbraco.Core.Models; -using Umbraco.Core.Models.Membership; -using Umbraco.Core.Security; -using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; -using Umbraco.Core.Services.Implement; -using UserProfile = Umbraco.Web.Models.ContentEditing.UserProfile; -using Umbraco.Web.Security; namespace Umbraco.Web.Models.Mapping { @@ -32,43 +22,11 @@ namespace Umbraco.Web.Models.Mapping public void DefineMaps(UmbracoMapper mapper) { - mapper.Define((source, context) => new MemberDisplay(), Map); - mapper.Define((source, context) => MemberService.CreateGenericMembershipProviderMember(source.UserName, source.Email, source.UserName, ""), Map); mapper.Define((source, context) => new MemberDisplay(), Map); mapper.Define((source, context) => new MemberBasic(), Map); - mapper.Define((source, context) => new MemberBasic(), Map); mapper.Define((source, context) => new MemberGroupDisplay(), Map); mapper.Define((source, context) => new ContentPropertyCollectionDto(), Map); - } - - private void Map(MembershipUser source, MemberDisplay target, MapperContext context) - { - //first convert to IMember - var member = context.Map(source); - //then convert to MemberDisplay - context.Map(member); - } - - // TODO: SD: I can't remember why this mapping is here? - // Umbraco.Code.MapAll -Properties -CreatorId -Level -Name -CultureInfos -ParentId - // Umbraco.Code.MapAll -Path -SortOrder -DeleteDate -WriterId -VersionId -PasswordQuestion - // Umbraco.Code.MapAll -RawPasswordAnswerValue -FailedPasswordAttempts - private void Map(MembershipUser source, IMember target, MapperContext context) - { - target.Comments = source.Comment; - target.CreateDate = source.CreationDate; - target.Email = source.Email; - target.Id = int.MaxValue; - target.IsApproved = source.IsApproved; - target.IsLockedOut = source.IsLockedOut; - target.Key = source.ProviderUserKey.TryConvertTo().Result; - target.LastLockoutDate = source.LastLockoutDate; - target.LastLoginDate = source.LastLoginDate; - target.LastPasswordChangeDate = source.LastPasswordChangedDate; - target.RawPasswordValue = source.CreationDate > DateTime.MinValue ? Guid.NewGuid().ToString("N") : ""; - target.UpdateDate = source.LastActivityDate; - target.Username = source.UserName; - } + } // Umbraco.Code.MapAll -Properties -Errors -Edited -Updater -Alias -IsChildOfListView // Umbraco.Code.MapAll -Trashed -IsContainer -VariesByCulture @@ -82,7 +40,6 @@ namespace Umbraco.Web.Models.Mapping target.Icon = source.ContentType.Icon; target.Id = source.Id; target.Key = source.Key; - target.MemberProviderFieldMapping = GetMemberProviderFieldMapping(); target.Name = source.Name; target.Owner = _commonMapper.GetOwner(source, context); target.ParentId = source.ParentId; @@ -118,23 +75,6 @@ namespace Umbraco.Web.Models.Mapping target.Username = source.Username; } - //TODO: SD: I can't remember why this mapping is here? - // Umbraco.Code.MapAll -Udi -Properties -ParentId -Path -SortOrder -Edited -Updater - // Umbraco.Code.MapAll -Trashed -Alias -ContentTypeId -ContentTypeAlias -VariesByCulture - private void Map(MembershipUser source, MemberBasic target, MapperContext context) - { - target.CreateDate = source.CreationDate; - target.Email = source.Email; - target.Icon = Constants.Icons.Member; - target.Id = int.MaxValue; - target.Key = source.ProviderUserKey.TryConvertTo().Result; - target.Name = source.UserName; - target.Owner = new UserProfile { Name = "Admin", UserId = -1 }; - target.State = ContentSavedState.Draft; - target.UpdateDate = source.LastActivityDate; - target.Username = source.UserName; -} - // Umbraco.Code.MapAll -Icon -Trashed -ParentId -Alias private void Map(IMemberGroup source, MemberGroupDisplay target, MapperContext context) { @@ -151,18 +91,5 @@ namespace Umbraco.Web.Models.Mapping target.Properties = context.MapEnumerable(source.Properties); } - private static IDictionary GetMemberProviderFieldMapping() - { - var provider = MembershipProviderExtensions.GetMembersMembershipProvider(); - - var umbracoProvider = (IUmbracoMemberTypeMembershipProvider)provider; - - return new Dictionary - { - {Constants.Conventions.Member.IsLockedOut, umbracoProvider.LockPropertyTypeAlias}, - {Constants.Conventions.Member.IsApproved, umbracoProvider.ApprovedPropertyTypeAlias}, - {Constants.Conventions.Member.Comments, umbracoProvider.CommentPropertyTypeAlias} - }; - } } } diff --git a/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs b/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs index 3e000e53d6..616ad81fa4 100644 --- a/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs @@ -1,18 +1,16 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Web.Security; using Umbraco.Core; using Umbraco.Core.Mapping; using Umbraco.Core.Composing; using Umbraco.Core.Models; -using Umbraco.Core.Models.Membership; -using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; using Umbraco.Core.Dictionary; using Umbraco.Web.Security; using Umbraco.Web.Security.Providers; +using Umbraco.Core.Configuration; namespace Umbraco.Web.Models.Mapping { @@ -29,22 +27,25 @@ namespace Umbraco.Web.Models.Mapping private readonly IUmbracoContextAccessor _umbracoContextAccessor; private readonly ILocalizedTextService _localizedTextService; private readonly IMemberTypeService _memberTypeService; - private readonly IUserService _userService; + private readonly IMemberService _memberService; + private readonly IMemberGroupService _memberGroupService; + private readonly IMemberPasswordConfiguration _memberPasswordConfiguration; - public MemberTabsAndPropertiesMapper(ICultureDictionary cultureDictionary, IUmbracoContextAccessor umbracoContextAccessor, ILocalizedTextService localizedTextService, IUserService userService, IMemberTypeService memberTypeService) + public MemberTabsAndPropertiesMapper(ICultureDictionary cultureDictionary, IUmbracoContextAccessor umbracoContextAccessor, ILocalizedTextService localizedTextService, IMemberTypeService memberTypeService, IMemberService memberService, IMemberGroupService memberGroupService, IMemberPasswordConfiguration memberPasswordConfiguration) : base(cultureDictionary, localizedTextService) { _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor)); _localizedTextService = localizedTextService ?? throw new ArgumentNullException(nameof(localizedTextService)); - _userService = userService ?? throw new ArgumentNullException(nameof(userService)); _memberTypeService = memberTypeService ?? throw new ArgumentNullException(nameof(memberTypeService)); + _memberService = memberService ?? throw new ArgumentNullException(nameof(memberService)); + _memberGroupService = memberGroupService ?? throw new ArgumentNullException(nameof(memberGroupService)); + _memberPasswordConfiguration = memberPasswordConfiguration; } /// /// Overridden to deal with custom member properties and permissions. public override IEnumerable> Map(IMember source, MapperContext context) { - var provider = MembershipProviderExtensions.GetMembersMembershipProvider(); var memberType = _memberTypeService.Get(source.ContentTypeId); @@ -55,12 +56,10 @@ namespace Umbraco.Web.Models.Mapping var resolved = base.Map(source, context); - var umbracoProvider = (IUmbracoMemberTypeMembershipProvider)provider; - // This is kind of a hack because a developer is supposed to be allowed to set their property editor - would have been much easier // if we just had all of the membership provider fields on the member table :( // TODO: But is there a way to map the IMember.IsLockedOut to the property ? i dunno. - var isLockedOutProperty = resolved.SelectMany(x => x.Properties).FirstOrDefault(x => x.Alias == umbracoProvider.LockPropertyTypeAlias); + var isLockedOutProperty = resolved.SelectMany(x => x.Properties).FirstOrDefault(x => x.Alias == Constants.Conventions.Member.IsLockedOut); if (isLockedOutProperty?.Value != null && isLockedOutProperty.Value.ToString() != "1") { isLockedOutProperty.View = "readonlyvalue"; @@ -96,7 +95,6 @@ namespace Umbraco.Web.Models.Mapping protected override IEnumerable GetCustomGenericProperties(IContentBase content) { var member = (IMember)content; - var membersProvider = MembershipProviderExtensions.GetMembersMembershipProvider(); var genericProperties = new List { @@ -127,18 +125,16 @@ namespace Umbraco.Web.Models.Mapping { Alias = $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}password", Label = _localizedTextService.Localize("password"), - // NOTE: The value here is a json value - but the only property we care about is the generatedPassword one if it exists, the newPassword exists - // only when creating a new member and we want to have a generated password pre-filled. + Value = new Dictionary { - // TODO: why ignoreCase, what are we doing here?! - {"generatedPassword", member.GetAdditionalDataValueIgnoreCase("GeneratedPassword", null)}, + // TODO: why ignoreCase, what are we doing here?! {"newPassword", member.GetAdditionalDataValueIgnoreCase("NewPassword", null)}, }, // TODO: Hard coding this because the changepassword doesn't necessarily need to be a resolvable (real) property editor View = "changepassword", // initialize the dictionary with the configuration from the default membership provider - Config = GetPasswordConfig(membersProvider, member) + Config = GetPasswordConfig(member) }, new ContentPropertyDisplay { @@ -153,16 +149,17 @@ namespace Umbraco.Web.Models.Mapping return genericProperties; } - private Dictionary GetPasswordConfig(MembersMembershipProvider membersProvider, IMember member) + private Dictionary GetPasswordConfig(IMember member) { - var result = new Dictionary(membersProvider.PasswordConfiguration.GetConfiguration()) + var result = new Dictionary(_memberPasswordConfiguration.GetConfiguration(true)) { // the password change toggle will only be displayed if there is already a password assigned. {"hasPassword", member.RawPasswordValue.IsNullOrWhiteSpace() == false} }; - result["enableReset"] = membersProvider.CanResetPassword(_userService); - result["allowManuallyChangingPassword"] = membersProvider.AllowManuallyChangingPassword; + // This will always be true for members since we always want to allow admins to change a password - so long as that + // user has access to edit members (but that security is taken care of separately) + result["allowManuallyChangingPassword"] = true; return result; } @@ -230,12 +227,13 @@ namespace Umbraco.Web.Models.Mapping return prop; } - internal static IDictionary GetMemberGroupValue(string username) + internal IDictionary GetMemberGroupValue(string username) { - var userRoles = username.IsNullOrWhiteSpace() ? null : Roles.GetRolesForUser(username); + var userRoles = username.IsNullOrWhiteSpace() ? null : _memberService.GetAllRoles(username); // create a dictionary of all roles (except internal roles) + "false" - var result = Roles.GetAllRoles().Distinct() + var result = _memberGroupService.GetAll() + .Select(x => x.Name) // if a role starts with __umbracoRole we won't show it as it's an internal role used for public access .Where(x => x.StartsWith(Constants.Conventions.Member.InternalRolePrefix) == false) .OrderBy(x => x, StringComparer.OrdinalIgnoreCase) diff --git a/src/Umbraco.Web/PasswordConfigurationExtensions.cs b/src/Umbraco.Web/PasswordConfigurationExtensions.cs index 8418531cb7..b12a5b6460 100644 --- a/src/Umbraco.Web/PasswordConfigurationExtensions.cs +++ b/src/Umbraco.Web/PasswordConfigurationExtensions.cs @@ -14,7 +14,8 @@ namespace Umbraco.Web /// /// public static IDictionary GetConfiguration( - this IPasswordConfiguration passwordConfiguration) + this IPasswordConfiguration passwordConfiguration, + bool allowManuallyChangingPassword = false) { return new Dictionary { @@ -24,15 +25,9 @@ namespace Umbraco.Web // that we can consider with IPasswordConfiguration, but these are currently still based on how membership providers worked. {"minNonAlphaNumericChars", passwordConfiguration.RequireNonLetterOrDigit ? 2 : 0}, - // TODO: These are legacy settings - we will always allow administrators to change another users password if the user - // has permission to the user section to edit them. Similarly, when we have ASP.Net identity enabled for members, these legacy settings - // will no longer exist and admins will just be able to change a members' password if they have access to the member section to edit them. - {"allowManuallyChangingPassword", true}, - {"enableReset", false}, // TODO: Actually, this is still used for the member editor, see MemberTabsAndPropertiesMapper.GetPasswordConfig, need to remove that eventually - {"enablePasswordRetrieval", false}, - {"requiresQuestionAnswer", false} - - + // A flag to indicate if the current password box should be shown or not, only a user that has access to change other user/member passwords + // doesn't have to specify the current password for the user/member. A user changing their own password must specify their current password. + {"allowManuallyChangingPassword", allowManuallyChangingPassword}, }; } diff --git a/src/Umbraco.Web/Routing/PublishedRouter.cs b/src/Umbraco.Web/Routing/PublishedRouter.cs index 91fc903ba8..30076ee2b0 100644 --- a/src/Umbraco.Web/Routing/PublishedRouter.cs +++ b/src/Umbraco.Web/Routing/PublishedRouter.cs @@ -1,15 +1,11 @@ using System; -using System.Collections.Generic; using System.Linq; using System.Threading; using System.Globalization; using System.IO; -using System.Web.Security; using Umbraco.Core; -using Umbraco.Core.Cache; using Umbraco.Core.Composing; using Umbraco.Core.Configuration.UmbracoSettings; -using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; diff --git a/src/Umbraco.Web/Security/AppBuilderExtensions.cs b/src/Umbraco.Web/Security/AppBuilderExtensions.cs index 8b56ad4a27..d252b4bc57 100644 --- a/src/Umbraco.Web/Security/AppBuilderExtensions.cs +++ b/src/Umbraco.Web/Security/AppBuilderExtensions.cs @@ -43,7 +43,6 @@ namespace Umbraco.Web.Security IGlobalSettings globalSettings, // TODO: This could probably be optional? IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator, IIpResolver ipResolver) { if (services == null) throw new ArgumentNullException(nameof(services)); @@ -59,7 +58,6 @@ namespace Umbraco.Web.Security contentSettings, globalSettings, passwordConfiguration, - passwordGenerator, ipResolver)); app.SetBackOfficeUserManagerType(); @@ -85,7 +83,6 @@ namespace Umbraco.Web.Security BackOfficeUserStore customUserStore, // TODO: This could probably be optional? IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator, IIpResolver ipResolver) { if (runtimeState == null) throw new ArgumentNullException(nameof(runtimeState)); @@ -98,7 +95,6 @@ namespace Umbraco.Web.Security customUserStore, contentSettings, passwordConfiguration, - passwordGenerator, ipResolver)); app.SetBackOfficeUserManagerType(); diff --git a/src/Umbraco.Web/Security/BackOfficeUserManager.cs b/src/Umbraco.Web/Security/BackOfficeUserManager.cs index 4f170960c2..52bf901bfa 100644 --- a/src/Umbraco.Web/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Web/Security/BackOfficeUserManager.cs @@ -1,9 +1,7 @@ using System; -using System.Linq; using System.Security.Claims; using System.Threading.Tasks; using System.Web; -using System.Web.Security; using Microsoft.AspNet.Identity; using Microsoft.AspNet.Identity.Owin; using Microsoft.Owin.Security.DataProtection; @@ -30,9 +28,8 @@ namespace Umbraco.Web.Security IdentityFactoryOptions options, IContentSection contentSectionConfig, IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator, IIpResolver ipResolver) - : base(store, passwordConfiguration, passwordGenerator, ipResolver) + : base(store, passwordConfiguration, ipResolver) { if (options == null) throw new ArgumentNullException("options"); InitUserManager(this, passwordConfiguration, options.DataProtectionProvider, contentSectionConfig); @@ -60,7 +57,6 @@ namespace Umbraco.Web.Security IContentSection contentSectionConfig, IGlobalSettings globalSettings, IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator, IIpResolver ipResolver) { if (options == null) throw new ArgumentNullException("options"); @@ -68,7 +64,7 @@ namespace Umbraco.Web.Security if (externalLoginService == null) throw new ArgumentNullException("externalLoginService"); var store = new BackOfficeUserStore(userService, entityService, externalLoginService, globalSettings, mapper); - var manager = new BackOfficeUserManager(store, options, contentSectionConfig, passwordConfiguration, passwordGenerator, ipResolver); + var manager = new BackOfficeUserManager(store, options, contentSectionConfig, passwordConfiguration, ipResolver); return manager; } @@ -85,10 +81,9 @@ namespace Umbraco.Web.Security BackOfficeUserStore customUserStore, IContentSection contentSectionConfig, IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator, IIpResolver ipResolver) { - var manager = new BackOfficeUserManager(customUserStore, options, contentSectionConfig, passwordConfiguration, passwordGenerator, ipResolver); + var manager = new BackOfficeUserManager(customUserStore, options, contentSectionConfig, passwordConfiguration, ipResolver); return manager; } #endregion @@ -102,14 +97,14 @@ namespace Umbraco.Web.Security public class BackOfficeUserManager : UserManager where T : BackOfficeIdentityUser { + private PasswordGenerator _passwordGenerator; + public BackOfficeUserManager(IUserStore store, IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator, IIpResolver ipResolver) : base(store) { PasswordConfiguration = passwordConfiguration; - PasswordGenerator = passwordGenerator; IpResolver = ipResolver; } @@ -150,24 +145,6 @@ namespace Umbraco.Web.Security return userIdentity; } - ///// - ///// Initializes the user manager with the correct options - ///// - ///// - ///// - ///// - ///// - ///// - //protected void InitUserManager( - // BackOfficeUserManager manager, - // IPasswordConfiguration passwordConfig, - // IContentSection contentSectionConfig, - // IdentityFactoryOptions options) - //{ - // //NOTE: This method is mostly here for backwards compat - // base.InitUserManager(manager, passwordConfig, options.DataProtectionProvider, contentSectionConfig); - //} - /// /// Initializes the user manager with the correct options /// @@ -265,7 +242,6 @@ namespace Umbraco.Web.Security /// public IBackOfficeUserPasswordChecker BackOfficeUserPasswordChecker { get; set; } public IPasswordConfiguration PasswordConfiguration { get; } - public IPasswordGenerator PasswordGenerator { get; } public IIpResolver IpResolver { get; } /// @@ -274,7 +250,8 @@ namespace Umbraco.Web.Security /// public string GeneratePassword() { - var password = PasswordGenerator.GeneratePassword(PasswordConfiguration); + if (_passwordGenerator == null) _passwordGenerator = new PasswordGenerator(PasswordConfiguration); + var password = _passwordGenerator.GeneratePassword(); return password; } @@ -348,8 +325,6 @@ namespace Umbraco.Web.Security public override Task ResetPasswordAsync(int userId, string token, string newPassword) { var result = base.ResetPasswordAsync(userId, token, newPassword); - if (result.Result.Succeeded) - RaisePasswordResetEvent(userId); return result; } @@ -585,12 +560,6 @@ namespace Umbraco.Web.Security OnPasswordChanged(new IdentityAuditEventArgs(AuditEvent.PasswordChanged, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId)); } - // TODO: I don't think this is required anymore since from 7.7 we no longer display the reset password checkbox since that didn't make sense. - internal void RaisePasswordResetEvent(int userId) - { - OnPasswordReset(new IdentityAuditEventArgs(AuditEvent.PasswordReset, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId)); - } - internal void RaiseResetAccessFailedCountEvent(int userId) { OnResetAccessFailedCount(new IdentityAuditEventArgs(AuditEvent.ResetAccessFailedCount, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId)); diff --git a/src/Umbraco.Web/Security/MembershipHelper.cs b/src/Umbraco.Web/Security/MembershipHelper.cs index fddbfb0f41..c9a507494c 100644 --- a/src/Umbraco.Web/Security/MembershipHelper.cs +++ b/src/Umbraco.Web/Security/MembershipHelper.cs @@ -16,6 +16,7 @@ using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Services; using Umbraco.Web.Editors; using Umbraco.Web.Security.Providers; +using System.ComponentModel.DataAnnotations; namespace Umbraco.Web.Security { @@ -28,7 +29,6 @@ namespace Umbraco.Web.Security private readonly RoleProvider _roleProvider; private readonly IMemberService _memberService; private readonly IMemberTypeService _memberTypeService; - private readonly IUserService _userService; private readonly IPublicAccessService _publicAccessService; private readonly AppCaches _appCaches; private readonly ILogger _logger; @@ -43,7 +43,6 @@ namespace Umbraco.Web.Security RoleProvider roleProvider, IMemberService memberService, IMemberTypeService memberTypeService, - IUserService userService, IPublicAccessService publicAccessService, AppCaches appCaches, ILogger logger @@ -53,7 +52,6 @@ namespace Umbraco.Web.Security MemberCache = memberCache; _memberService = memberService; _memberTypeService = memberTypeService; - _userService = userService; _publicAccessService = publicAccessService; _appCaches = appCaches; _logger = logger; @@ -645,8 +643,8 @@ namespace Umbraco.Web.Security /// public virtual Attempt ChangePassword(string username, ChangingPasswordModel passwordModel, MembershipProvider membershipProvider) { - var passwordChanger = new PasswordChanger(_logger, _userService, HttpContext); - return passwordChanger.ChangePasswordWithMembershipProvider(username, passwordModel, membershipProvider); + var passwordChanger = new PasswordChanger(_logger); + return ChangePasswordWithMembershipProvider(username, passwordModel, membershipProvider); } /// @@ -735,5 +733,63 @@ namespace Umbraco.Web.Security return sb.ToString(); } + /// + /// 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.Warn(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" }) }); + } + + } + } } diff --git a/src/Umbraco.Web/Security/MembershipProviderBase.cs b/src/Umbraco.Web/Security/MembershipProviderBase.cs index 3a0a661e22..5060b417f5 100644 --- a/src/Umbraco.Web/Security/MembershipProviderBase.cs +++ b/src/Umbraco.Web/Security/MembershipProviderBase.cs @@ -52,16 +52,6 @@ namespace Umbraco.Web.Security get { return false; } } - /// - /// Providers can override this setting, by default this is false which means that the provider will - /// authenticate the username + password when ChangePassword is called. This property exists purely for - /// backwards compatibility. - /// - public virtual bool AllowManuallyChangingPassword - { - get { return false; } - } - /// /// Returns the raw password value for a given user /// @@ -307,24 +297,10 @@ namespace Umbraco.Web.Security /// /// true if the password was updated successfully; otherwise, false. /// - /// - /// Checks to ensure the AllowManuallyChangingPassword rule is adhered to - /// public override bool ChangePassword(string username, string oldPassword, string newPassword) { string rawPasswordValue = string.Empty; - if (oldPassword.IsNullOrWhiteSpace() && AllowManuallyChangingPassword == false) - { - //we need to lookup the member since this could be a brand new member without a password set - var rawPassword = GetRawPassword(username); - rawPasswordValue = rawPassword.Success ? rawPassword.Result : string.Empty; - if (rawPassword.Success == false || rawPasswordValue.StartsWith(Constants.Security.EmptyPasswordPrefix) == false) - { - //If the old password is empty and AllowManuallyChangingPassword is false, than this provider cannot just arbitrarily change the password - throw new NotSupportedException("This provider does not support manually changing the password"); - } - } - + var args = new ValidatePasswordEventArgs(username, newPassword, false); OnValidatingPassword(args); @@ -339,14 +315,13 @@ namespace Umbraco.Web.Security // * the member is new and doesn't have a password set // * during installation to set the admin password var installing = Current.RuntimeState.Level == RuntimeLevel.Install; - if (AllowManuallyChangingPassword == false - && (rawPasswordValue.StartsWith(Constants.Security.EmptyPasswordPrefix) - || (installing && oldPassword == "default"))) + if (rawPasswordValue.StartsWith(Constants.Security.EmptyPasswordPrefix) + || (installing && oldPassword == "default")) { return PerformChangePassword(username, oldPassword, newPassword); } - if (AllowManuallyChangingPassword == false) + if (!oldPassword.IsNullOrWhiteSpace()) { if (ValidateUser(username, oldPassword) == false) return false; } @@ -385,7 +360,7 @@ namespace Umbraco.Web.Security throw new NotSupportedException("Updating the password Question and Answer is not available if requiresQuestionAndAnswer is not set in web.config"); } - if (AllowManuallyChangingPassword == false) + if (!password.IsNullOrWhiteSpace()) { if (ValidateUser(username, password) == false) { diff --git a/src/Umbraco.Web/Security/MembershipProviderExtensions.cs b/src/Umbraco.Web/Security/MembershipProviderExtensions.cs index 8d51cd748d..fd2c9c19aa 100644 --- a/src/Umbraco.Web/Security/MembershipProviderExtensions.cs +++ b/src/Umbraco.Web/Security/MembershipProviderExtensions.cs @@ -23,40 +23,6 @@ namespace Umbraco.Web.Security return membershipMember; } - /// - /// Extension method to check if a password can be reset based on a given provider and the current request (logged in user) - /// - /// - /// - /// - /// - /// An Admin can always reset the password - /// - internal static bool CanResetPassword(this MembershipProvider provider, IUserService userService) - { - if (provider == null) throw new ArgumentNullException("provider"); - - var canReset = provider.EnablePasswordReset; - - if (userService == null) return canReset; - - //we need to check for the special case in which a user is an admin - in which case they can reset the password even if EnablePasswordReset == false - if (provider.EnablePasswordReset == false) - { - var identity = Thread.CurrentPrincipal.GetUmbracoIdentity(); - if (identity != null) - { - var user = userService.GetUserById(identity.Id.TryConvertTo().Result); - if (user == null) throw new InvalidOperationException("No user with username " + identity.Username + " found"); - var userIsAdmin = user.IsAdmin(); - if (userIsAdmin) - { - canReset = true; - } - } - } - return canReset; - } /// /// Method to get the Umbraco Members membership provider based on its alias diff --git a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs index 33aae20f80..d393078cc6 100644 --- a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs @@ -17,7 +17,7 @@ namespace Umbraco.Web.Security.Providers /// /// Custom Membership Provider for Umbraco Members (User authentication for Frontend applications NOT umbraco CMS) /// - public class MembersMembershipProvider : UmbracoMembershipProvider, IUmbracoMemberTypeMembershipProvider + public class MembersMembershipProvider : UmbracoMembershipProvider { public MembersMembershipProvider() : this(Current.Services.MemberService, Current.Services.MemberTypeService, Current.UmbracoVersion, Current.HostingEnvironment, Current.IpResolver) diff --git a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs index ff906949a1..d2eeebaeaf 100644 --- a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs @@ -44,16 +44,6 @@ namespace Umbraco.Web.Security.Providers protected abstract MembershipUser ConvertToMembershipUser(TEntity entity); - private bool _allowManuallyChangingPassword = false; - - /// - /// For backwards compatibility, this provider supports this option by default it is false - /// - public override bool AllowManuallyChangingPassword - { - get { return _allowManuallyChangingPassword; } - } - /// /// Initializes the provider. /// @@ -72,8 +62,6 @@ namespace Umbraco.Web.Security.Providers // Initialize base provider class base.Initialize(name, config); - - _allowManuallyChangingPassword = config.GetValue("allowManuallyChangingPassword", false); } /// @@ -531,7 +519,7 @@ namespace Umbraco.Web.Security.Providers }; } - var authenticated = PasswordSecurity.CheckPassword(password, member.RawPasswordValue); + var authenticated = PasswordSecurity.VerifyPassword(password, member.RawPasswordValue); if (authenticated == false) { diff --git a/src/Umbraco.Web/Trees/MemberTreeController.cs b/src/Umbraco.Web/Trees/MemberTreeController.cs index 7c97873a2c..84909d5fda 100644 --- a/src/Umbraco.Web/Trees/MemberTreeController.cs +++ b/src/Umbraco.Web/Trees/MemberTreeController.cs @@ -6,7 +6,6 @@ using System.Net.Http; using System.Net.Http.Formatting; using System.Web.Http; using System.Web.Http.ModelBinding; -using System.Web.Security; using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Services; @@ -37,11 +36,9 @@ namespace Umbraco.Web.Trees public MemberTreeController(UmbracoTreeSearcher treeSearcher) { _treeSearcher = treeSearcher; - _provider = MembershipProviderExtensions.GetMembersMembershipProvider(); } private readonly UmbracoTreeSearcher _treeSearcher; - private readonly MembershipProvider _provider; /// /// Gets an individual tree node diff --git a/src/Umbraco.Web/UmbracoDefaultOwinStartup.cs b/src/Umbraco.Web/UmbracoDefaultOwinStartup.cs index 4f628ea7b5..3ddedd9ea4 100644 --- a/src/Umbraco.Web/UmbracoDefaultOwinStartup.cs +++ b/src/Umbraco.Web/UmbracoDefaultOwinStartup.cs @@ -30,7 +30,6 @@ namespace Umbraco.Web protected IUmbracoSettingsSection UmbracoSettings => Current.Configs.Settings(); protected IUserPasswordConfiguration UserPasswordConfig => Current.Configs.UserPasswordConfig(); protected IRuntimeState RuntimeState => Core.Composing.Current.RuntimeState; - protected IPasswordGenerator PasswordGenerator => Core.Composing.Current.PasswordGenerator; protected ServiceContext Services => Current.Services; protected UmbracoMapper Mapper => Current.Mapper; protected IIpResolver IpResolver => Current.IpResolver; @@ -90,7 +89,6 @@ namespace Umbraco.Web UmbracoSettings.Content, GlobalSettings, UserPasswordConfig, - PasswordGenerator, IpResolver); }