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.Core/Composing/Current.cs b/src/Umbraco.Core/Composing/Current.cs index 8b5eab0139..e0cedca0ce 100644 --- a/src/Umbraco.Core/Composing/Current.cs +++ b/src/Umbraco.Core/Composing/Current.cs @@ -127,9 +127,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 0a2e39592f..4b26fe6201 100644 --- a/src/Umbraco.Core/Runtime/CoreInitialComposer.cs +++ b/src/Umbraco.Core/Runtime/CoreInitialComposer.cs @@ -135,7 +135,6 @@ namespace Umbraco.Core.Runtime composition.SetCultureDictionaryFactory(); composition.Register(f => f.GetInstance().CreateDictionary(), Lifetime.Singleton); - composition.RegisterUnique(); } } } diff --git a/src/Umbraco.Core/Security/PasswordSecurity.cs b/src/Umbraco.Core/Security/PasswordSecurity.cs index 6ffcff67de..b38e125a44 100644 --- a/src/Umbraco.Core/Security/PasswordSecurity.cs +++ b/src/Umbraco.Core/Security/PasswordSecurity.cs @@ -1,19 +1,58 @@ 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) { string salt; @@ -21,34 +60,34 @@ namespace Umbraco.Core.Security 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 +142,21 @@ 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); + 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 +168,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 +194,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 +210,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 +227,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.Tests/Membership/MembershipProviderBaseTests.cs b/src/Umbraco.Tests/Membership/MembershipProviderBaseTests.cs index d629479d95..92d54e9dbe 100644 --- a/src/Umbraco.Tests/Membership/MembershipProviderBaseTests.cs +++ b/src/Umbraco.Tests/Membership/MembershipProviderBaseTests.cs @@ -15,28 +15,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() { 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() { 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() { @@ -47,18 +25,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() { 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 05dd7d47da..a606da9c90 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 e690364dd6..463ef4339c 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 1bcbc1e420..d01721b3b9 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()); - 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 25885d0f04..ba5e691b0e 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 } Current.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..2dd319142b 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 @@ -143,8 +118,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/views/common/overlays/user/user.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/overlays/user/user.controller.js index d098d3dd7c..5e19235dce 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 })) { 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/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/MemberController.cs b/src/Umbraco.Web/Editors/MemberController.cs index 67234b3de2..01ff756990 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,19 @@ 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 PasswordChanger _passwordChanger; + private PasswordSecurity PasswordSecurity => _passwordSecurity ?? (_passwordSecurity = new PasswordSecurity(_passwordConfig)); + private PasswordChanger PasswordChanger => _passwordChanger ?? (_passwordChanger = new PasswordChanger(Logger)); public PagedResult GetPagedResults( int pageNumber = 1, @@ -149,10 +152,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 +166,7 @@ namespace Umbraco.Web.Editors [FileUploadCleanupFilter] [OutgoingEditorModelEvent] [MemberSaveValidation] - public MemberDisplay PostSave( + public async Task PostSave( [ModelBinder(typeof(MemberBinder))] MemberSave contentItem) { @@ -178,6 +181,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 +191,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 +224,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 +274,34 @@ 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 + }; + + 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 - - 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; + contentItem.PersistedContent.WriterId = Security.CurrentUser.Id; //if the user doesn't have access to sensitive values, then we need to check if any of the built in member property types //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 (!Security.CurrentUser.HasAccessToSensitiveData()) { var memberType = Services.MemberTypeService.Get(contentItem.PersistedContent.ContentTypeId); var sensitiveProperties = memberType @@ -346,145 +326,25 @@ namespace Umbraco.Web.Editors } } - //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"); - } - //if they were locked but now they are trying to be unlocked - if (membershipUser.IsLockedOut && contentItem.IsLockedOut == false) + if (contentItem.PersistedContent.IsLockedOut && contentItem.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 && contentItem.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; + // change the password + contentItem.PersistedContent.RawPasswordValue = PasswordChanger.ChangePassword(contentItem.Password, PasswordSecurity); } private static void UpdateName(MemberSave memberSave) @@ -496,111 +356,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/PasswordChanger.cs b/src/Umbraco.Web/Editors/PasswordChanger.cs index e1546cae51..c16a743dd7 100644 --- a/src/Umbraco.Web/Editors/PasswordChanger.cs +++ b/src/Umbraco.Web/Editors/PasswordChanger.cs @@ -1,14 +1,13 @@ using System; using System.ComponentModel.DataAnnotations; using System.Threading.Tasks; -using System.Web; using System.Web.Security; using Umbraco.Core; +using Umbraco.Core.Configuration; 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 +17,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 +40,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,6 +103,14 @@ namespace Umbraco.Web.Editors return Attempt.Succeed(new PasswordChangedModel()); } + public string ChangePassword(ChangingPasswordModel passwordModel, PasswordSecurity passwordSecurity) + { + if (passwordModel.NewPassword.IsNullOrWhiteSpace()) + throw new InvalidOperationException("No password value"); + + return passwordSecurity.HashPasswordForStorage(passwordModel.NewPassword); + } + /// /// Changes password for a member/user given the membership provider and the password change model /// @@ -137,87 +128,9 @@ namespace Umbraco.Web.Editors // 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(); + if (membershipProvider == null) throw new ArgumentNullException(nameof(membershipProvider)); 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. @@ -226,73 +139,34 @@ namespace Umbraco.Web.Editors 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 (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 (passwordModel.OldPassword.IsNullOrWhiteSpace() == false) - { - //if an old password is supplied try to change it + //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" }) }); - } + 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 ex2) + catch (Exception ex) { - _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" }) }); + _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/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/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/MemberSave.cs b/src/Umbraco.Web/Models/ContentEditing/MemberSave.cs index 5132dbc3ec..19b41535a4 100644 --- a/src/Umbraco.Web/Models/ContentEditing/MemberSave.cs +++ b/src/Umbraco.Web/Models/ContentEditing/MemberSave.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; using System.Runtime.Serialization; using Newtonsoft.Json.Linq; using Umbraco.Core.Models; @@ -19,6 +20,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")] diff --git a/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs b/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs index 3e000e53d6..67694d3ea8 100644 --- a/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs @@ -155,14 +155,15 @@ namespace Umbraco.Web.Models.Mapping private Dictionary GetPasswordConfig(MembersMembershipProvider membersProvider, IMember member) { - var result = new Dictionary(membersProvider.PasswordConfiguration.GetConfiguration()) + var result = new Dictionary(membersProvider.PasswordConfiguration.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; } 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/Security/AppBuilderExtensions.cs b/src/Umbraco.Web/Security/AppBuilderExtensions.cs index e8693d9cc6..9d842d1219 100644 --- a/src/Umbraco.Web/Security/AppBuilderExtensions.cs +++ b/src/Umbraco.Web/Security/AppBuilderExtensions.cs @@ -41,8 +41,7 @@ namespace Umbraco.Web.Security IContentSection contentSettings, IGlobalSettings globalSettings, // TODO: This could probably be optional? - IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator) + IPasswordConfiguration passwordConfiguration) { if (services == null) throw new ArgumentNullException(nameof(services)); @@ -56,8 +55,7 @@ namespace Umbraco.Web.Security mapper, contentSettings, globalSettings, - passwordConfiguration, - passwordGenerator)); + passwordConfiguration)); app.SetBackOfficeUserManagerType(); @@ -80,8 +78,7 @@ namespace Umbraco.Web.Security IGlobalSettings globalSettings, BackOfficeUserStore customUserStore, // TODO: This could probably be optional? - IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator) + IPasswordConfiguration passwordConfiguration) { if (runtimeState == null) throw new ArgumentNullException(nameof(runtimeState)); if (customUserStore == null) throw new ArgumentNullException(nameof(customUserStore)); @@ -92,8 +89,7 @@ namespace Umbraco.Web.Security options, customUserStore, contentSettings, - passwordConfiguration, - passwordGenerator)); + passwordConfiguration)); app.SetBackOfficeUserManagerType(); diff --git a/src/Umbraco.Web/Security/BackOfficeUserManager.cs b/src/Umbraco.Web/Security/BackOfficeUserManager.cs index f0044c40d5..4476edbd10 100644 --- a/src/Umbraco.Web/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Web/Security/BackOfficeUserManager.cs @@ -28,9 +28,8 @@ namespace Umbraco.Web.Security IUserStore store, IdentityFactoryOptions options, IContentSection contentSectionConfig, - IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator) - : base(store, passwordConfiguration, passwordGenerator) + IPasswordConfiguration passwordConfiguration) + : base(store, passwordConfiguration) { if (options == null) throw new ArgumentNullException("options"); InitUserManager(this, passwordConfiguration, options.DataProtectionProvider, contentSectionConfig); @@ -57,15 +56,14 @@ namespace Umbraco.Web.Security UmbracoMapper mapper, IContentSection contentSectionConfig, IGlobalSettings globalSettings, - IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator) + IPasswordConfiguration passwordConfiguration) { if (options == null) throw new ArgumentNullException("options"); if (userService == null) throw new ArgumentNullException("userService"); 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); + var manager = new BackOfficeUserManager(store, options, contentSectionConfig, passwordConfiguration); return manager; } @@ -81,10 +79,9 @@ namespace Umbraco.Web.Security IdentityFactoryOptions options, BackOfficeUserStore customUserStore, IContentSection contentSectionConfig, - IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator) + IPasswordConfiguration passwordConfiguration) { - var manager = new BackOfficeUserManager(customUserStore, options, contentSectionConfig, passwordConfiguration, passwordGenerator); + var manager = new BackOfficeUserManager(customUserStore, options, contentSectionConfig, passwordConfiguration); return manager; } #endregion @@ -98,13 +95,13 @@ namespace Umbraco.Web.Security public class BackOfficeUserManager : UserManager where T : BackOfficeIdentityUser { + private PasswordGenerator _passwordGenerator; + public BackOfficeUserManager(IUserStore store, - IPasswordConfiguration passwordConfiguration, - IPasswordGenerator passwordGenerator) + IPasswordConfiguration passwordConfiguration) : base(store) { PasswordConfiguration = passwordConfiguration; - PasswordGenerator = passwordGenerator; } #region What we support do not currently @@ -144,24 +141,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 /// @@ -259,15 +238,17 @@ namespace Umbraco.Web.Security /// public IBackOfficeUserPasswordChecker BackOfficeUserPasswordChecker { get; set; } public IPasswordConfiguration PasswordConfiguration { get; } - public IPasswordGenerator PasswordGenerator { get; } + + /// /// Helper method to generate a password for a user based on the current password validator /// /// public string GeneratePassword() - { - var password = PasswordGenerator.GeneratePassword(PasswordConfiguration); + { + if (_passwordGenerator == null) _passwordGenerator = new PasswordGenerator(PasswordConfiguration); + var password = _passwordGenerator.GeneratePassword(); return password; } @@ -341,8 +322,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; } @@ -578,12 +557,6 @@ namespace Umbraco.Web.Security OnPasswordChanged(new IdentityAuditEventArgs(AuditEvent.PasswordChanged, 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, GetCurrentRequestIpAddress(), affectedUser: userId)); - } - internal void RaiseResetAccessFailedCountEvent(int userId) { OnResetAccessFailedCount(new IdentityAuditEventArgs(AuditEvent.ResetAccessFailedCount, GetCurrentRequestIpAddress(), affectedUser: userId)); diff --git a/src/Umbraco.Web/Security/MembershipHelper.cs b/src/Umbraco.Web/Security/MembershipHelper.cs index fddbfb0f41..da6e846020 100644 --- a/src/Umbraco.Web/Security/MembershipHelper.cs +++ b/src/Umbraco.Web/Security/MembershipHelper.cs @@ -28,7 +28,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 +42,6 @@ namespace Umbraco.Web.Security RoleProvider roleProvider, IMemberService memberService, IMemberTypeService memberTypeService, - IUserService userService, IPublicAccessService publicAccessService, AppCaches appCaches, ILogger logger @@ -53,7 +51,6 @@ namespace Umbraco.Web.Security MemberCache = memberCache; _memberService = memberService; _memberTypeService = memberTypeService; - _userService = userService; _publicAccessService = publicAccessService; _appCaches = appCaches; _logger = logger; @@ -645,7 +642,7 @@ namespace Umbraco.Web.Security /// public virtual Attempt ChangePassword(string username, ChangingPasswordModel passwordModel, MembershipProvider membershipProvider) { - var passwordChanger = new PasswordChanger(_logger, _userService, HttpContext); + var passwordChanger = new PasswordChanger(_logger); return passwordChanger.ChangePasswordWithMembershipProvider(username, passwordModel, membershipProvider); } diff --git a/src/Umbraco.Web/Security/MembershipProviderBase.cs b/src/Umbraco.Web/Security/MembershipProviderBase.cs index 3bd8bf262b..b3d6fa4f52 100644 --- a/src/Umbraco.Web/Security/MembershipProviderBase.cs +++ b/src/Umbraco.Web/Security/MembershipProviderBase.cs @@ -42,16 +42,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 /// @@ -297,24 +287,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); @@ -329,14 +305,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; } @@ -375,7 +350,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 43b970d0d1..3e26bfeb20 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/UmbracoMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs index d1128c574c..f8e165b33d 100644 --- a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs @@ -39,16 +39,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. /// @@ -67,8 +57,6 @@ namespace Umbraco.Web.Security.Providers // Initialize base provider class base.Initialize(name, config); - - _allowManuallyChangingPassword = config.GetValue("allowManuallyChangingPassword", false); } /// @@ -526,7 +514,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/UmbracoDefaultOwinStartup.cs b/src/Umbraco.Web/UmbracoDefaultOwinStartup.cs index 801a09aaae..371333431a 100644 --- a/src/Umbraco.Web/UmbracoDefaultOwinStartup.cs +++ b/src/Umbraco.Web/UmbracoDefaultOwinStartup.cs @@ -29,7 +29,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; @@ -87,8 +86,7 @@ namespace Umbraco.Web Mapper, UmbracoSettings.Content, GlobalSettings, - UserPasswordConfig, - PasswordGenerator); + UserPasswordConfig); } ///