From e47f81efdc0e992e7eebe53814d55c5203ace705 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 27 May 2020 13:48:26 +1000 Subject: [PATCH] Gettting password formats and hashing sorted, ensuring the password format on the user is used --- .../MemberPasswordConfigurationSettings.cs | 2 +- .../UserPasswordConfigurationSettings.cs | 2 +- .../PasswordConfigurationElement.cs | 2 +- .../BackOffice/BackOfficeIdentityUser.cs | 7 ++ .../BackOffice/IdentityMapDefinition.cs | 1 + src/Umbraco.Core/Constants-Security.cs | 3 + src/Umbraco.Core/HealthCheck/HealthCheck.cs | 2 +- .../HealthCheck/HealthCheckAction.cs | 2 +- .../HealthCheck/HealthCheckStatus.cs | 2 +- src/Umbraco.Core/Models/Member.cs | 10 +- .../Models/Membership/IMembershipUser.cs | 5 + src/Umbraco.Core/Models/Membership/IUser.cs | 1 + src/Umbraco.Core/Models/Membership/User.cs | 37 +++++--- .../Models/Membership/UserPasswordSettings.cs | 22 +++++ .../Security/PasswordSecurity.cs | 71 +++++---------- .../BackOffice/BackOfficeUserStore.cs | 3 +- .../Persistence/Factories/UserFactory.cs | 3 +- .../Repositories/Implement/UserRepository.cs | 32 +++---- .../Builders/Interfaces/IWithLoginBuilder.cs | 2 + .../Builders/MemberBuilder.cs | 7 ++ .../Builders/UserBuilder.cs | 7 ++ .../Repositories/UserRepositoryTest.cs | 5 +- .../UmbracoSettings/SecurityElementTests.cs | 5 +- .../Security/PasswordSecurityTests.cs | 23 +---- .../UmbracoServiceMembershipProviderTests.cs | 2 +- .../Repositories/UserRepositoryTest.cs | 3 +- .../BackOfficeOwinUserManagerTests.cs | 3 +- .../Stubs/TestUserPasswordConfig.cs | 3 +- src/Umbraco.Tests/Umbraco.Tests.csproj | 1 - .../Web/Controllers/ContentControllerTests.cs | 4 +- .../Web/Controllers/UsersControllerTests.cs | 2 +- ...oBackOfficeApplicationBuilderExtensions.cs | 1 - ...coBackOfficeServiceCollectionExtensions.cs | 13 ++- .../Runtime/BackOfficeComposer.cs | 1 - .../Security/BackOfficePasswordHasher.cs | 91 +++++++++++++++++++ ...nfigureUmbracoBackOfficeIdentityOptions.cs | 17 +++- src/Umbraco.Web/Editors/MemberController.cs | 13 +-- .../Security/ConfiguredPasswordValidator.cs | 31 +++++-- src/Umbraco.Web/Umbraco.Web.csproj | 1 - 39 files changed, 302 insertions(+), 140 deletions(-) create mode 100644 src/Umbraco.Core/Models/Membership/UserPasswordSettings.cs rename src/{Umbraco.Web => Umbraco.Core}/Security/PasswordSecurity.cs (76%) rename src/{Umbraco.Tests => Umbraco.Tests.UnitTests/Umbraco.Core}/Security/PasswordSecurityTests.cs (78%) create mode 100644 src/Umbraco.Web.BackOffice/Security/BackOfficePasswordHasher.cs diff --git a/src/Umbraco.Configuration/Models/MemberPasswordConfigurationSettings.cs b/src/Umbraco.Configuration/Models/MemberPasswordConfigurationSettings.cs index c7b147349e..5a8313a351 100644 --- a/src/Umbraco.Configuration/Models/MemberPasswordConfigurationSettings.cs +++ b/src/Umbraco.Configuration/Models/MemberPasswordConfigurationSettings.cs @@ -30,7 +30,7 @@ namespace Umbraco.Configuration.Models _configuration.GetValue(Prefix + "RequireUppercase", false); public string HashAlgorithmType => - _configuration.GetValue(Prefix + "HashAlgorithmType", "HMACSHA256"); + _configuration.GetValue(Prefix + "HashAlgorithmType", Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName); // TODO: Need to change to current format when we do members public int MaxFailedAccessAttemptsBeforeLockout => _configuration.GetValue(Prefix + "MaxFailedAccessAttemptsBeforeLockout", 5); diff --git a/src/Umbraco.Configuration/Models/UserPasswordConfigurationSettings.cs b/src/Umbraco.Configuration/Models/UserPasswordConfigurationSettings.cs index 5e68b16203..25ce3e3d9a 100644 --- a/src/Umbraco.Configuration/Models/UserPasswordConfigurationSettings.cs +++ b/src/Umbraco.Configuration/Models/UserPasswordConfigurationSettings.cs @@ -28,7 +28,7 @@ namespace Umbraco.Configuration.Models _configuration.GetValue(Prefix + "RequireUppercase", false); public string HashAlgorithmType => - _configuration.GetValue(Prefix + "HashAlgorithmType", "HMACSHA256"); + _configuration.GetValue(Prefix + "HashAlgorithmType", Constants.Security.AspNetCoreV3PasswordHashAlgorithmName); public int MaxFailedAccessAttemptsBeforeLockout => _configuration.GetValue(Prefix + "MaxFailedAccessAttemptsBeforeLockout", 5); diff --git a/src/Umbraco.Configuration/UmbracoSettings/PasswordConfigurationElement.cs b/src/Umbraco.Configuration/UmbracoSettings/PasswordConfigurationElement.cs index fcc0e1d017..91b5cae7a4 100644 --- a/src/Umbraco.Configuration/UmbracoSettings/PasswordConfigurationElement.cs +++ b/src/Umbraco.Configuration/UmbracoSettings/PasswordConfigurationElement.cs @@ -22,7 +22,7 @@ namespace Umbraco.Core.Configuration.UmbracoSettings [ConfigurationProperty("useLegacyEncoding", DefaultValue = "false")] public bool UseLegacyEncoding => (bool)base["useLegacyEncoding"]; - [ConfigurationProperty("hashAlgorithmType", DefaultValue = "HMACSHA256")] + [ConfigurationProperty("hashAlgorithmType", DefaultValue = Constants.Security.AspNetCoreV3PasswordHashAlgorithmName)] public string HashAlgorithmType => (string)base["hashAlgorithmType"]; [ConfigurationProperty("maxFailedAccessAttemptsBeforeLockout", DefaultValue = "5")] diff --git a/src/Umbraco.Core/BackOffice/BackOfficeIdentityUser.cs b/src/Umbraco.Core/BackOffice/BackOfficeIdentityUser.cs index ea160ef1cf..8df253b296 100644 --- a/src/Umbraco.Core/BackOffice/BackOfficeIdentityUser.cs +++ b/src/Umbraco.Core/BackOffice/BackOfficeIdentityUser.cs @@ -22,6 +22,7 @@ namespace Umbraco.Core.BackOffice private string _name; private int _accessFailedCount; private string _passwordHash; + private string _passwordConfig; private string _culture; private ObservableCollection _logins; private Lazy> _getLogins; @@ -174,6 +175,12 @@ namespace Umbraco.Core.BackOffice set => _beingDirty.SetPropertyValueAndDetectChanges(value, ref _passwordHash, nameof(PasswordHash)); } + public string PasswordConfig + { + get => _passwordConfig; + set => _beingDirty.SetPropertyValueAndDetectChanges(value, ref _passwordConfig, nameof(PasswordConfig)); + } + /// /// Content start nodes assigned to the User (not ones assigned to the user's groups) diff --git a/src/Umbraco.Core/BackOffice/IdentityMapDefinition.cs b/src/Umbraco.Core/BackOffice/IdentityMapDefinition.cs index 59590907f6..e8d16a7903 100644 --- a/src/Umbraco.Core/BackOffice/IdentityMapDefinition.cs +++ b/src/Umbraco.Core/BackOffice/IdentityMapDefinition.cs @@ -57,6 +57,7 @@ namespace Umbraco.Core.BackOffice target.Name = source.Name; target.AccessFailedCount = source.FailedPasswordAttempts; target.PasswordHash = GetPasswordHash(source.RawPasswordValue); + target.PasswordConfig = source.PasswordConfiguration; target.StartContentIds = source.StartContentIds; target.StartMediaIds = source.StartMediaIds; target.Culture = source.GetUserCulture(_textService, _globalSettings).ToString(); // project CultureInfo to string diff --git a/src/Umbraco.Core/Constants-Security.cs b/src/Umbraco.Core/Constants-Security.cs index b90a741f91..491f257f5c 100644 --- a/src/Umbraco.Core/Constants-Security.cs +++ b/src/Umbraco.Core/Constants-Security.cs @@ -53,6 +53,9 @@ public const string AllowedApplicationsClaimType = "http://umbraco.org/2015/02/identity/claims/backoffice/allowedapp"; public const string SessionIdClaimType = "http://umbraco.org/2015/02/identity/claims/backoffice/sessionid"; + public const string AspNetCoreV3PasswordHashAlgorithmName = "PBKDF2.ASPNETCORE.V3"; + public const string AspNetCoreV2PasswordHashAlgorithmName = "PBKDF2.ASPNETCORE.V2"; + public const string AspNetUmbraco8PasswordHashAlgorithmName = "HMACSHA256"; } } } diff --git a/src/Umbraco.Core/HealthCheck/HealthCheck.cs b/src/Umbraco.Core/HealthCheck/HealthCheck.cs index 81aa1e47d4..73defd2fef 100644 --- a/src/Umbraco.Core/HealthCheck/HealthCheck.cs +++ b/src/Umbraco.Core/HealthCheck/HealthCheck.cs @@ -9,7 +9,7 @@ namespace Umbraco.Web.HealthCheck /// /// Provides a base class for health checks. /// - [DataContract(Name = "healtCheck", Namespace = "")] + [DataContract(Name = "healthCheck", Namespace = "")] public abstract class HealthCheck : IDiscoverable { protected HealthCheck() diff --git a/src/Umbraco.Core/HealthCheck/HealthCheckAction.cs b/src/Umbraco.Core/HealthCheck/HealthCheckAction.cs index 124068df72..b64eb6746a 100644 --- a/src/Umbraco.Core/HealthCheck/HealthCheckAction.cs +++ b/src/Umbraco.Core/HealthCheck/HealthCheckAction.cs @@ -4,7 +4,7 @@ using System.Runtime.Serialization; namespace Umbraco.Web.HealthCheck { - [DataContract(Name = "healtCheckAction", Namespace = "")] + [DataContract(Name = "healthCheckAction", Namespace = "")] public class HealthCheckAction { /// diff --git a/src/Umbraco.Core/HealthCheck/HealthCheckStatus.cs b/src/Umbraco.Core/HealthCheck/HealthCheckStatus.cs index 1d06f352c0..245267ff8e 100644 --- a/src/Umbraco.Core/HealthCheck/HealthCheckStatus.cs +++ b/src/Umbraco.Core/HealthCheck/HealthCheckStatus.cs @@ -8,7 +8,7 @@ namespace Umbraco.Web.HealthCheck /// The status returned for a health check when it performs it check /// TODO: This model will be used in the WebApi result so needs attributes for JSON usage /// - [DataContract(Name = "healtCheckStatus", Namespace = "")] + [DataContract(Name = "healthCheckStatus", Namespace = "")] public class HealthCheckStatus { public HealthCheckStatus(string message) diff --git a/src/Umbraco.Core/Models/Member.cs b/src/Umbraco.Core/Models/Member.cs index 640bd36214..706729b71a 100644 --- a/src/Umbraco.Core/Models/Member.cs +++ b/src/Umbraco.Core/Models/Member.cs @@ -18,6 +18,7 @@ namespace Umbraco.Core.Models private string _username; private string _email; private string _rawPasswordValue; + private string _passwordConfig; /// /// Constructor for creating an empty Member object @@ -159,6 +160,13 @@ namespace Umbraco.Core.Models } } + [IgnoreDataMember] + public string PasswordConfiguration + { + get => _passwordConfig; + set => SetPropertyValueAndDetectChanges(value, ref _passwordConfig, nameof(PasswordConfiguration)); + } + /// /// Gets or sets the Groups that Member is part of /// @@ -516,6 +524,6 @@ namespace Umbraco.Core.Models /// [IgnoreDataMember] - public bool HasAdditionalData => _additionalData != null; + public bool HasAdditionalData => _additionalData != null; } } diff --git a/src/Umbraco.Core/Models/Membership/IMembershipUser.cs b/src/Umbraco.Core/Models/Membership/IMembershipUser.cs index 4252900cee..9b1c8a0c07 100644 --- a/src/Umbraco.Core/Models/Membership/IMembershipUser.cs +++ b/src/Umbraco.Core/Models/Membership/IMembershipUser.cs @@ -16,6 +16,11 @@ namespace Umbraco.Core.Models.Membership /// string RawPasswordValue { get; set; } + /// + /// The user's specific password config (i.e. algorithm type, etc...) + /// + string PasswordConfiguration { get; set; } + string Comments { get; set; } bool IsApproved { get; set; } bool IsLockedOut { get; set; } diff --git a/src/Umbraco.Core/Models/Membership/IUser.cs b/src/Umbraco.Core/Models/Membership/IUser.cs index 60cc0ec5f2..3a3a18b5ab 100644 --- a/src/Umbraco.Core/Models/Membership/IUser.cs +++ b/src/Umbraco.Core/Models/Membership/IUser.cs @@ -5,6 +5,7 @@ using Umbraco.Core.Models.Entities; namespace Umbraco.Core.Models.Membership { + /// /// Defines the interface for a /// diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index 80c57dccec..9de7614230 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -41,10 +41,10 @@ namespace Umbraco.Core.Models.Membership public User(IGlobalSettings globalSettings, string name, string email, string username, string rawPasswordValue) : this(globalSettings) { - if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value cannot be null or whitespace.", "name"); - if (string.IsNullOrWhiteSpace(email)) throw new ArgumentException("Value cannot be null or whitespace.", "email"); - if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value cannot be null or whitespace.", "username"); - if (string.IsNullOrEmpty(rawPasswordValue)) throw new ArgumentException("Value cannot be null or empty.", "rawPasswordValue"); + if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(name)); + if (string.IsNullOrWhiteSpace(email)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(email)); + if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(username)); + if (string.IsNullOrEmpty(rawPasswordValue)) throw new ArgumentException("Value cannot be null or empty.", nameof(rawPasswordValue)); _name = name; _email = email; @@ -65,25 +65,29 @@ namespace Umbraco.Core.Models.Membership /// /// /// + /// /// /// /// - public User(IGlobalSettings globalSettings, int id, string name, string email, string username, string rawPasswordValue, IEnumerable userGroups, int[] startContentIds, int[] startMediaIds) + public User(IGlobalSettings globalSettings, int id, string name, string email, string username, + string rawPasswordValue, string passwordConfig, + IEnumerable userGroups, int[] startContentIds, int[] startMediaIds) : this(globalSettings) { //we allow whitespace for this value so just check null - if (rawPasswordValue == null) throw new ArgumentNullException("rawPasswordValue"); - if (userGroups == null) throw new ArgumentNullException("userGroups"); - if (startContentIds == null) throw new ArgumentNullException("startContentIds"); - if (startMediaIds == null) throw new ArgumentNullException("startMediaIds"); - if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value cannot be null or whitespace.", "name"); - if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value cannot be null or whitespace.", "username"); + if (rawPasswordValue == null) throw new ArgumentNullException(nameof(rawPasswordValue)); + if (userGroups == null) throw new ArgumentNullException(nameof(userGroups)); + if (startContentIds == null) throw new ArgumentNullException(nameof(startContentIds)); + if (startMediaIds == null) throw new ArgumentNullException(nameof(startMediaIds)); + if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(name)); + if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(username)); Id = id; _name = name; _email = email; _username = username; _rawPasswordValue = rawPasswordValue; + _passwordConfig = passwordConfig; _userGroups = new HashSet(userGroups); _isApproved = true; _isLockedOut = false; @@ -105,6 +109,7 @@ namespace Umbraco.Core.Models.Membership private DateTime? _invitedDate; private string _email; private string _rawPasswordValue; + private string _passwordConfig; private IEnumerable _allowedSections; private HashSet _userGroups; private bool _isApproved; @@ -147,13 +152,21 @@ namespace Umbraco.Core.Models.Membership get => _email; set => SetPropertyValueAndDetectChanges(value, ref _email, nameof(Email)); } - [DataMember] + + [IgnoreDataMember] public string RawPasswordValue { get => _rawPasswordValue; set => SetPropertyValueAndDetectChanges(value, ref _rawPasswordValue, nameof(RawPasswordValue)); } + [IgnoreDataMember] + public string PasswordConfiguration + { + get => _passwordConfig; + set => SetPropertyValueAndDetectChanges(value, ref _passwordConfig, nameof(PasswordConfiguration)); + } + [DataMember] public bool IsApproved { diff --git a/src/Umbraco.Core/Models/Membership/UserPasswordSettings.cs b/src/Umbraco.Core/Models/Membership/UserPasswordSettings.cs new file mode 100644 index 0000000000..4ce349a1c5 --- /dev/null +++ b/src/Umbraco.Core/Models/Membership/UserPasswordSettings.cs @@ -0,0 +1,22 @@ +using System.Runtime.Serialization; + +namespace Umbraco.Core.Models.Membership +{ + /// + /// The data stored against the user for their password configuration + /// + [DataContract(Name = "userPasswordSettings", Namespace = "")] + public class UserPasswordSettings + { + /// + /// The algorithm name + /// + /// + /// This doesn't explicitly need to map to a 'true' algorithm name, this may match an algorithm name alias that + /// uses many different options such as PBKDF2.ASPNETCORE.V3 which would map to the aspnetcore's v3 implementation of PBKDF2 + /// PBKDF2 with HMAC-SHA256, 128-bit salt, 256-bit subkey, 10000 iterations. + /// + [DataMember(Name = "hashAlgorithm")] + public string HashAlgorithm { get; set; } + } +} diff --git a/src/Umbraco.Web/Security/PasswordSecurity.cs b/src/Umbraco.Core/Security/PasswordSecurity.cs similarity index 76% rename from src/Umbraco.Web/Security/PasswordSecurity.cs rename to src/Umbraco.Core/Security/PasswordSecurity.cs index e061478117..353f2afb0d 100644 --- a/src/Umbraco.Web/Security/PasswordSecurity.cs +++ b/src/Umbraco.Core/Security/PasswordSecurity.cs @@ -1,20 +1,24 @@ 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 { - public IPasswordConfiguration PasswordConfiguration { get; } - public PasswordGenerator _generator; - public ConfiguredPasswordValidator _validator; + // TODO: This class could/should be renamed since it's really purely about legacy hashing, we want to use the new hashing available + // to us but this is here for compatibility purposes. + + // TODO: This class no longer has the logic available to verify the old old old password format, we should + // include this ability so that upgrades for very old versions/data can work and then auto-migrate to the new password format. + + private readonly IPasswordConfiguration _passwordConfiguration; + private readonly PasswordGenerator _generator; /// /// Constructor @@ -22,31 +26,11 @@ namespace Umbraco.Core.Security /// public PasswordSecurity(IPasswordConfiguration passwordConfiguration) { - PasswordConfiguration = passwordConfiguration; + _passwordConfiguration = passwordConfiguration; + _generator = new PasswordGenerator(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(); - } + public string GeneratePassword() => _generator.GeneratePassword(); /// /// Returns a hashed password value used to store in a data store @@ -89,7 +73,7 @@ namespace Umbraco.Core.Security var saltBytes = Convert.FromBase64String(salt); byte[] inArray; - var hashAlgorithm = GetHashAlgorithm(pass); + var hashAlgorithm = GetCurrentHashAlgorithm(); var algorithm = hashAlgorithm as KeyedHashAlgorithm; if (algorithm != null) { @@ -185,37 +169,30 @@ namespace Umbraco.Core.Security } /// - /// Return the hash algorithm to use + /// Return the hash algorithm to use based on the /// /// /// - public HashAlgorithm GetHashAlgorithm(string password) + private HashAlgorithm GetCurrentHashAlgorithm() { - 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; } - /// - /// Encodes the password. - /// - /// The password. - /// The encoded password. - private string LegacyEncodePassword(string password) + public bool SupportHashAlgorithm(string algorithm) { - var hashAlgorith = GetHashAlgorithm(password); - var encodedPassword = Convert.ToBase64String(hashAlgorith.ComputeHash(Encoding.Unicode.GetBytes(password))); - return encodedPassword; + if (algorithm.InvariantEquals(typeof(HMACSHA256).Name)) + return true; + + // TODO: Need to add the old old old format in here too which was just HMACSHA1 IIRC but had a custom key implementation as the password itself + return false; } - - - - } } diff --git a/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserStore.cs b/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserStore.cs index b75735e688..4386aa7861 100644 --- a/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserStore.cs +++ b/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserStore.cs @@ -45,7 +45,6 @@ namespace Umbraco.Core.BackOffice if (userService == null) throw new ArgumentNullException("userService"); if (externalLoginService == null) throw new ArgumentNullException("externalLoginService"); _mapper = mapper; - _userService = userService; _externalLoginService = externalLoginService; } @@ -245,6 +244,7 @@ namespace Umbraco.Core.BackOffice if (string.IsNullOrEmpty(passwordHash)) throw new ArgumentException("Value can't be empty.", nameof(passwordHash)); user.PasswordHash = passwordHash; + // TODO: Need to set the user.PasswordConfig based on what the current configuration is return Task.CompletedTask; } @@ -820,6 +820,7 @@ namespace Umbraco.Core.BackOffice { anythingChanged = true; user.RawPasswordValue = identityUser.PasswordHash; + user.PasswordConfiguration = identityUser.PasswordConfig; } if (identityUser.IsPropertyDirty("Culture") diff --git a/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs b/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs index 8f841b0260..b56e822e92 100644 --- a/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs +++ b/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs @@ -12,7 +12,7 @@ namespace Umbraco.Core.Persistence.Factories { var guidId = dto.Id.ToGuid(); - var user = new User(globalSettings, dto.Id, dto.UserName, dto.Email, dto.Login,dto.Password, + var user = new User(globalSettings, dto.Id, dto.UserName, dto.Email, dto.Login, dto.Password, dto.PasswordConfig, dto.UserGroupDtos.Select(x => ToReadOnlyGroup(x)).ToArray(), dto.UserStartNodeDtos.Where(x => x.StartNodeType == (int)UserStartNodeDto.StartNodeTypeValue.Content).Select(x => x.StartNode).ToArray(), dto.UserStartNodeDtos.Where(x => x.StartNodeType == (int)UserStartNodeDto.StartNodeTypeValue.Media).Select(x => x.StartNode).ToArray()); @@ -64,6 +64,7 @@ namespace Umbraco.Core.Persistence.Factories Login = entity.Username, NoConsole = entity.IsLockedOut, Password = entity.RawPasswordValue, + PasswordConfig = entity.PasswordConfiguration, UserLanguage = entity.Language, UserName = entity.Name, SecurityStampToken = entity.SecurityStamp, diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs index 5311ee8c7f..e191ac08bf 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Linq; using System.Linq.Expressions; using System.Text; -using Newtonsoft.Json; using NPoco; using Umbraco.Core.Cache; using Umbraco.Core.Configuration; @@ -15,6 +14,7 @@ using Umbraco.Core.Persistence.Factories; using Umbraco.Core.Persistence.Mappers; using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Scoping; +using Umbraco.Core.Serialization; namespace Umbraco.Core.Persistence.Repositories.Implement { @@ -26,6 +26,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement private readonly IMapperCollection _mapperCollection; private readonly IGlobalSettings _globalSettings; private readonly IUserPasswordConfiguration _passwordConfiguration; + private readonly IJsonSerializer _jsonSerializer; private string _passwordConfigJson; private bool _passwordConfigInitialized; @@ -39,26 +40,31 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// A dictionary specifying the configuration for user passwords. If this is null then no password configuration will be persisted or read. /// /// - public UserRepository(IScopeAccessor scopeAccessor, AppCaches appCaches, ILogger logger, IMapperCollection mapperCollection, IGlobalSettings globalSettings, IUserPasswordConfiguration passwordConfiguration) + public UserRepository(IScopeAccessor scopeAccessor, AppCaches appCaches, ILogger logger, IMapperCollection mapperCollection, IGlobalSettings globalSettings, IUserPasswordConfiguration passwordConfiguration, IJsonSerializer jsonSerializer) : base(scopeAccessor, appCaches, logger) { _mapperCollection = mapperCollection ?? throw new ArgumentNullException(nameof(mapperCollection)); _globalSettings = globalSettings ?? throw new ArgumentNullException(nameof(globalSettings)); _passwordConfiguration = passwordConfiguration ?? throw new ArgumentNullException(nameof(passwordConfiguration)); + _jsonSerializer = jsonSerializer; } /// /// Returns a serialized dictionary of the password configuration that is stored against the user in the database /// - private string PasswordConfigJson + private string DefaultPasswordConfigJson { get { if (_passwordConfigInitialized) return _passwordConfigJson; - var passwordConfig = new Dictionary { { "hashAlgorithm", _passwordConfiguration.HashAlgorithmType } }; - _passwordConfigJson = passwordConfig == null ? null : JsonConvert.SerializeObject(passwordConfig); + var passwordConfig = new UserPasswordSettings + { + HashAlgorithm = _passwordConfiguration.HashAlgorithmType + }; + + _passwordConfigJson = passwordConfig == null ? null : _jsonSerializer.Serialize(passwordConfig); _passwordConfigInitialized = true; return _passwordConfigJson; } @@ -438,10 +444,8 @@ ORDER BY colName"; 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 - if (PasswordConfigJson != null) - userDto.PasswordConfig = PasswordConfigJson; + // check if we have a user config else use the default + userDto.PasswordConfig = entity.PasswordConfiguration ?? DefaultPasswordConfigJson; var id = Convert.ToInt32(Database.Insert(userDto)); entity.Id = id; @@ -534,13 +538,9 @@ ORDER BY colName"; 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 - if (PasswordConfigJson != null) - { - userDto.PasswordConfig = PasswordConfigJson; - changedCols.Add("passwordConfig"); - } + // check if we have a user config else use the default + userDto.PasswordConfig = entity.PasswordConfiguration ?? DefaultPasswordConfigJson; + changedCols.Add("passwordConfig"); } // If userlogin or the email has changed then need to reset security stamp diff --git a/src/Umbraco.Tests.Common/Builders/Interfaces/IWithLoginBuilder.cs b/src/Umbraco.Tests.Common/Builders/Interfaces/IWithLoginBuilder.cs index dc099f55b9..5c11dd7d05 100644 --- a/src/Umbraco.Tests.Common/Builders/Interfaces/IWithLoginBuilder.cs +++ b/src/Umbraco.Tests.Common/Builders/Interfaces/IWithLoginBuilder.cs @@ -5,5 +5,7 @@ string Username { get; set; } string RawPasswordValue { get; set; } + + string PasswordConfig { get; set; } } } diff --git a/src/Umbraco.Tests.Common/Builders/MemberBuilder.cs b/src/Umbraco.Tests.Common/Builders/MemberBuilder.cs index c29bd32d71..ff954b741e 100644 --- a/src/Umbraco.Tests.Common/Builders/MemberBuilder.cs +++ b/src/Umbraco.Tests.Common/Builders/MemberBuilder.cs @@ -34,6 +34,7 @@ namespace Umbraco.Tests.Common.Builders private string _path; private string _username; private string _rawPasswordValue; + private string _passwordConfig; private string _email; private int? _failedPasswordAttempts; private bool? _isApproved; @@ -237,6 +238,12 @@ namespace Umbraco.Tests.Common.Builders set => _rawPasswordValue = value; } + string IWithLoginBuilder.PasswordConfig + { + get => _passwordConfig; + set => _passwordConfig = value; + } + string IWithEmailBuilder.Email { get => _email; diff --git a/src/Umbraco.Tests.Common/Builders/UserBuilder.cs b/src/Umbraco.Tests.Common/Builders/UserBuilder.cs index 9dd9ff047f..506b24085e 100644 --- a/src/Umbraco.Tests.Common/Builders/UserBuilder.cs +++ b/src/Umbraco.Tests.Common/Builders/UserBuilder.cs @@ -27,6 +27,7 @@ namespace Umbraco.Tests.Common.Builders private string _name; private string _username; private string _rawPasswordValue; + private string _passwordConfig; private string _email; private int? _failedPasswordAttempts; private bool? _isApproved; @@ -183,6 +184,12 @@ namespace Umbraco.Tests.Common.Builders set => _rawPasswordValue = value; } + string IWithLoginBuilder.PasswordConfig + { + get => _passwordConfig; + set => _passwordConfig = value; + } + string IWithEmailBuilder.Email { get => _email; diff --git a/src/Umbraco.Tests.Integration/Persistence/Repositories/UserRepositoryTest.cs b/src/Umbraco.Tests.Integration/Persistence/Repositories/UserRepositoryTest.cs index 23c9e398d3..dddcff4331 100644 --- a/src/Umbraco.Tests.Integration/Persistence/Repositories/UserRepositoryTest.cs +++ b/src/Umbraco.Tests.Integration/Persistence/Repositories/UserRepositoryTest.cs @@ -11,6 +11,7 @@ using Umbraco.Core.Persistence.Mappers; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.Repositories.Implement; using Umbraco.Core.Scoping; +using Umbraco.Core.Serialization; using Umbraco.Tests.Common.Builders.Extensions; using Umbraco.Tests.Integration.Testing; using Umbraco.Tests.Testing; @@ -24,7 +25,7 @@ namespace Umbraco.Tests.Persistence.Repositories private UserRepository CreateRepository(IScopeProvider provider) { var accessor = (IScopeAccessor) provider; - var repository = new UserRepository(accessor, AppCaches.Disabled, Logger, Mappers, GlobalSettings, Mock.Of()); + var repository = new UserRepository(accessor, AppCaches.Disabled, Logger, Mappers, GlobalSettings, Mock.Of(), new JsonNetSerializer()); return repository; } @@ -116,7 +117,7 @@ namespace Umbraco.Tests.Persistence.Repositories var id = user.Id; - var repository2 = new UserRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, Mock.Of(),GlobalSettings, Mock.Of()); + var repository2 = new UserRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, Mock.Of(),GlobalSettings, Mock.Of(), new JsonNetSerializer()); repository2.Delete(user); diff --git a/src/Umbraco.Tests.Integration/Umbraco.Configuration/UmbracoSettings/SecurityElementTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Configuration/UmbracoSettings/SecurityElementTests.cs index 6255bfa790..d97e9915ba 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Configuration/UmbracoSettings/SecurityElementTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Configuration/UmbracoSettings/SecurityElementTests.cs @@ -1,4 +1,5 @@ using NUnit.Framework; +using Umbraco.Core; namespace Umbraco.Tests.Integration.Umbraco.Configuration.UmbracoSettings { @@ -68,7 +69,7 @@ namespace Umbraco.Tests.Integration.Umbraco.Configuration.UmbracoSettings [Test] public void UserPasswordConfiguration_HashAlgorithmType() { - Assert.IsTrue(UserPasswordConfiguration.HashAlgorithmType == "HMACSHA256"); + Assert.IsTrue(UserPasswordConfiguration.HashAlgorithmType == Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName); } [Test] @@ -110,7 +111,7 @@ namespace Umbraco.Tests.Integration.Umbraco.Configuration.UmbracoSettings [Test] public void MemberPasswordConfiguration_HashAlgorithmType() { - Assert.IsTrue(MemberPasswordConfiguration.HashAlgorithmType == "HMACSHA256"); + Assert.IsTrue(MemberPasswordConfiguration.HashAlgorithmType == Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName); } [Test] diff --git a/src/Umbraco.Tests/Security/PasswordSecurityTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/PasswordSecurityTests.cs similarity index 78% rename from src/Umbraco.Tests/Security/PasswordSecurityTests.cs rename to src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/PasswordSecurityTests.cs index b1646edd28..daf1f8e8e0 100644 --- a/src/Umbraco.Tests/Security/PasswordSecurityTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/PasswordSecurityTests.cs @@ -1,26 +1,15 @@ using Moq; using NUnit.Framework; -using System; -using System.Collections.Generic; -using System.Linq; using System.Security.Cryptography; -using System.Text; -using System.Threading.Tasks; +using Umbraco.Core; using Umbraco.Core.Configuration; using Umbraco.Core.Security; -namespace Umbraco.Tests.Security +namespace Umbraco.Tests.UnitTests.Umbraco.Core.Security { [TestFixture] public class PasswordSecurityTests { - [Test] - public void Get_Hash_Algorithm_Default() - { - var passwordSecurity = new PasswordSecurity(Mock.Of(x => x.HashAlgorithmType == "HMACSHA256")); - var alg = passwordSecurity.GetHashAlgorithm("blah"); // not resolved - Assert.IsTrue(alg is HMACSHA256); - } [Test] public void Check_Password_Hashed_Non_KeyedHashAlgorithm() @@ -40,7 +29,7 @@ namespace Umbraco.Tests.Security [Test] public void Check_Password_Hashed_KeyedHashAlgorithm() { - var passwordSecurity = new PasswordSecurity(Mock.Of(x => x.HashAlgorithmType == "HMACSHA256")); + var passwordSecurity = new PasswordSecurity(Mock.Of(x => x.HashAlgorithmType == Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName)); string salt; var pass = "ThisIsAHashedPassword"; @@ -55,7 +44,7 @@ namespace Umbraco.Tests.Security [Test] public void Format_Pass_For_Storage_Hashed() { - var passwordSecurity = new PasswordSecurity(Mock.Of(x => x.HashAlgorithmType == "HMACSHA256")); + var passwordSecurity = new PasswordSecurity(Mock.Of(x => x.HashAlgorithmType == Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName)); var salt = PasswordSecurity.GenerateSalt(); var stored = "ThisIsAHashedPassword"; @@ -68,7 +57,7 @@ namespace Umbraco.Tests.Security [Test] public void Get_Stored_Password_Hashed() { - var passwordSecurity = new PasswordSecurity(Mock.Of(x => x.HashAlgorithmType == "HMACSHA256")); + var passwordSecurity = new PasswordSecurity(Mock.Of(x => x.HashAlgorithmType == Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName)); var salt = PasswordSecurity.GenerateSalt(); var stored = salt + "ThisIsAHashedPassword"; @@ -91,9 +80,7 @@ namespace Umbraco.Tests.Security var result = PasswordSecurity.GenerateSalt(); if (i > 0) - { Assert.AreEqual(lastLength, result.Length); - } lastLength = result.Length; } diff --git a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs index 3875637ecb..4f2cb22b4a 100644 --- a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs +++ b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs @@ -97,7 +97,7 @@ namespace Umbraco.Tests.Membership .Returns(() => createdMember); var provider = new MembersMembershipProvider(membershipServiceMock.Object, memberTypeServiceMock.Object, TestHelper.GetUmbracoVersion(), TestHelper.GetHostingEnvironment(), TestHelper.GetIpResolver()); - provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Hashed" }, { "hashAlgorithmType", "HMACSHA256" } }); + provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Hashed" }, { "hashAlgorithmType", Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName } }); MembershipCreateStatus status; diff --git a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs index 8938a69579..a14eea1d91 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs @@ -14,6 +14,7 @@ using Umbraco.Tests.Testing; using Umbraco.Core.PropertyEditors; using System; using Umbraco.Core.Configuration; +using Umbraco.Core.Serialization; namespace Umbraco.Tests.Persistence.Repositories { @@ -67,7 +68,7 @@ namespace Umbraco.Tests.Persistence.Repositories private UserRepository CreateRepository(IScopeProvider provider) { var accessor = (IScopeAccessor) provider; - var repository = new UserRepository(accessor, AppCaches.Disabled, Logger, Mappers, TestObjects.GetGlobalSettings(), Mock.Of()); + var repository = new UserRepository(accessor, AppCaches.Disabled, Logger, Mappers, TestObjects.GetGlobalSettings(), Mock.Of(), new JsonNetSerializer()); return repository; } diff --git a/src/Umbraco.Tests/Security/BackOfficeOwinUserManagerTests.cs b/src/Umbraco.Tests/Security/BackOfficeOwinUserManagerTests.cs index 705387b3ae..2f31c9d6b2 100644 --- a/src/Umbraco.Tests/Security/BackOfficeOwinUserManagerTests.cs +++ b/src/Umbraco.Tests/Security/BackOfficeOwinUserManagerTests.cs @@ -6,6 +6,7 @@ using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Owin.Security.DataProtection; using Moq; using NUnit.Framework; +using Umbraco.Core; using Umbraco.Core.BackOffice; using Umbraco.Core.Configuration; using Umbraco.Core.Models.Membership; @@ -30,7 +31,7 @@ namespace Umbraco.Tests.Security mockDataProtectionProvider.Setup(x => x.Create(It.IsAny())) .Returns(new Mock().Object); mockPasswordConfiguration.Setup(x => x.HashAlgorithmType) - .Returns("HMACSHA256"); + .Returns(Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName); var userManager = BackOfficeOwinUserManager.Create( mockPasswordConfiguration.Object, diff --git a/src/Umbraco.Tests/TestHelpers/Stubs/TestUserPasswordConfig.cs b/src/Umbraco.Tests/TestHelpers/Stubs/TestUserPasswordConfig.cs index ac89c1e2b5..df99139d82 100644 --- a/src/Umbraco.Tests/TestHelpers/Stubs/TestUserPasswordConfig.cs +++ b/src/Umbraco.Tests/TestHelpers/Stubs/TestUserPasswordConfig.cs @@ -1,4 +1,5 @@ using Umbraco.Core.Configuration; +using Umbraco.Core; namespace Umbraco.Tests.TestHelpers.Stubs { @@ -16,7 +17,7 @@ namespace Umbraco.Tests.TestHelpers.Stubs public bool UseLegacyEncoding => false; - public string HashAlgorithmType => "HMACSHA256"; + public string HashAlgorithmType => Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName; public int MaxFailedAccessAttemptsBeforeLockout => 5; } diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index f43ba6aa5f..0a3787aa8a 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -163,7 +163,6 @@ - diff --git a/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs index ad79391380..65259fc6ac 100644 --- a/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs @@ -59,9 +59,9 @@ namespace Umbraco.Tests.Web.Controllers var userServiceMock = new Mock(); userServiceMock.Setup(service => service.GetUserById(It.IsAny())) - .Returns((int id) => id == 1234 ? new User(TestObjects.GetGlobalSettings(), 1234, "Test", "test@test.com", "test@test.com", "", new List(), new int[0], new int[0]) : null); + .Returns((int id) => id == 1234 ? new User(TestObjects.GetGlobalSettings(), 1234, "Test", "test@test.com", "test@test.com", "", null, new List(), new int[0], new int[0]) : null); userServiceMock.Setup(x => x.GetProfileById(It.IsAny())) - .Returns((int id) => id == 1234 ? new User(TestObjects.GetGlobalSettings(), 1234, "Test", "test@test.com", "test@test.com", "", new List(), new int[0], new int[0]) : null); + .Returns((int id) => id == 1234 ? new User(TestObjects.GetGlobalSettings(), 1234, "Test", "test@test.com", "test@test.com", "", null, new List(), new int[0], new int[0]) : null); userServiceMock.Setup(service => service.GetPermissionsForPath(It.IsAny(), It.IsAny())) .Returns(new EntityPermissionSet(123, new EntityPermissionCollection(new[] { diff --git a/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs index 1158d00b4e..92560cf485 100644 --- a/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/UsersControllerTests.cs @@ -86,7 +86,7 @@ namespace Umbraco.Tests.Web.Controllers userServiceMock.Setup(service => service.GetUserGroupsByAlias(It.IsAny())) .Returns(new[] { Mock.Of(group => group.Id == 123 && group.Alias == "writers" && group.Name == "Writers") }); userServiceMock.Setup(service => service.GetUserById(It.IsAny())) - .Returns((int id) => id == 1234 ? new User(TestObjects.GetGlobalSettings(), 1234, "Test", "test@test.com", "test@test.com", "", new List(), new int[0], new int[0]) : null); + .Returns((int id) => id == 1234 ? new User(TestObjects.GetGlobalSettings(), 1234, "Test", "test@test.com", "test@test.com", "", null, new List(), new int[0], new int[0]) : null); var usersController = new UsersController( Factory.GetInstance(), diff --git a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeApplicationBuilderExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeApplicationBuilderExtensions.cs index 2b60a2fcb0..d0ed409610 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeApplicationBuilderExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeApplicationBuilderExtensions.cs @@ -3,7 +3,6 @@ using Microsoft.AspNetCore.Builder; using Microsoft.Extensions.DependencyInjection; using SixLabors.ImageSharp.Web.DependencyInjection; using Umbraco.Web.BackOffice.Routing; -using Microsoft.AspNetCore.Builder; namespace Umbraco.Extensions { diff --git a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeServiceCollectionExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeServiceCollectionExtensions.cs index 5bf5224b07..ffc209edf9 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBackOfficeServiceCollectionExtensions.cs @@ -3,6 +3,9 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Umbraco.Core; using Umbraco.Core.BackOffice; +using Umbraco.Core.Configuration; +using Umbraco.Core.Security; +using Umbraco.Core.Serialization; using Umbraco.Net; using Umbraco.Web.BackOffice.Security; using Umbraco.Web.Common.AspNetCore; @@ -34,6 +37,7 @@ namespace Umbraco.Extensions .AddDefaultTokenProviders() .AddUserStore() .AddUserManager() + .AddSignInManager() .AddClaimsPrincipalFactory>(); // Configure the options specifically for the UmbracoBackOfficeIdentityOptions instance @@ -53,15 +57,20 @@ namespace Umbraco.Extensions // able to configure IdentityOptions to a specific provider since there is no named options. So we have strongly typed options // and strongly typed ILookupNormalizer and IdentityErrorDescriber since those are 'global' and we need to be unintrusive. + // TODO: Could move all of this to BackOfficeComposer? + // Services used by identity services.TryAddScoped, UserValidator>(); services.TryAddScoped, PasswordValidator>(); - services.TryAddScoped, PasswordHasher>(); + services.TryAddScoped>( + services => new BackOfficePasswordHasher( + new PasswordSecurity(services.GetRequiredService()), + services.GetRequiredService())); services.TryAddScoped, DefaultUserConfirmation>(); services.TryAddScoped, UserClaimsPrincipalFactory>(); services.TryAddScoped>(); - // CUSTOM: + // CUSTOM: services.TryAddScoped(); services.TryAddScoped(); diff --git a/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs b/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs index 2826eddde1..b13cb4a192 100644 --- a/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs +++ b/src/Umbraco.Web.BackOffice/Runtime/BackOfficeComposer.cs @@ -16,7 +16,6 @@ namespace Umbraco.Web.BackOffice.Runtime { composition.RegisterUnique(); composition.RegisterUnique(); - composition.Register(Lifetime.Scope); composition.RegisterUnique(); } diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficePasswordHasher.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficePasswordHasher.cs new file mode 100644 index 0000000000..dc3f7b75ee --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficePasswordHasher.cs @@ -0,0 +1,91 @@ +using Microsoft.AspNetCore.Identity; +using Umbraco.Core.BackOffice; +using Umbraco.Core.Security; +using Umbraco.Core; +using Umbraco.Core.Models.Membership; +using Microsoft.Extensions.Options; +using Umbraco.Core.Serialization; + +namespace Umbraco.Web.BackOffice.Security +{ + /// + /// A password hasher for back office users + /// + public class BackOfficePasswordHasher : PasswordHasher + { + private readonly PasswordSecurity _passwordSecurity; + private readonly IJsonSerializer _jsonSerializer; + + public BackOfficePasswordHasher(PasswordSecurity passwordSecurity, IJsonSerializer jsonSerializer) + { + _passwordSecurity = passwordSecurity; + _jsonSerializer = jsonSerializer; + } + + public override string HashPassword(BackOfficeIdentityUser user, string password) + { + if (!user.PasswordConfig.IsNullOrWhiteSpace()) + { + // check if the (legacy) password security supports this hash algorith and if so then use it + var deserialized = _jsonSerializer.Deserialize(user.PasswordConfig); + if (_passwordSecurity.SupportHashAlgorithm(deserialized.HashAlgorithm)) + return _passwordSecurity.HashPasswordForStorage(password); + + // We will explicitly detect names here since this allows us to future proof these checks. + // The default is PBKDF2.ASPNETCORE.V3: + // PBKDF2 with HMAC-SHA256, 128-bit salt, 256-bit subkey, 10000 iterations. + // The underlying class only lets us change 2 things which is the version: options.CompatibilityMode and the iteration count + // The PBKDF2.ASPNETCORE.V2 settings are: + // PBKDF2 with HMAC-SHA1, 128-bit salt, 256-bit subkey, 1000 iterations. + + switch (deserialized.HashAlgorithm) + { + case Constants.Security.AspNetCoreV3PasswordHashAlgorithmName: + return base.HashPassword(user, password); + case Constants.Security.AspNetCoreV2PasswordHashAlgorithmName: + var v2Hasher = new PasswordHasher(new V2PasswordHasherOptions()); + return v2Hasher.HashPassword(user, password); + } + } + + // else keep the default + return base.HashPassword(user, password); + } + + public override PasswordVerificationResult VerifyHashedPassword(BackOfficeIdentityUser user, string hashedPassword, string providedPassword) + { + if (!user.PasswordConfig.IsNullOrWhiteSpace()) + { + // check if the (legacy) password security supports this hash algorith and if so then use it + var deserialized = _jsonSerializer.Deserialize(user.PasswordConfig); + if (_passwordSecurity.SupportHashAlgorithm(deserialized.HashAlgorithm)) + { + var result = _passwordSecurity.VerifyPassword(providedPassword, hashedPassword); + return result + ? PasswordVerificationResult.Success + : PasswordVerificationResult.Failed; + } + + switch (deserialized.HashAlgorithm) + { + case Constants.Security.AspNetCoreV3PasswordHashAlgorithmName: + return base.VerifyHashedPassword(user, hashedPassword, providedPassword); + case Constants.Security.AspNetCoreV2PasswordHashAlgorithmName: + var v2Hasher = new PasswordHasher(new V2PasswordHasherOptions()); + return v2Hasher.VerifyHashedPassword(user, hashedPassword, providedPassword); + } + } + + // else go the default + return base.VerifyHashedPassword(user, hashedPassword, providedPassword); + } + + private class V2PasswordHasherOptions : IOptions + { + public PasswordHasherOptions Value => new PasswordHasherOptions + { + CompatibilityMode = PasswordHasherCompatibilityMode.IdentityV2 + }; + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureUmbracoBackOfficeIdentityOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureUmbracoBackOfficeIdentityOptions.cs index 83ba6d1b89..245c989f88 100644 --- a/src/Umbraco.Web.BackOffice/Security/ConfigureUmbracoBackOfficeIdentityOptions.cs +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureUmbracoBackOfficeIdentityOptions.cs @@ -1,5 +1,6 @@ using System; using System.Security.Claims; +using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Options; using Umbraco.Core; using Umbraco.Core.BackOffice; @@ -29,12 +30,18 @@ namespace Umbraco.Web.BackOffice.Security options.Lockout.AllowedForNewUsers = true; options.Lockout.DefaultLockoutTimeSpan = TimeSpan.FromDays(30); - options.Password.RequiredLength = _userPasswordConfiguration.RequiredLength; - options.Password.RequireNonAlphanumeric = _userPasswordConfiguration.RequireNonLetterOrDigit; - options.Password.RequireDigit = _userPasswordConfiguration.RequireDigit; - options.Password.RequireLowercase = _userPasswordConfiguration.RequireLowercase; - options.Password.RequireUppercase = _userPasswordConfiguration.RequireUppercase; + ConfigurePasswordOptions(_userPasswordConfiguration, options.Password); + options.Lockout.MaxFailedAccessAttempts = _userPasswordConfiguration.MaxFailedAccessAttemptsBeforeLockout; } + + public static void ConfigurePasswordOptions(IPasswordConfiguration input, PasswordOptions output) + { + output.RequiredLength = input.RequiredLength; + output.RequireNonAlphanumeric = input.RequireNonLetterOrDigit; + output.RequireDigit = input.RequireDigit; + output.RequireLowercase = input.RequireLowercase; + output.RequireUppercase = input.RequireUppercase; + } } } diff --git a/src/Umbraco.Web/Editors/MemberController.cs b/src/Umbraco.Web/Editors/MemberController.cs index 30d34d4bd6..15f031c3a6 100644 --- a/src/Umbraco.Web/Editors/MemberController.cs +++ b/src/Umbraco.Web/Editors/MemberController.cs @@ -31,7 +31,6 @@ using Umbraco.Web.Mvc; using Umbraco.Web.WebApi; using Umbraco.Web.WebApi.Filters; using Constants = Umbraco.Core.Constants; -using Umbraco.Core.Strings; using Umbraco.Core.Mapping; using Umbraco.Web.Routing; @@ -64,12 +63,14 @@ namespace Umbraco.Web.Editors { _passwordConfig = passwordConfig ?? throw new ArgumentNullException(nameof(passwordConfig)); _propertyEditors = propertyEditors ?? throw new ArgumentNullException(nameof(propertyEditors)); + _passwordSecurity = new PasswordSecurity(_passwordConfig); + _passwordValidator = new ConfiguredPasswordValidator(); } private readonly IMemberPasswordConfiguration _passwordConfig; private readonly PropertyEditorCollection _propertyEditors; - private PasswordSecurity _passwordSecurity; - private PasswordSecurity PasswordSecurity => _passwordSecurity ?? (_passwordSecurity = new PasswordSecurity(_passwordConfig)); + private readonly PasswordSecurity _passwordSecurity; + private readonly IPasswordValidator _passwordValidator; public PagedResult GetPagedResults( int pageNumber = 1, @@ -296,7 +297,7 @@ namespace Umbraco.Web.Editors var member = new Member(contentItem.Name, contentItem.Email, contentItem.Username, memberType, true) { CreatorId = Security.CurrentUser.Id, - RawPasswordValue = PasswordSecurity.HashPasswordForStorage(contentItem.Password.NewPassword), + RawPasswordValue = _passwordSecurity.HashPasswordForStorage(contentItem.Password.NewPassword), Comments = contentItem.Comments, IsApproved = contentItem.IsApproved }; @@ -358,7 +359,7 @@ namespace Umbraco.Web.Editors return; // set the password - contentItem.PersistedContent.RawPasswordValue = PasswordSecurity.HashPasswordForStorage(contentItem.Password.NewPassword); + contentItem.PersistedContent.RawPasswordValue = _passwordSecurity.HashPasswordForStorage(contentItem.Password.NewPassword); } private static void UpdateName(MemberSave memberSave) @@ -383,7 +384,7 @@ namespace Umbraco.Web.Editors if (contentItem.Password != null && !contentItem.Password.NewPassword.IsNullOrWhiteSpace()) { - var validPassword = await PasswordSecurity.IsValidPasswordAsync(contentItem.Password.NewPassword); + var validPassword = await _passwordValidator.ValidateAsync(_passwordConfig, contentItem.Password.NewPassword); if (!validPassword) { ModelState.AddPropertyError( diff --git a/src/Umbraco.Web/Security/ConfiguredPasswordValidator.cs b/src/Umbraco.Web/Security/ConfiguredPasswordValidator.cs index 15baef1eaf..bbc9722f69 100644 --- a/src/Umbraco.Web/Security/ConfiguredPasswordValidator.cs +++ b/src/Umbraco.Web/Security/ConfiguredPasswordValidator.cs @@ -1,20 +1,31 @@ using Microsoft.AspNet.Identity; +using System.Collections.Generic; +using System.Threading.Tasks; using Umbraco.Core.Configuration; namespace Umbraco.Core.Security { - /// - /// Ensure that both the normal password validator rules are processed along with the underlying membership provider rules - /// - public class ConfiguredPasswordValidator : PasswordValidator + // NOTE: Migrated to netcore (in a different way) + public interface IPasswordValidator { - public ConfiguredPasswordValidator(IPasswordConfiguration config) + Task>> ValidateAsync(IPasswordConfiguration config, string password); + } + + // NOTE: Migrated to netcore (in a different way) + public class ConfiguredPasswordValidator : PasswordValidator, IPasswordValidator + { + async Task>> IPasswordValidator.ValidateAsync(IPasswordConfiguration passwordConfiguration, string password) { - RequiredLength = config.RequiredLength; - RequireNonLetterOrDigit = config.RequireNonLetterOrDigit; - RequireDigit = config.RequireDigit; - RequireLowercase = config.RequireLowercase; - RequireUppercase = config.RequireUppercase; + RequiredLength = passwordConfiguration.RequiredLength; + RequireNonLetterOrDigit = passwordConfiguration.RequireNonLetterOrDigit; + RequireDigit = passwordConfiguration.RequireDigit; + RequireLowercase = passwordConfiguration.RequireLowercase; + RequireUppercase = passwordConfiguration.RequireUppercase; + + var result = await ValidateAsync(password); + if (result.Succeeded) + return Attempt>.Succeed(); + return Attempt>.Fail(result.Errors); } } } diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 4fbf06a86e..9e885eaca9 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -199,7 +199,6 @@ -