From 5c7528554334483479dff6db6110c243653de8ed Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 19 Jul 2017 20:05:47 +1000 Subject: [PATCH 1/5] U4-8643 Usermanagement - Store password algorithm in Usertable --- src/Umbraco.Core/Models/Rdbms/UserDto.cs | 7 +++++++ .../UpdateUserTables.cs | 16 ++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/Umbraco.Core/Models/Rdbms/UserDto.cs b/src/Umbraco.Core/Models/Rdbms/UserDto.cs index 41620cccaf..6090fbd174 100644 --- a/src/Umbraco.Core/Models/Rdbms/UserDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/UserDto.cs @@ -40,6 +40,13 @@ namespace Umbraco.Core.Models.Rdbms [Column("userPassword")] [Length(500)] public string Password { get; set; } + + /// + /// This will represent a JSON structure of how the password has been created (i.e hash algorithm, iterations) + /// + [Column("passwordConfig")] + [Length(500)] + public string PasswordConfig { get; set; } [Column("userEmail")] public string Email { get; set; } diff --git a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenSevenZero/UpdateUserTables.cs b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenSevenZero/UpdateUserTables.cs index 74238de4ca..3374e4a53f 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenSevenZero/UpdateUserTables.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenSevenZero/UpdateUserTables.cs @@ -1,7 +1,10 @@ using System.Linq; +using System.Web.Security; +using Newtonsoft.Json; using Umbraco.Core.Logging; using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Persistence.SqlSyntax; +using Umbraco.Core.Security; namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSevenSevenZero { @@ -28,6 +31,19 @@ namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSevenSevenZe if (columns.Any(x => x.TableName.InvariantEquals("umbracoUser") && x.ColumnName.InvariantEquals("invitedDate")) == false) Create.Column("invitedDate").OnTable("umbracoUser").AsDateTime().Nullable(); + + if (columns.Any(x => x.TableName.InvariantEquals("umbracoUser") && x.ColumnName.InvariantEquals("passwordConfig")) == false) + { + Create.Column("passwordConfig").OnTable("umbracoUser").AsString(500).Nullable(); + //Check if we have a known config, we only want to store config for hashing + var membershipProvider = MembershipProviderExtensions.GetUsersMembershipProvider(); + if (membershipProvider.PasswordFormat == MembershipPasswordFormat.Hashed) + { + var json = JsonConvert.SerializeObject(new { hashAlgorithm = Membership.HashAlgorithmType }); + Execute.Sql("UPDATE umbracoUser SET passwordConfig = '" + json + "'"); + } + } + } public override void Down() From 63befa55d5ec53fe0ebb1b7c514f0d1a1afcbc3f Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 20 Jul 2017 12:53:09 +1000 Subject: [PATCH 2/5] Updates password changing for users to use ASP.NET Identity, there's still some more work here to do to implement the IUserAwarePasswordHashing and to result some of the TODOs. Also moved all password changing logic to PasswordChanger even for legacy membership providers. --- .../Security/BackOfficeUserManager.cs | 9 +- .../Security/MembershipPasswordHasher.cs | 31 --- .../MembershipProviderPasswordHasher.cs | 47 ++++ src/Umbraco.Core/Umbraco.Core.csproj | 2 +- .../Editors/AuthenticationController.cs | 4 +- .../Editors/CurrentUserController.cs | 5 +- .../Editors/PasswordChangeControllerHelper.cs | 39 --- src/Umbraco.Web/Editors/PasswordChanger.cs | 246 ++++++++++++++++++ src/Umbraco.Web/Editors/UsersController.cs | 6 +- .../Install/InstallSteps/NewInstallStep.cs | 1 + src/Umbraco.Web/Security/MembershipHelper.cs | 125 +-------- src/Umbraco.Web/Security/WebSecurity.cs | 8 +- src/Umbraco.Web/Umbraco.Web.csproj | 2 +- 13 files changed, 320 insertions(+), 205 deletions(-) delete mode 100644 src/Umbraco.Core/Security/MembershipPasswordHasher.cs create mode 100644 src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs delete mode 100644 src/Umbraco.Web/Editors/PasswordChangeControllerHelper.cs create mode 100644 src/Umbraco.Web/Editors/PasswordChanger.cs diff --git a/src/Umbraco.Core/Security/BackOfficeUserManager.cs b/src/Umbraco.Core/Security/BackOfficeUserManager.cs index ad22c1426e..993f0671f3 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserManager.cs @@ -137,7 +137,9 @@ namespace Umbraco.Core.Security /// Initializes the user manager with the correct options /// /// - /// + /// + /// The for the users called UsersMembershipProvider + /// /// /// protected void InitUserManager( @@ -153,11 +155,10 @@ namespace Umbraco.Core.Security }; // Configure validation logic for passwords - var provider = MembershipProviderExtensions.GetUsersMembershipProvider(); - manager.PasswordValidator = new MembershipProviderPasswordValidator(provider); + manager.PasswordValidator = new MembershipProviderPasswordValidator(membershipProvider); //use a custom hasher based on our membership provider - manager.PasswordHasher = new MembershipPasswordHasher(membershipProvider); + manager.PasswordHasher = new MembershipProviderPasswordHasher(membershipProvider); if (dataProtectionProvider != null) { diff --git a/src/Umbraco.Core/Security/MembershipPasswordHasher.cs b/src/Umbraco.Core/Security/MembershipPasswordHasher.cs deleted file mode 100644 index 56daa3efdd..0000000000 --- a/src/Umbraco.Core/Security/MembershipPasswordHasher.cs +++ /dev/null @@ -1,31 +0,0 @@ -using Microsoft.AspNet.Identity; - -namespace Umbraco.Core.Security -{ - /// - /// A custom password hasher that conforms to the current password hashing done in Umbraco - /// - internal class MembershipPasswordHasher : IPasswordHasher - { - private readonly MembershipProviderBase _provider; - - public MembershipPasswordHasher(MembershipProviderBase provider) - { - _provider = provider; - } - - public string HashPassword(string password) - { - return _provider.HashPasswordForStorage(password); - } - - public PasswordVerificationResult VerifyHashedPassword(string hashedPassword, string providedPassword) - { - return _provider.VerifyPassword(providedPassword, hashedPassword) - ? PasswordVerificationResult.Success - : PasswordVerificationResult.Failed; - } - - - } -} \ No newline at end of file diff --git a/src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs b/src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs new file mode 100644 index 0000000000..01f62ca51b --- /dev/null +++ b/src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs @@ -0,0 +1,47 @@ +using System; +using Microsoft.AspNet.Identity; + +namespace Umbraco.Core.Security +{ + public interface IUserAwarePasswordHasher + where TKey : IEquatable + { + string HashPassword(TKey id, string password); + string VerifyHashedPassword(TKey id, string hashedPassword, string providedPassword); + } + + public interface IMembershipProviderPasswordHasher : IPasswordHasher + { + MembershipProviderBase MembershipProvider { get; } + } + + /// + /// A custom password hasher that conforms to the password hashing done with membership providers + /// + public class MembershipProviderPasswordHasher : IMembershipProviderPasswordHasher + { + /// + /// Exposes the underlying MembershipProvider + /// + public MembershipProviderBase MembershipProvider { get; private set; } + + public MembershipProviderPasswordHasher(MembershipProviderBase provider) + { + MembershipProvider = provider; + } + + public string HashPassword(string password) + { + return MembershipProvider.HashPasswordForStorage(password); + } + + public PasswordVerificationResult VerifyHashedPassword(string hashedPassword, string providedPassword) + { + return MembershipProvider.VerifyPassword(providedPassword, hashedPassword) + ? PasswordVerificationResult.Success + : PasswordVerificationResult.Failed; + } + + + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 0edbdc77cf..4d46e6c319 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -663,7 +663,7 @@ - + diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index f61e11a791..f38a9965cd 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -56,7 +56,9 @@ namespace Umbraco.Web.Editors /// [WebApi.UmbracoAuthorize(requireApproval: false)] public IDictionary GetMembershipProviderConfig() - { + { + //TODO: Check if the current PasswordValidator is an IMembershipProviderPasswordValidator, if + //it's not than we should return some generic defaults var provider = Core.Security.MembershipProviderExtensions.GetUsersMembershipProvider(); return provider.GetConfiguration(Services.UserService); } diff --git a/src/Umbraco.Web/Editors/CurrentUserController.cs b/src/Umbraco.Web/Editors/CurrentUserController.cs index 8db46ec1e8..a374c5579d 100644 --- a/src/Umbraco.Web/Editors/CurrentUserController.cs +++ b/src/Umbraco.Web/Editors/CurrentUserController.cs @@ -86,9 +86,10 @@ namespace Umbraco.Web.Editors /// /// If the password is being reset it will return the newly reset password, otherwise will return an empty value /// - public ModelWithNotifications PostChangePassword(ChangingPasswordModel data) + public async Task> PostChangePassword(ChangingPasswordModel data) { - var passwordChangeResult = PasswordChangeControllerHelper.ChangePassword(Security.CurrentUser, data, ModelState, Members); + var passwordChanger = new PasswordChanger(Logger, Services.UserService); + var passwordChangeResult = await passwordChanger.ChangePasswordWithIdentityAsync(Security.CurrentUser, data, ModelState, UserManager); if (passwordChangeResult.Success) { diff --git a/src/Umbraco.Web/Editors/PasswordChangeControllerHelper.cs b/src/Umbraco.Web/Editors/PasswordChangeControllerHelper.cs deleted file mode 100644 index 899e0eb9da..0000000000 --- a/src/Umbraco.Web/Editors/PasswordChangeControllerHelper.cs +++ /dev/null @@ -1,39 +0,0 @@ -using System; -using System.Linq; -using System.Web.Http.ModelBinding; -using Umbraco.Core; -using Umbraco.Core.Models.Membership; -using Umbraco.Web.Models; -using Umbraco.Web.Security; - -namespace Umbraco.Web.Editors -{ - internal class PasswordChangeControllerHelper - { - - public static Attempt ChangePassword( - IUser currentUser, - ChangingPasswordModel data, - ModelStateDictionary modelState, - MembershipHelper membersHelper) - { - var userProvider = Core.Security.MembershipProviderExtensions.GetUsersMembershipProvider(); - - if (userProvider.RequiresQuestionAndAnswer) - { - throw new NotSupportedException("Currently the user editor does not support providers that have RequiresQuestionAndAnswer specified"); - } - - var passwordChangeResult = membersHelper.ChangePassword(currentUser.Username, data, userProvider); - if (passwordChangeResult.Success == false) - { - //it wasn't successful, so add the change error to the model state - var fieldName = passwordChangeResult.Result.ChangeError.MemberNames.FirstOrDefault() ?? "password"; - modelState.AddModelError(fieldName, - passwordChangeResult.Result.ChangeError.ErrorMessage); - } - - return passwordChangeResult; - } - } -} \ No newline at end of file diff --git a/src/Umbraco.Web/Editors/PasswordChanger.cs b/src/Umbraco.Web/Editors/PasswordChanger.cs new file mode 100644 index 0000000000..ffd5a9b0fd --- /dev/null +++ b/src/Umbraco.Web/Editors/PasswordChanger.cs @@ -0,0 +1,246 @@ +using System; +using System.ComponentModel.DataAnnotations; +using System.Linq; +using System.Threading.Tasks; +using System.Web.Http.ModelBinding; +using System.Web.Security; +using Microsoft.AspNet.Identity; +using umbraco.cms.businesslogic.packager; +using Umbraco.Core; +using Umbraco.Core.Logging; +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; + +namespace Umbraco.Web.Editors +{ + internal class PasswordChanger + { + private readonly ILogger _logger; + private readonly IUserService _userService; + + public PasswordChanger(ILogger logger, IUserService userService) + { + _logger = logger; + _userService = userService; + } + + public async Task> ChangePasswordWithIdentityAsync( + IUser currentUser, + ChangingPasswordModel passwordModel, + ModelStateDictionary modelState, + BackOfficeUserManager userMgr) + { + if (passwordModel == null) throw new ArgumentNullException("passwordModel"); + if (userMgr == null) throw new ArgumentNullException("userMgr"); + + //check if this identity implementation is powered by an underlying membership provider (it will be in most cases) + var membershipPasswordHasher = userMgr.PasswordHasher as IMembershipProviderPasswordHasher; + + //check if this identity implementation is powered by an IUserAwarePasswordHasher (it will be by default in 7.7+ but not for upgrades) + var userAwarePasswordHasher = userMgr.PasswordHasher as IUserAwarePasswordHasher; + + if (membershipPasswordHasher != null && userAwarePasswordHasher == null) + { + //if this isn't using an IUserAwarePasswordHasher, then fallback to the old way + if (membershipPasswordHasher.MembershipProvider.RequiresQuestionAndAnswer) + throw new NotSupportedException("Currently the user editor does not support providers that have RequiresQuestionAndAnswer specified"); + return ChangePasswordWithMembershipProvider(currentUser.Username, passwordModel, membershipPasswordHasher.MembershipProvider); + } + + //get the real password validator, thsi should not be null but in some very rare cases it could be, in which case + //we need to create a default password validator to use since we have no idea what it actually is or what it's rules are + //this is an Edge Case! + var passwordValidator = userMgr.PasswordValidator as PasswordValidator + ?? (membershipPasswordHasher != null + ? new MembershipProviderPasswordValidator(membershipPasswordHasher.MembershipProvider) + : new PasswordValidator()); + + //Are we resetting the password?? + if (passwordModel.Reset.HasValue && passwordModel.Reset.Value) + { + //ok, we should be able to reset it + var resetToken = await userMgr.GeneratePasswordResetTokenAsync(currentUser.Id); + var newPass = Membership.GeneratePassword(passwordValidator.RequiredLength, passwordValidator.RequireNonLetterOrDigit ? 2 : 0); + var resetResult = await userMgr.ResetPasswordAsync(currentUser.Id, resetToken, newPass); + + if (resetResult.Succeeded == false) + { + var errors = string.Join(". ", resetResult.Errors); + _logger.Warn(string.Format("Could not reset member password {0}", errors)); + return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not reset password, errors: " + errors, new[] { "resetPassword" }) }); + } + + return Attempt.Succeed(new PasswordChangedModel { ResetPassword = newPass }); + } + + //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 + //TODO: What if the current user is admin? We should allow manually changing then? + 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 suplied try to change it + var changeResult = await userMgr.ChangePasswordAsync(currentUser.Id, passwordModel.OldPassword, passwordModel.NewPassword); + if (changeResult.Succeeded == false) + { + var errors = string.Join(". ", changeResult.Errors); + _logger.Warn(string.Format("Could not change member password {0}", errors)); + return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, errors: " + errors, new[] { "value" }) }); + } + return Attempt.Succeed(new PasswordChangedModel()); + } + + //We shouldn't really get here + return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, invalid information supplied", new[] { "value" }) }); + } + + /// + /// Changes password for a member/user given the membership provider and the password change model + /// + /// + /// + /// + /// + public Attempt ChangePasswordWithMembershipProvider(string username, ChangingPasswordModel passwordModel, MembershipProvider membershipProvider) + { + // 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("passwordModel"); + if (membershipProvider == null) throw new ArgumentNullException("membershipProvider"); + + //Are we resetting the password?? + if (passwordModel.Reset.HasValue && passwordModel.Reset.Value) + { + var canReset = membershipProvider.CanResetPassword(_userService); + 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); + + //return the generated pword + return Attempt.Succeed(new PasswordChangedModel { ResetPassword = newPass }); + } + catch (Exception ex) + { + _logger.WarnWithException("Could not reset member password", ex); + 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" }) }); + } + + //This is an edge case and is only necessary for backwards compatibility: + var umbracoBaseProvider = membershipProvider as MembershipProviderBase; + 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, "", passwordModel.NewPassword); + 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.WarnWithException("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" }) }); + } + } + + //The provider does not support manually chaning the password but no old password 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 suplied 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.WarnWithException("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" }) }); + } + } + + 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.WarnWithException("Could not change member password", ex1); + 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.WarnWithException("Could not retrieve member password", ex2); + return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, error: " + ex2.Message + " (see log for full details)", new[] { "value" }) }); + } + } + + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Editors/UsersController.cs b/src/Umbraco.Web/Editors/UsersController.cs index 0039128e86..9164ea8630 100644 --- a/src/Umbraco.Web/Editors/UsersController.cs +++ b/src/Umbraco.Web/Editors/UsersController.cs @@ -404,7 +404,7 @@ namespace Umbraco.Web.Editors /// /// /// - public UserDisplay PostSaveUser(UserSave userSave) + public async Task PostSaveUser(UserSave userSave) { if (userSave == null) throw new ArgumentNullException("userSave"); @@ -460,7 +460,9 @@ namespace Umbraco.Web.Editors var resetPasswordValue = string.Empty; if (userSave.ChangePassword != null) { - var passwordChangeResult = PasswordChangeControllerHelper.ChangePassword(found, userSave.ChangePassword, ModelState, Members); + var passwordChanger = new PasswordChanger(Logger, Services.UserService); + + var passwordChangeResult = await passwordChanger.ChangePasswordWithIdentityAsync(found, userSave.ChangePassword, ModelState, UserManager); if (passwordChangeResult.Success) { //depending on how the provider is configured, the password may be reset so let's store that for later diff --git a/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs b/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs index 7a6a54f9e7..2d0daf1aa9 100644 --- a/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs +++ b/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs @@ -32,6 +32,7 @@ namespace Umbraco.Web.Install.InstallSteps _applicationContext = applicationContext; } + //TODO: Change all logic in this step to use ASP.NET Identity NOT MembershipProviders private MembershipProvider CurrentProvider { get diff --git a/src/Umbraco.Web/Security/MembershipHelper.cs b/src/Umbraco.Web/Security/MembershipHelper.cs index 81487c41d9..43d3c55c65 100644 --- a/src/Umbraco.Web/Security/MembershipHelper.cs +++ b/src/Umbraco.Web/Security/MembershipHelper.cs @@ -13,6 +13,7 @@ using Umbraco.Core.Security; using Umbraco.Web.Models; using Umbraco.Web.PublishedCache; using Umbraco.Core.Cache; +using Umbraco.Web.Editors; using Umbraco.Web.Security.Providers; using MPE = global::Umbraco.Core.Security.MembershipProviderExtensions; @@ -655,128 +656,8 @@ namespace Umbraco.Web.Security /// public virtual Attempt ChangePassword(string username, ChangingPasswordModel passwordModel, MembershipProvider membershipProvider) { - // 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("passwordModel"); - if (membershipProvider == null) throw new ArgumentNullException("membershipProvider"); - - //Are we resetting the password?? - if (passwordModel.Reset.HasValue && passwordModel.Reset.Value) - { - var canReset = membershipProvider.CanResetPassword(_applicationContext.Services.UserService); - 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); - - //return the generated pword - return Attempt.Succeed(new PasswordChangedModel { ResetPassword = newPass }); - } - catch (Exception ex) - { - LogHelper.WarnWithException("Could not reset member password", ex); - 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" }) }); - } - - //This is an edge case and is only necessary for backwards compatibility: - var umbracoBaseProvider = membershipProvider as MembershipProviderBase; - 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, "", passwordModel.NewPassword); - 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) - { - LogHelper.WarnWithException("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" }) }); - } - } - - //The provider does not support manually chaning the password but no old password 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 suplied 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) - { - LogHelper.WarnWithException("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" }) }); - } - } - - 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) - { - LogHelper.WarnWithException("Could not change member password", ex1); - 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) - { - LogHelper.WarnWithException("Could not retrieve member password", ex2); - return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, error: " + ex2.Message + " (see log for full details)", new[] { "value" }) }); - } + var passwordChanger = new PasswordChanger(_applicationContext.ProfilingLogger.Logger, _applicationContext.Services.UserService); + return passwordChanger.ChangePasswordWithMembershipProvider(username, passwordModel, membershipProvider); } /// diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index e4a29bd6cc..e46250bed8 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -170,10 +170,14 @@ namespace Umbraco.Web.Security /// /// /// + /// + /// This uses ASP.NET Identity to perform the validation + /// public virtual bool ValidateBackOfficeCredentials(string username, string password) { - var membershipProvider = Core.Security.MembershipProviderExtensions.GetUsersMembershipProvider(); - return membershipProvider != null && membershipProvider.ValidateUser(username, password); + var backofficeuser = Mapper.Map(CurrentUser); + backofficeuser.UserName = username; + return UserManager.CheckPasswordAsync(backofficeuser, password).Result; } /// diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index afa4790262..29f4d04f26 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -333,7 +333,7 @@ - + From 85e5b023b77006784003e67f98c819faa41ff8c1 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 20 Jul 2017 13:44:37 +1000 Subject: [PATCH 3/5] Gets UserAwareMembershipProviderPasswordHasher there, updates defaults for MembershipProvider, updates default web.config so the new hashing can be used --- .../Security/BackOfficeUserManager.cs | 160 ++++++++++++++---- .../Security/MembershipProviderBase.cs | 10 +- .../MembershipProviderPasswordHasher.cs | 38 ++++- src/Umbraco.Web.UI/web.Template.config | 2 +- src/Umbraco.Web/Editors/PasswordChanger.cs | 20 +-- 5 files changed, 179 insertions(+), 51 deletions(-) diff --git a/src/Umbraco.Core/Security/BackOfficeUserManager.cs b/src/Umbraco.Core/Security/BackOfficeUserManager.cs index 993f0671f3..b1c4b23bbb 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserManager.cs @@ -158,7 +158,7 @@ namespace Umbraco.Core.Security manager.PasswordValidator = new MembershipProviderPasswordValidator(membershipProvider); //use a custom hasher based on our membership provider - manager.PasswordHasher = new MembershipProviderPasswordHasher(membershipProvider); + manager.PasswordHasher = GetDefaultPasswordHasher(membershipProvider); if (dataProtectionProvider != null) { @@ -194,6 +194,76 @@ namespace Umbraco.Core.Security //manager.SmsService = new SmsService(); } + /// + /// This will determine which password hasher to use based on what is defined in config + /// + /// + protected virtual IPasswordHasher GetDefaultPasswordHasher(MembershipProviderBase provider) + { + //if the current user membership provider is unkown (this would be rare), then return the default password hasher + if (provider.IsUmbracoUsersProvider() == false) + return new PasswordHasher(); + + //if the configured provider has legacy features enabled, then return the membership provider password hasher + if (provider.AllowManuallyChangingPassword || provider.DefaultUseLegacyEncoding) + return new MembershipProviderPasswordHasher(provider); + + //we can use the user aware password hasher (which will be the default and preferred way) + return new UserAwareMembershipProviderPasswordHasher(provider); + } + + /// + /// Gets/sets the default back office user password checker + /// + public IBackOfficeUserPasswordChecker BackOfficeUserPasswordChecker { get; set; } + + /// + /// Helper method to generate a password for a user based on the current password validator + /// + /// + public string GeneratePassword() + { + var passwordValidator = PasswordValidator as PasswordValidator; + + if (passwordValidator == null) + { + var membershipPasswordHasher = PasswordHasher as IMembershipProviderPasswordHasher; + + //get the real password validator, this should not be null but in some very rare cases it could be, in which case + //we need to create a default password validator to use since we have no idea what it actually is or what it's rules are + //this is an Edge Case! + passwordValidator = PasswordValidator as PasswordValidator + ?? (membershipPasswordHasher != null + ? new MembershipProviderPasswordValidator(membershipPasswordHasher.MembershipProvider) + : new PasswordValidator()); + } + + var password = Membership.GeneratePassword( + passwordValidator.RequiredLength, + passwordValidator.RequireNonLetterOrDigit ? 2 : 0); + + var random = new Random(); + + var passwordChars = password.ToCharArray(); + + if (passwordValidator.RequireDigit && passwordChars.ContainsAny(Enumerable.Range(48, 58).Select(x => (char)x))) + password += Convert.ToChar(random.Next(48, 58)); // 0-9 + + if (passwordValidator.RequireLowercase && passwordChars.ContainsAny(Enumerable.Range(97, 123).Select(x => (char)x))) + password += Convert.ToChar(random.Next(97, 123)); // a-z + + if (passwordValidator.RequireUppercase && passwordChars.ContainsAny(Enumerable.Range(65, 91).Select(x => (char)x))) + password += Convert.ToChar(random.Next(65, 91)); // A-Z + + if (passwordValidator.RequireNonLetterOrDigit && passwordChars.ContainsAny(Enumerable.Range(33, 48).Select(x => (char)x))) + password += Convert.ToChar(random.Next(33, 48)); // symbols !"#$%&'()*+,-./ + + return password; + } + + + #region Overrides for password logic + /// /// Logic used to validate a username and password /// @@ -229,43 +299,71 @@ namespace Umbraco.Core.Security return await base.CheckPasswordAsync(user, password); } + public override Task ChangePasswordAsync(int userId, string currentPassword, string newPassword) + { + return base.ChangePasswordAsync(userId, currentPassword, newPassword); + } + /// - /// Gets/sets the default back office user password checker + /// Override to determine how to hash the password /// - public IBackOfficeUserPasswordChecker BackOfficeUserPasswordChecker { get; set; } + /// + /// + /// + /// + /// + /// This method is called anytime the password needs to be hashed for storage (i.e. including when reset password is used) + /// + protected override async Task UpdatePassword(IUserPasswordStore passwordStore, T user, string newPassword) + { + var userAwarePasswordHasher = PasswordHasher as IUserAwarePasswordHasher; + if (userAwarePasswordHasher == null) + return await base.UpdatePassword(passwordStore, user, newPassword); + + var result = await PasswordValidator.ValidateAsync(newPassword); + if (result.Succeeded == false) + return result; + + await passwordStore.SetPasswordHashAsync(user, userAwarePasswordHasher.HashPassword(user, newPassword)); + await UpdateSecurityStampInternal(user); + return IdentityResult.Success; + + + } /// - /// Helper method to generate a password for a user based on the current password validator + /// This is copied from the underlying .NET base class since they decied to not expose it + /// + /// + /// + private async Task UpdateSecurityStampInternal(BackOfficeIdentityUser user) + { + if (SupportsUserSecurityStamp == false) + return; + await GetSecurityStore().SetSecurityStampAsync(user, NewSecurityStamp()); + } + + /// + /// This is copied from the underlying .NET base class since they decied to not expose it /// /// - public string GeneratePassword() + private IUserSecurityStampStore GetSecurityStore() { - var passwordValidator = PasswordValidator as PasswordValidator; - if (passwordValidator != null) - { - var password = Membership.GeneratePassword( - passwordValidator.RequiredLength, - passwordValidator.RequireNonLetterOrDigit ? 2 : 0); - - var random = new Random(); - - var passwordChars = password.ToCharArray(); - - if (passwordValidator.RequireDigit && passwordChars.ContainsAny(Enumerable.Range(48, 58).Select(x => (char)x))) - password += Convert.ToChar(random.Next(48, 58)); // 0-9 - - if (passwordValidator.RequireLowercase && passwordChars.ContainsAny(Enumerable.Range(97, 123).Select(x => (char)x))) - password += Convert.ToChar(random.Next(97, 123)); // a-z - - if (passwordValidator.RequireUppercase && passwordChars.ContainsAny(Enumerable.Range(65, 91).Select(x => (char)x))) - password += Convert.ToChar(random.Next(65, 91)); // A-Z - - if (passwordValidator.RequireNonLetterOrDigit && passwordChars.ContainsAny(Enumerable.Range(33, 48).Select(x => (char)x))) - password += Convert.ToChar(random.Next(33, 48)); // symbols !"#$%&'()*+,-./ - - return password; - } - throw new NotSupportedException("Cannot generate a password since the type of the password validator (" + PasswordValidator.GetType() + ") is not known"); + var store = Store as IUserSecurityStampStore; + if (store == null) + throw new NotSupportedException("The current user store does not implement " + typeof(IUserSecurityStampStore<>)); + return store; } + + /// + /// This is copied from the underlying .NET base class since they decied to not expose it + /// + /// + private static string NewSecurityStamp() + { + return Guid.NewGuid().ToString(); + } + + #endregion } } diff --git a/src/Umbraco.Core/Security/MembershipProviderBase.cs b/src/Umbraco.Core/Security/MembershipProviderBase.cs index 4d4dad3fe8..305ef2b42d 100644 --- a/src/Umbraco.Core/Security/MembershipProviderBase.cs +++ b/src/Umbraco.Core/Security/MembershipProviderBase.cs @@ -32,19 +32,19 @@ namespace Umbraco.Core.Security } /// - /// Providers can override this setting, default is 7 + /// Providers can override this setting, default is 10 /// public virtual int DefaultMinPasswordLength { - get { return 7; } + get { return 10; } } /// - /// Providers can override this setting, default is 1 + /// Providers can override this setting, default is 0 /// public virtual int DefaultMinNonAlphanumericChars { - get { return 1; } + get { return 0; } } /// @@ -224,7 +224,7 @@ namespace Umbraco.Core.Security base.Initialize(name, config); _enablePasswordRetrieval = config.GetValue("enablePasswordRetrieval", false); - _enablePasswordReset = config.GetValue("enablePasswordReset", false); + _enablePasswordReset = config.GetValue("enablePasswordReset", true); _requiresQuestionAndAnswer = config.GetValue("requiresQuestionAndAnswer", false); _requiresUniqueEmail = config.GetValue("requiresUniqueEmail", true); _maxInvalidPasswordAttempts = GetIntValue(config, "maxInvalidPasswordAttempts", 5, false, 0); diff --git a/src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs b/src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs index 01f62ca51b..5697ab68fc 100644 --- a/src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs +++ b/src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs @@ -1,22 +1,52 @@ using System; using Microsoft.AspNet.Identity; +using Umbraco.Core.Models.Identity; namespace Umbraco.Core.Security { - public interface IUserAwarePasswordHasher + /// + /// A password hasher that is User aware so that it can process the hashing based on the user's settings + /// + /// + /// + public interface IUserAwarePasswordHasher + where TUser : class, IUser where TKey : IEquatable { - string HashPassword(TKey id, string password); - string VerifyHashedPassword(TKey id, string hashedPassword, string providedPassword); + string HashPassword(TUser user, string password); + string VerifyHashedPassword(TUser user, string hashedPassword, string providedPassword); } + /// + /// A password hasher that is based on the rules configured for a membership provider + /// public interface IMembershipProviderPasswordHasher : IPasswordHasher { MembershipProviderBase MembershipProvider { get; } } /// - /// A custom password hasher that conforms to the password hashing done with membership providers + /// The default password hasher that is User aware so that it can process the hashing based on the user's settings + /// + public class UserAwareMembershipProviderPasswordHasher : MembershipProviderPasswordHasher, IUserAwarePasswordHasher + { + public UserAwareMembershipProviderPasswordHasher(MembershipProviderBase provider) : base(provider) + { + } + + public string HashPassword(BackOfficeIdentityUser user, string password) + { + throw new NotImplementedException(); + } + + public string VerifyHashedPassword(BackOfficeIdentityUser user, string hashedPassword, string providedPassword) + { + throw new NotImplementedException(); + } + } + + /// + /// A password hasher that conforms to the password hashing done with membership providers /// public class MembershipProviderPasswordHasher : IMembershipProviderPasswordHasher { diff --git a/src/Umbraco.Web.UI/web.Template.config b/src/Umbraco.Web.UI/web.Template.config index 3a484944e6..ff292435ed 100644 --- a/src/Umbraco.Web.UI/web.Template.config +++ b/src/Umbraco.Web.UI/web.Template.config @@ -249,7 +249,7 @@ - + diff --git a/src/Umbraco.Web/Editors/PasswordChanger.cs b/src/Umbraco.Web/Editors/PasswordChanger.cs index ffd5a9b0fd..17fb9eeb19 100644 --- a/src/Umbraco.Web/Editors/PasswordChanger.cs +++ b/src/Umbraco.Web/Editors/PasswordChanger.cs @@ -41,7 +41,7 @@ namespace Umbraco.Web.Editors var membershipPasswordHasher = userMgr.PasswordHasher as IMembershipProviderPasswordHasher; //check if this identity implementation is powered by an IUserAwarePasswordHasher (it will be by default in 7.7+ but not for upgrades) - var userAwarePasswordHasher = userMgr.PasswordHasher as IUserAwarePasswordHasher; + var userAwarePasswordHasher = userMgr.PasswordHasher as IUserAwarePasswordHasher; if (membershipPasswordHasher != null && userAwarePasswordHasher == null) { @@ -49,22 +49,22 @@ namespace Umbraco.Web.Editors if (membershipPasswordHasher.MembershipProvider.RequiresQuestionAndAnswer) throw new NotSupportedException("Currently the user editor does not support providers that have RequiresQuestionAndAnswer specified"); return ChangePasswordWithMembershipProvider(currentUser.Username, passwordModel, membershipPasswordHasher.MembershipProvider); - } + } - //get the real password validator, thsi should not be null but in some very rare cases it could be, in which case - //we need to create a default password validator to use since we have no idea what it actually is or what it's rules are - //this is an Edge Case! - var passwordValidator = userMgr.PasswordValidator as PasswordValidator - ?? (membershipPasswordHasher != null - ? new MembershipProviderPasswordValidator(membershipPasswordHasher.MembershipProvider) - : new PasswordValidator()); + //if we are here, then a IUserAwarePasswordHasher is available, however we cannot proceed in that case if for some odd reason + //the user has configured the membership provider to not be hashed. This will actually never occur because the BackOfficeUserManager + //will throw if it's not hashed, but we should make sure to check anyways (i.e. in case we want to unit test!) + if (membershipPasswordHasher != null && membershipPasswordHasher.MembershipProvider.PasswordFormat != MembershipPasswordFormat.Hashed) + { + throw new InvalidOperationException("The membership provider cannot have a password format of " + membershipPasswordHasher.MembershipProvider.PasswordFormat + " and be configured with secured hashed passwords"); + } //Are we resetting the password?? if (passwordModel.Reset.HasValue && passwordModel.Reset.Value) { //ok, we should be able to reset it var resetToken = await userMgr.GeneratePasswordResetTokenAsync(currentUser.Id); - var newPass = Membership.GeneratePassword(passwordValidator.RequiredLength, passwordValidator.RequireNonLetterOrDigit ? 2 : 0); + var newPass = userMgr.GeneratePassword(); var resetResult = await userMgr.ResetPasswordAsync(currentUser.Id, resetToken, newPass); if (resetResult.Succeeded == false) From b58799eb3d93af6e9f22f5522c43cbe0c8cdfa6d Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 20 Jul 2017 13:58:42 +1000 Subject: [PATCH 4/5] Gets everything implemented and it all works - still uses the old hashing mechanism but everything is ready to be plugged in to support any new types of hashing. --- .../Security/BackOfficeUserManager.cs | 21 ++++++++- .../IMembershipProviderPasswordHasher.cs | 12 ++++++ .../Security/IUserAwarePasswordHasher.cs | 18 ++++++++ .../MembershipProviderPasswordHasher.cs | 43 ------------------- ...erAwareMembershipProviderPasswordHasher.cs | 30 +++++++++++++ src/Umbraco.Core/Umbraco.Core.csproj | 3 ++ 6 files changed, 82 insertions(+), 45 deletions(-) create mode 100644 src/Umbraco.Core/Security/IMembershipProviderPasswordHasher.cs create mode 100644 src/Umbraco.Core/Security/IUserAwarePasswordHasher.cs create mode 100644 src/Umbraco.Core/Security/UserAwareMembershipProviderPasswordHasher.cs diff --git a/src/Umbraco.Core/Security/BackOfficeUserManager.cs b/src/Umbraco.Core/Security/BackOfficeUserManager.cs index b1c4b23bbb..ef712623a5 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserManager.cs @@ -302,8 +302,25 @@ namespace Umbraco.Core.Security public override Task ChangePasswordAsync(int userId, string currentPassword, string newPassword) { return base.ChangePasswordAsync(userId, currentPassword, newPassword); - } - + } + + /// + /// Override to determine how to hash the password + /// + /// + /// + /// + /// + protected override async Task VerifyPasswordAsync(IUserPasswordStore store, T user, string password) + { + var userAwarePasswordHasher = PasswordHasher as IUserAwarePasswordHasher; + if (userAwarePasswordHasher == null) + return await base.VerifyPasswordAsync(store, user, password); + + var hash = await store.GetPasswordHashAsync(user); + return userAwarePasswordHasher.VerifyHashedPassword(user, hash, password) != PasswordVerificationResult.Failed; + } + /// /// Override to determine how to hash the password /// diff --git a/src/Umbraco.Core/Security/IMembershipProviderPasswordHasher.cs b/src/Umbraco.Core/Security/IMembershipProviderPasswordHasher.cs new file mode 100644 index 0000000000..42715d280a --- /dev/null +++ b/src/Umbraco.Core/Security/IMembershipProviderPasswordHasher.cs @@ -0,0 +1,12 @@ +using Microsoft.AspNet.Identity; + +namespace Umbraco.Core.Security +{ + /// + /// A password hasher that is based on the rules configured for a membership provider + /// + public interface IMembershipProviderPasswordHasher : IPasswordHasher + { + MembershipProviderBase MembershipProvider { get; } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Security/IUserAwarePasswordHasher.cs b/src/Umbraco.Core/Security/IUserAwarePasswordHasher.cs new file mode 100644 index 0000000000..cea4d5a144 --- /dev/null +++ b/src/Umbraco.Core/Security/IUserAwarePasswordHasher.cs @@ -0,0 +1,18 @@ +using System; +using Microsoft.AspNet.Identity; + +namespace Umbraco.Core.Security +{ + /// + /// A password hasher that is User aware so that it can process the hashing based on the user's settings + /// + /// + /// + public interface IUserAwarePasswordHasher + where TUser : class, IUser + where TKey : IEquatable + { + string HashPassword(TUser user, string password); + PasswordVerificationResult VerifyHashedPassword(TUser user, string hashedPassword, string providedPassword); + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs b/src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs index 5697ab68fc..f518f99c55 100644 --- a/src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs +++ b/src/Umbraco.Core/Security/MembershipProviderPasswordHasher.cs @@ -1,50 +1,7 @@ -using System; using Microsoft.AspNet.Identity; -using Umbraco.Core.Models.Identity; namespace Umbraco.Core.Security { - /// - /// A password hasher that is User aware so that it can process the hashing based on the user's settings - /// - /// - /// - public interface IUserAwarePasswordHasher - where TUser : class, IUser - where TKey : IEquatable - { - string HashPassword(TUser user, string password); - string VerifyHashedPassword(TUser user, string hashedPassword, string providedPassword); - } - - /// - /// A password hasher that is based on the rules configured for a membership provider - /// - public interface IMembershipProviderPasswordHasher : IPasswordHasher - { - MembershipProviderBase MembershipProvider { get; } - } - - /// - /// The default password hasher that is User aware so that it can process the hashing based on the user's settings - /// - public class UserAwareMembershipProviderPasswordHasher : MembershipProviderPasswordHasher, IUserAwarePasswordHasher - { - public UserAwareMembershipProviderPasswordHasher(MembershipProviderBase provider) : base(provider) - { - } - - public string HashPassword(BackOfficeIdentityUser user, string password) - { - throw new NotImplementedException(); - } - - public string VerifyHashedPassword(BackOfficeIdentityUser user, string hashedPassword, string providedPassword) - { - throw new NotImplementedException(); - } - } - /// /// A password hasher that conforms to the password hashing done with membership providers /// diff --git a/src/Umbraco.Core/Security/UserAwareMembershipProviderPasswordHasher.cs b/src/Umbraco.Core/Security/UserAwareMembershipProviderPasswordHasher.cs new file mode 100644 index 0000000000..7d19d72c3c --- /dev/null +++ b/src/Umbraco.Core/Security/UserAwareMembershipProviderPasswordHasher.cs @@ -0,0 +1,30 @@ +using System; +using Microsoft.AspNet.Identity; +using Umbraco.Core.Models.Identity; + +namespace Umbraco.Core.Security +{ + /// + /// The default password hasher that is User aware so that it can process the hashing based on the user's settings + /// + public class UserAwareMembershipProviderPasswordHasher : MembershipProviderPasswordHasher, IUserAwarePasswordHasher + { + public UserAwareMembershipProviderPasswordHasher(MembershipProviderBase provider) : base(provider) + { + } + + public string HashPassword(BackOfficeIdentityUser user, string password) + { + //TODO: Implement the logic for this, we need to lookup the password format for the user and hash accordingly: http://issues.umbraco.org/issue/U4-10089 + //NOTE: For now this just falls back to the hashing we are currently using + return base.HashPassword(password); + } + + public PasswordVerificationResult VerifyHashedPassword(BackOfficeIdentityUser user, string hashedPassword, string providedPassword) + { + //TODO: Implement the logic for this, we need to lookup the password format for the user and hash accordingly: http://issues.umbraco.org/issue/U4-10089 + //NOTE: For now this just falls back to the hashing we are currently using + return base.VerifyHashedPassword(hashedPassword, providedPassword); + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 4d46e6c319..2a43836b16 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -663,10 +663,13 @@ + + + From 540c3389671af76aa277098d4c1715ccb39a7a3f Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 20 Jul 2017 14:09:51 +1000 Subject: [PATCH 5/5] ensure the password config values are saved when the user is created/updated --- .../Repositories/UserRepository.cs | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs index cb9c471103..03de3dba41 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs @@ -4,6 +4,8 @@ using System.ComponentModel; using System.Linq; using System.Linq.Expressions; using System.Text; +using System.Web.Security; +using Newtonsoft.Json; using Umbraco.Core.Logging; using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models.Membership; @@ -15,6 +17,7 @@ using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Persistence.Relators; using Umbraco.Core.Persistence.SqlSyntax; using Umbraco.Core.Persistence.UnitOfWork; +using Umbraco.Core.Security; namespace Umbraco.Core.Persistence.Repositories { @@ -223,9 +226,18 @@ ORDER BY colName"; if (entity.SecurityStamp.IsNullOrWhiteSpace()) { entity.SecurityStamp = Guid.NewGuid().ToString(); - } + } - var userDto = UserFactory.BuildDto(entity); + var userDto = UserFactory.BuildDto(entity); + + //Check if we have a known config, we only want to store config for hashing + //TODO: This logic will need to be updated when we do http://issues.umbraco.org/issue/U4-10089 + var membershipProvider = MembershipProviderExtensions.GetUsersMembershipProvider(); + if (membershipProvider.PasswordFormat == MembershipPasswordFormat.Hashed) + { + var json = JsonConvert.SerializeObject(new { hashAlgorithm = Membership.HashAlgorithmType }); + userDto.PasswordConfig = json; + } var id = Convert.ToInt32(Database.Insert(userDto)); entity.Id = id; @@ -319,6 +331,15 @@ ORDER BY colName"; userDto.SecurityStampToken = entity.SecurityStamp = Guid.NewGuid().ToString(); changedCols.Add("securityStampToken"); } + + //Check if we have a known config, we only want to store config for hashing + //TODO: This logic will need to be updated when we do http://issues.umbraco.org/issue/U4-10089 + var membershipProvider = MembershipProviderExtensions.GetUsersMembershipProvider(); + if (membershipProvider.PasswordFormat == MembershipPasswordFormat.Hashed) + { + var json = JsonConvert.SerializeObject(new { hashAlgorithm = Membership.HashAlgorithmType }); + userDto.PasswordConfig = json; + } } //only update the changed cols