diff --git a/src/Umbraco.Core/Configuration/Models/MemberPasswordConfigurationSettings.cs b/src/Umbraco.Core/Configuration/Models/MemberPasswordConfigurationSettings.cs index 33afc51db4..708a7e476d 100644 --- a/src/Umbraco.Core/Configuration/Models/MemberPasswordConfigurationSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/MemberPasswordConfigurationSettings.cs @@ -24,7 +24,7 @@ namespace Umbraco.Cms.Core.Configuration.Models public bool RequireUppercase { get; set; } = false; /// - public string HashAlgorithmType { get; set; } = Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName; + public string HashAlgorithmType { get; set; } = Constants.Security.AspNetCoreV3PasswordHashAlgorithmName; /// public int MaxFailedAccessAttemptsBeforeLockout { get; set; } = 5; diff --git a/src/Umbraco.Core/Models/Membership/UserPasswordSettings.cs b/src/Umbraco.Core/Models/Membership/PersistedPasswordSettings.cs similarity index 90% rename from src/Umbraco.Core/Models/Membership/UserPasswordSettings.cs rename to src/Umbraco.Core/Models/Membership/PersistedPasswordSettings.cs index b9f271be2f..d3ae5c5920 100644 --- a/src/Umbraco.Core/Models/Membership/UserPasswordSettings.cs +++ b/src/Umbraco.Core/Models/Membership/PersistedPasswordSettings.cs @@ -1,4 +1,4 @@ -using System.Runtime.Serialization; +using System.Runtime.Serialization; namespace Umbraco.Cms.Core.Models.Membership { @@ -6,7 +6,7 @@ namespace Umbraco.Cms.Core.Models.Membership /// The data stored against the user for their password configuration /// [DataContract(Name = "userPasswordSettings", Namespace = "")] - public class UserPasswordSettings + public class PersistedPasswordSettings { /// /// The algorithm name diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs index 526f5a7279..12d8a4afd2 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs @@ -205,6 +205,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade To("{50A43237-A6F4-49E2-A7A6-5DAD65C84669}"); To("{3D8DADEF-0FDA-4377-A5F0-B52C2110E8F2}"); To("{1303BDCF-2295-4645-9526-2F32E8B35ABD}"); + To("{86AC839A-0D08-4D09-B7B5-027445E255A1}"); //FINAL } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/AddPasswordConfigToMemberTable.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/AddPasswordConfigToMemberTable.cs new file mode 100644 index 0000000000..e01fb2eaab --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/AddPasswordConfigToMemberTable.cs @@ -0,0 +1,23 @@ +using System.Linq; +using Umbraco.Cms.Infrastructure.Persistence.Dtos; + +namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_0_0 +{ + public class AddPasswordConfigToMemberTable : MigrationBase + { + public AddPasswordConfigToMemberTable(IMigrationContext context) + : base(context) + { + } + + /// + /// Adds new columns to members table + /// + public override void Migrate() + { + var columns = SqlSyntax.GetColumnsInSchema(Context.Database).ToList(); + + AddColumnIfNotExists(columns, "passwordConfig"); + } + } +} diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/MemberTableColumns.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/MemberTableColumns.cs index 4ef3d070ec..bf26e97540 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/MemberTableColumns.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/MemberTableColumns.cs @@ -1,4 +1,4 @@ -using System.Linq; +using System.Linq; using Umbraco.Cms.Infrastructure.Persistence.Dtos; namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_0_0 @@ -11,7 +11,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_0_0 } /// - /// Adds new External Login token table + /// Adds new columns to members table /// public override void Migrate() { diff --git a/src/Umbraco.Infrastructure/Persistence/Dtos/MemberDto.cs b/src/Umbraco.Infrastructure/Persistence/Dtos/MemberDto.cs index 4568d30686..be34a473c1 100644 --- a/src/Umbraco.Infrastructure/Persistence/Dtos/MemberDto.cs +++ b/src/Umbraco.Infrastructure/Persistence/Dtos/MemberDto.cs @@ -32,6 +32,14 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Dtos [Constraint(Default = "''")] 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")] + [NullSetting(NullSetting = NullSettings.Null)] + [Length(500)] + public string PasswordConfig { get; set; } + [Column("securityStampToken")] [NullSetting(NullSetting = NullSettings.Null)] [Length(255)] diff --git a/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs b/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs index 76b9a30af0..749633a1f3 100644 --- a/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs +++ b/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs @@ -127,7 +127,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Factories content.Id = dto.NodeId; content.SecurityStamp = dto.SecurityStampToken; content.EmailConfirmedDate = dto.EmailConfirmedDate; - + content.PasswordConfiguration = dto.PasswordConfig; content.Key = nodeDto.UniqueId; content.VersionId = contentVersionDto.Id; @@ -218,7 +218,8 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Factories SecurityStampToken = entity.SecurityStamp, EmailConfirmedDate = entity.EmailConfirmedDate, ContentDto = contentDto, - ContentVersionDto = BuildContentVersionDto(entity, contentDto) + ContentVersionDto = BuildContentVersionDto(entity, contentDto), + PasswordConfig = entity.PasswordConfiguration }; return dto; } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs index c1d2580efd..45c904eff0 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs @@ -2,10 +2,13 @@ using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using NPoco; using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.Persistence.Repositories; using Umbraco.Cms.Core.PropertyEditors; @@ -26,32 +29,67 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement /// public class MemberRepository : ContentRepositoryBase, IMemberRepository { + private readonly MemberPasswordConfigurationSettings _passwordConfiguration; private readonly IMemberTypeRepository _memberTypeRepository; private readonly ITagRepository _tagRepository; private readonly IPasswordHasher _passwordHasher; - private readonly IJsonSerializer _serializer; + private readonly IJsonSerializer _jsonSerializer; private readonly IMemberGroupRepository _memberGroupRepository; private readonly IRepositoryCachePolicy _memberByUsernameCachePolicy; + private bool _passwordConfigInitialized; + private string _passwordConfigJson; - public MemberRepository(IScopeAccessor scopeAccessor, AppCaches cache, ILogger logger, - IMemberTypeRepository memberTypeRepository, IMemberGroupRepository memberGroupRepository, ITagRepository tagRepository, ILanguageRepository languageRepository, IRelationRepository relationRepository, IRelationTypeRepository relationTypeRepository, + public MemberRepository( + IScopeAccessor scopeAccessor, + AppCaches cache, + ILogger logger, + IMemberTypeRepository memberTypeRepository, + IMemberGroupRepository memberGroupRepository, + ITagRepository tagRepository, + ILanguageRepository languageRepository, + IRelationRepository relationRepository, + IRelationTypeRepository relationTypeRepository, IPasswordHasher passwordHasher, Lazy propertyEditors, DataValueReferenceFactoryCollection dataValueReferenceFactories, IDataTypeService dataTypeService, IJsonSerializer serializer, - IEventAggregator eventAggregator) + IEventAggregator eventAggregator, + IOptions passwordConfiguration) : base(scopeAccessor, cache, logger, languageRepository, relationRepository, relationTypeRepository, propertyEditors, dataValueReferenceFactories, dataTypeService, eventAggregator) { _memberTypeRepository = memberTypeRepository ?? throw new ArgumentNullException(nameof(memberTypeRepository)); _tagRepository = tagRepository ?? throw new ArgumentNullException(nameof(tagRepository)); _passwordHasher = passwordHasher; - _serializer = serializer; + _jsonSerializer = serializer; _memberGroupRepository = memberGroupRepository; - + _passwordConfiguration = passwordConfiguration.Value; _memberByUsernameCachePolicy = new DefaultRepositoryCachePolicy(GlobalIsolatedCache, ScopeAccessor, DefaultOptions); } + /// + /// Returns a serialized dictionary of the password configuration that is stored against the member in the database + /// + private string DefaultPasswordConfigJson + { + get + { + if (_passwordConfigInitialized) + { + return _passwordConfigJson; + } + + var passwordConfig = new PersistedPasswordSettings + { + HashAlgorithm = _passwordConfiguration.HashAlgorithmType + }; + + _passwordConfigJson = passwordConfig == null ? null : _jsonSerializer.Serialize(passwordConfig); + _passwordConfigInitialized = true; + return _passwordConfigJson; + } + } + protected override MemberRepository This => this; public override int RecycleBinId => throw new NotSupportedException(); @@ -262,17 +300,20 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement entity.SanitizeEntityPropertiesForXmlStorage(); // create the dto - var dto = ContentBaseFactory.BuildDto(entity); + MemberDto memberDto = ContentBaseFactory.BuildDto(entity); + + // check if we have a user config else use the default + memberDto.PasswordConfig = entity.PasswordConfiguration ?? DefaultPasswordConfigJson; // derive path and level from parent - var parent = GetParentNodeDto(entity.ParentId); + NodeDto parent = GetParentNodeDto(entity.ParentId); var level = parent.Level + 1; // get sort order var sortOrder = GetNewChildSortOrder(entity.ParentId, 0); // persist the node dto - var nodeDto = dto.ContentDto.NodeDto; + NodeDto nodeDto = memberDto.ContentDto.NodeDto; nodeDto.Path = parent.Path; nodeDto.Level = Convert.ToInt16(level); nodeDto.SortOrder = sortOrder; @@ -304,36 +345,36 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement entity.Level = level; // persist the content dto - var contentDto = dto.ContentDto; + var contentDto = memberDto.ContentDto; contentDto.NodeId = nodeDto.NodeId; Database.Insert(contentDto); // persist the content version dto // assumes a new version id and version date (modified date) has been set - var contentVersionDto = dto.ContentVersionDto; + var contentVersionDto = memberDto.ContentVersionDto; contentVersionDto.NodeId = nodeDto.NodeId; contentVersionDto.Current = true; Database.Insert(contentVersionDto); entity.VersionId = contentVersionDto.Id; // persist the member dto - dto.NodeId = nodeDto.NodeId; + memberDto.NodeId = nodeDto.NodeId; // if the password is empty, generate one with the special prefix // this will hash the guid with a salt so should be nicely random if (entity.RawPasswordValue.IsNullOrWhiteSpace()) { - dto.Password = Cms.Core.Constants.Security.EmptyPasswordPrefix + _passwordHasher.HashPassword(Guid.NewGuid().ToString("N")); - entity.RawPasswordValue = dto.Password; + memberDto.Password = Cms.Core.Constants.Security.EmptyPasswordPrefix + _passwordHasher.HashPassword(Guid.NewGuid().ToString("N")); + entity.RawPasswordValue = memberDto.Password; } - Database.Insert(dto); + Database.Insert(memberDto); // persist the property data InsertPropertyValues(entity, 0, out _, out _); - SetEntityTags(entity, _tagRepository, _serializer); + SetEntityTags(entity, _tagRepository, _jsonSerializer); PersistRelations(entity); @@ -368,17 +409,17 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement } // create the dto - MemberDto dto = ContentBaseFactory.BuildDto(entity); + MemberDto memberDto = ContentBaseFactory.BuildDto(entity); // update the node dto - NodeDto nodeDto = dto.ContentDto.NodeDto; + NodeDto nodeDto = memberDto.ContentDto.NodeDto; Database.Update(nodeDto); // update the content dto - Database.Update(dto.ContentDto); + Database.Update(memberDto.ContentDto); // update the content version dto - Database.Update(dto.ContentVersionDto); + Database.Update(memberDto.ContentVersionDto); // update the member dto // but only the changed columns, 'cos we cannot update password if empty @@ -399,6 +440,13 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement changedCols.Add("LoginName"); } + // this can occur from an upgrade + if (memberDto.PasswordConfig.IsNullOrWhiteSpace()) + { + memberDto.PasswordConfig = DefaultPasswordConfigJson; + changedCols.Add("passwordConfig"); + } + // do NOT update the password if it has not changed or if it is null or empty if (entity.IsPropertyDirty("RawPasswordValue") && !string.IsNullOrWhiteSpace(entity.RawPasswordValue)) { @@ -407,33 +455,37 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement // If the security stamp hasn't already updated we need to force it if (entity.IsPropertyDirty("SecurityStamp") == false) { - dto.SecurityStampToken = entity.SecurityStamp = Guid.NewGuid().ToString(); + memberDto.SecurityStampToken = entity.SecurityStamp = Guid.NewGuid().ToString(); changedCols.Add("securityStampToken"); } + + // check if we have a user config else use the default + memberDto.PasswordConfig = entity.PasswordConfiguration ?? DefaultPasswordConfigJson; + changedCols.Add("passwordConfig"); } // If userlogin or the email has changed then need to reset security stamp if (changedCols.Contains("Email") || changedCols.Contains("LoginName")) { - dto.EmailConfirmedDate = null; + memberDto.EmailConfirmedDate = null; changedCols.Add("emailConfirmedDate"); // If the security stamp hasn't already updated we need to force it if (entity.IsPropertyDirty("SecurityStamp") == false) { - dto.SecurityStampToken = entity.SecurityStamp = Guid.NewGuid().ToString(); + memberDto.SecurityStampToken = entity.SecurityStamp = Guid.NewGuid().ToString(); changedCols.Add("securityStampToken"); } } if (changedCols.Count > 0) { - Database.Update(dto, changedCols); + Database.Update(memberDto, changedCols); } ReplacePropertyValues(entity, entity.VersionId, 0, out _, out _); - SetEntityTags(entity, _tagRepository, _serializer); + SetEntityTags(entity, _tagRepository, _jsonSerializer); PersistRelations(entity); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs index 734b3d07a0..0dd6e2d43c 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs @@ -73,7 +73,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement return _passwordConfigJson; } - var passwordConfig = new UserPasswordSettings + var passwordConfig = new PersistedPasswordSettings { HashAlgorithm = _passwordConfiguration.HashAlgorithmType }; @@ -462,7 +462,7 @@ ORDER BY colName"; entity.SecurityStamp = Guid.NewGuid().ToString(); } - var userDto = UserFactory.BuildDto(entity); + UserDto userDto = UserFactory.BuildDto(entity); // check if we have a user config else use the default userDto.PasswordConfig = entity.PasswordConfiguration ?? DefaultPasswordConfigJson; diff --git a/src/Umbraco.Infrastructure/Security/BackOfficePasswordHasher.cs b/src/Umbraco.Infrastructure/Security/BackOfficePasswordHasher.cs index 2132752866..3989ec07e0 100644 --- a/src/Umbraco.Infrastructure/Security/BackOfficePasswordHasher.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficePasswordHasher.cs @@ -1,93 +1,21 @@ -using Microsoft.AspNetCore.Identity; -using Microsoft.Extensions.Options; -using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Serialization; -using Umbraco.Extensions; using Constants = Umbraco.Cms.Core.Constants; namespace Umbraco.Cms.Core.Security { + /// /// A password hasher for back office users /// /// /// This allows us to verify passwords in old formats and roll forward to the latest format /// - public class BackOfficePasswordHasher : PasswordHasher + public class BackOfficePasswordHasher : UmbracoPasswordHasher { - private readonly LegacyPasswordSecurity _legacyPasswordSecurity; - private readonly IJsonSerializer _jsonSerializer; - private readonly PasswordHasher _aspnetV2PasswordHasher = new PasswordHasher(new V2PasswordHasherOptions()); - public BackOfficePasswordHasher(LegacyPasswordSecurity passwordSecurity, IJsonSerializer jsonSerializer) + : base(passwordSecurity, jsonSerializer) { - _legacyPasswordSecurity = passwordSecurity; - _jsonSerializer = jsonSerializer; - } - - public override string HashPassword(BackOfficeIdentityUser user, string password) - { - // Always use the latest/current hash algorithm when hashing new passwords for storage. - // NOTE: This is only overridden to show that we can since we may need to adjust this in the future - // if new/different formats are required. - return base.HashPassword(user, password); - } - - /// - /// Verifies a user's hashed password - /// - /// - /// - /// - /// - /// - /// This will check the user's current hashed password format stored with their user row and use that to verify the hash. This could be any hashes - /// from the very old v4, to the older v6-v8, to the older aspnet identity and finally to the most recent - /// - 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 (_legacyPasswordSecurity.SupportHashAlgorithm(deserialized.HashAlgorithm)) - { - var result = _legacyPasswordSecurity.VerifyPassword(deserialized.HashAlgorithm, providedPassword, hashedPassword); - return result - ? PasswordVerificationResult.SuccessRehashNeeded - : PasswordVerificationResult.Failed; - } - - // We will explicitly detect names here - // 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.VerifyHashedPassword(user, hashedPassword, providedPassword); - case Constants.Security.AspNetCoreV2PasswordHashAlgorithmName: - var legacyResult = _aspnetV2PasswordHasher.VerifyHashedPassword(user, hashedPassword, providedPassword); - if (legacyResult == PasswordVerificationResult.Success) - return PasswordVerificationResult.SuccessRehashNeeded; - return legacyResult; - } - } - - // else go the default (v3) - return base.VerifyHashedPassword(user, hashedPassword, providedPassword); - } - - private class V2PasswordHasherOptions : IOptions - { - public PasswordHasherOptions Value => new PasswordHasherOptions - { - CompatibilityMode = PasswordHasherCompatibilityMode.IdentityV2 - }; } } } diff --git a/src/Umbraco.Infrastructure/Security/MemberPasswordHasher.cs b/src/Umbraco.Infrastructure/Security/MemberPasswordHasher.cs index 6a0f2f21e2..2d2cad1624 100644 --- a/src/Umbraco.Infrastructure/Security/MemberPasswordHasher.cs +++ b/src/Umbraco.Infrastructure/Security/MemberPasswordHasher.cs @@ -1,7 +1,10 @@ using System; using Microsoft.AspNetCore.Identity; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Core.Serialization; +using Umbraco.Extensions; namespace Umbraco.Cms.Core.Security { @@ -11,11 +14,12 @@ namespace Umbraco.Cms.Core.Security /// /// This will check for the ASP.NET Identity password hash flag before falling back to the legacy password hashing format ("HMACSHA256") /// - public class MemberPasswordHasher : PasswordHasher + public class MemberPasswordHasher : UmbracoPasswordHasher { - private readonly LegacyPasswordSecurity _legacyPasswordHasher; - - public MemberPasswordHasher(LegacyPasswordSecurity legacyPasswordHasher) => _legacyPasswordHasher = legacyPasswordHasher ?? throw new ArgumentNullException(nameof(legacyPasswordHasher)); + public MemberPasswordHasher(LegacyPasswordSecurity legacyPasswordHasher, IJsonSerializer jsonSerializer) + : base(legacyPasswordHasher, jsonSerializer) + { + } /// /// Verifies a user's hashed password @@ -27,6 +31,20 @@ namespace Umbraco.Cms.Core.Security /// Thrown when the correct hashing algorith cannot be determined public override PasswordVerificationResult VerifyHashedPassword(MemberIdentityUser user, string hashedPassword, string providedPassword) { + if (user is null) + { + throw new ArgumentNullException(nameof(user)); + } + + // if there's password config use the base implementation + if (!user.PasswordConfig.IsNullOrWhiteSpace()) + { + return base.VerifyHashedPassword(user, hashedPassword, providedPassword); + } + + // Else we need to detect what the password is. This will be the case + // for upgrades since no password config will exist. + byte[] decodedHashedPassword = null; bool isAspNetIdentityHash = false; @@ -51,7 +69,7 @@ namespace Umbraco.Cms.Core.Security throw new InvalidOperationException("unable to determine member password hashing algorith"); } - var isValid = _legacyPasswordHasher.VerifyPassword( + var isValid = LegacyPasswordSecurity.VerifyPassword( Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName, providedPassword, hashedPassword); diff --git a/src/Umbraco.Infrastructure/Security/MemberRoleStore.cs b/src/Umbraco.Infrastructure/Security/MemberRoleStore.cs index a87b3c7f7e..685a21d175 100644 --- a/src/Umbraco.Infrastructure/Security/MemberRoleStore.cs +++ b/src/Umbraco.Infrastructure/Security/MemberRoleStore.cs @@ -224,12 +224,17 @@ namespace Umbraco.Cms.Core.Security /// private UmbracoIdentityRole MapFromMemberGroup(IMemberGroup memberGroup) { + // NOTE: there is a ConcurrencyStamp property but we don't use it. The purpose + // of this value is to try to prevent concurrent writes in the DB but this is + // an implementation detail at the data source level that has leaked into the + // model. A good writeup of that is here: + // https://stackoverflow.com/a/37362173 + // For our purposes currently we won't worry about this. + var result = new UmbracoIdentityRole { Id = memberGroup.Id.ToString(), Name = memberGroup.Name - // TODO: Implement this functionality, requires DB and logic updates - //ConcurrencyStamp }; return result; } @@ -247,8 +252,6 @@ namespace Umbraco.Cms.Core.Security if (role.IsPropertyDirty(nameof(UmbracoIdentityRole.Name)) && !string.IsNullOrEmpty(role.Name) && memberGroup.Name != role.Name) { - // TODO: Need to support ConcurrencyStamp and logic - memberGroup.Name = role.Name; anythingChanged = true; } diff --git a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs index 5329c64a0e..81b0ee126e 100644 --- a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs @@ -611,6 +611,10 @@ namespace Umbraco.Cms.Core.Security public IPublishedContent GetPublishedMember(MemberIdentityUser user) { + if (user == null) + { + return null; + } IMember member = _memberService.GetByKey(user.Key); if (member == null) { diff --git a/src/Umbraco.Infrastructure/Security/UmbracoIdentityRole.cs b/src/Umbraco.Infrastructure/Security/UmbracoIdentityRole.cs index ccf9448604..0b0285d852 100644 --- a/src/Umbraco.Infrastructure/Security/UmbracoIdentityRole.cs +++ b/src/Umbraco.Infrastructure/Security/UmbracoIdentityRole.cs @@ -57,7 +57,12 @@ namespace Umbraco.Cms.Core.Security /// public bool HasIdentity { get; protected set; } - // TODO: We should support this and it's logic + // NOTE: The purpose + // of this value is to try to prevent concurrent writes in the DB but this is + // an implementation detail at the data source level that has leaked into the + // model. A good writeup of that is here: + // https://stackoverflow.com/a/37362173 + // For our purposes currently we won't worry about this. public override string ConcurrencyStamp { get => base.ConcurrencyStamp; set => base.ConcurrencyStamp = value; } /// diff --git a/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs b/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs index f91fac1549..05e70076c6 100644 --- a/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs +++ b/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs @@ -69,6 +69,14 @@ namespace Umbraco.Cms.Core.Security } } + // NOTE: The purpose + // of this value is to try to prevent concurrent writes in the DB but this is + // an implementation detail at the data source level that has leaked into the + // model. A good writeup of that is here: + // https://stackoverflow.com/a/37362173 + // For our purposes currently we won't worry about this. + public override string ConcurrencyStamp { get => base.ConcurrencyStamp; set => base.ConcurrencyStamp = value; } + /// /// Gets or sets last login date /// diff --git a/src/Umbraco.Infrastructure/Security/UmbracoPasswordHasher.cs b/src/Umbraco.Infrastructure/Security/UmbracoPasswordHasher.cs new file mode 100644 index 0000000000..73d6d2b025 --- /dev/null +++ b/src/Umbraco.Infrastructure/Security/UmbracoPasswordHasher.cs @@ -0,0 +1,92 @@ +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Serialization; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Core.Security +{ + public class UmbracoPasswordHasher : PasswordHasher + where TUser: UmbracoIdentityUser + { + private readonly IJsonSerializer _jsonSerializer; + private readonly PasswordHasher _aspnetV2PasswordHasher = new PasswordHasher(new V2PasswordHasherOptions()); + + public UmbracoPasswordHasher(LegacyPasswordSecurity legacyPasswordSecurity, IJsonSerializer jsonSerializer) + { + LegacyPasswordSecurity = legacyPasswordSecurity ?? throw new System.ArgumentNullException(nameof(legacyPasswordSecurity)); + _jsonSerializer = jsonSerializer ?? throw new System.ArgumentNullException(nameof(jsonSerializer)); + } + + public LegacyPasswordSecurity LegacyPasswordSecurity { get; } + + public override string HashPassword(TUser user, string password) + { + // Always use the latest/current hash algorithm when hashing new passwords for storage. + // NOTE: This is only overridden to show that we can since we may need to adjust this in the future + // if new/different formats are required. + return base.HashPassword(user, password); + } + + /// + /// Verifies a user's hashed password + /// + /// + /// + /// + /// + /// + /// This will check the user's current hashed password format stored with their user row and use that to verify the hash. This could be any hashes + /// from the very old v4, to the older v6-v8, to the older aspnet identity and finally to the most recent + /// + public override PasswordVerificationResult VerifyHashedPassword(TUser user, string hashedPassword, string providedPassword) + { + if (user is null) + { + throw new System.ArgumentNullException(nameof(user)); + } + + 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 (LegacyPasswordSecurity.SupportHashAlgorithm(deserialized.HashAlgorithm)) + { + var result = LegacyPasswordSecurity.VerifyPassword(deserialized.HashAlgorithm, providedPassword, hashedPassword); + return result + ? PasswordVerificationResult.SuccessRehashNeeded + : PasswordVerificationResult.Failed; + } + + // We will explicitly detect names here + // 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.VerifyHashedPassword(user, hashedPassword, providedPassword); + case Constants.Security.AspNetCoreV2PasswordHashAlgorithmName: + var legacyResult = _aspnetV2PasswordHasher.VerifyHashedPassword(user, hashedPassword, providedPassword); + if (legacyResult == PasswordVerificationResult.Success) + return PasswordVerificationResult.SuccessRehashNeeded; + return legacyResult; + } + } + + // else go the default (v3) + return base.VerifyHashedPassword(user, hashedPassword, providedPassword); + } + + private class V2PasswordHasherOptions : IOptions + { + public PasswordHasherOptions Value => new PasswordHasherOptions + { + CompatibilityMode = PasswordHasherCompatibilityMode.IdentityV2 + }; + } + } +} diff --git a/src/Umbraco.Tests.Common/Builders/MemberBuilder.cs b/src/Umbraco.Tests.Common/Builders/MemberBuilder.cs index fd6e272fc4..c192b4cc07 100644 --- a/src/Umbraco.Tests.Common/Builders/MemberBuilder.cs +++ b/src/Umbraco.Tests.Common/Builders/MemberBuilder.cs @@ -116,6 +116,7 @@ namespace Umbraco.Cms.Tests.Common.Builders DateTime lastLockoutDate = _lastLockoutDate ?? DateTime.Now; DateTime lastLoginDate = _lastLoginDate ?? DateTime.Now; DateTime lastPasswordChangeDate = _lastPasswordChangeDate ?? DateTime.Now; + var passwordConfig = _passwordConfig ?? "{\"hashAlgorithm\":\"PBKDF2.ASPNETCORE.V3\"}"; if (_memberTypeBuilder is null && _memberType is null) { @@ -135,6 +136,7 @@ namespace Umbraco.Cms.Tests.Common.Builders Path = path, SortOrder = sortOrder, Trashed = trashed, + PasswordConfiguration = passwordConfig }; if (_propertyIdsIncrementingFrom.HasValue) diff --git a/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs b/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs index 39afb391aa..635a17a2b1 100644 --- a/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs +++ b/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs @@ -18,7 +18,6 @@ using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Composing; using Umbraco.Cms.Core.DependencyInjection; -using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.DependencyInjection; @@ -181,8 +180,11 @@ namespace Umbraco.Cms.Tests.Integration.TestServerTest public override void Configure(IApplicationBuilder app) { app.UseUmbraco() - .WithBackOffice() - .WithWebsite() + .WithMiddleware(u => + { + u.WithBackOffice(); + u.WithWebsite(); + }) .WithEndpoints(u => { u.UseBackOfficeEndpoints(); diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberRepositoryTest.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberRepositoryTest.cs index ccfd6fdcf8..57d64638fb 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberRepositoryTest.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/MemberRepositoryTest.cs @@ -6,11 +6,14 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Moq; using NPoco; using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Configuration; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence.Querying; @@ -54,7 +57,23 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence.Repos IRelationRepository relationRepository = GetRequiredService(); var propertyEditors = new Lazy(() => new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty()))); var dataValueReferences = new DataValueReferenceFactoryCollection(Enumerable.Empty()); - return new MemberRepository(accessor, AppCaches.Disabled, LoggerFactory.CreateLogger(), MemberTypeRepository, MemberGroupRepository, tagRepo, Mock.Of(), relationRepository, relationTypeRepository, PasswordHasher, propertyEditors, dataValueReferences, DataTypeService, JsonSerializer, Mock.Of()); + return new MemberRepository( + accessor, + AppCaches.Disabled, + LoggerFactory.CreateLogger(), + MemberTypeRepository, + MemberGroupRepository, + tagRepo, + Mock.Of(), + relationRepository, + relationTypeRepository, + PasswordHasher, + propertyEditors, + dataValueReferences, + DataTypeService, + JsonSerializer, + Mock.Of(), + Options.Create(new MemberPasswordConfigurationSettings())); } [Test] diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Security/MemberPasswordHasherTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberPasswordHasherTests.cs similarity index 81% rename from src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Security/MemberPasswordHasherTests.cs rename to src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberPasswordHasherTests.cs index 967b628a10..3dd2826baa 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Security/MemberPasswordHasherTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberPasswordHasherTests.cs @@ -3,13 +3,14 @@ using Microsoft.AspNetCore.Identity; using NUnit.Framework; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Infrastructure.Security; +using Umbraco.Cms.Infrastructure.Serialization; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Security { [TestFixture] public class MemberPasswordHasherTests { - private MemberPasswordHasher CreateSut() => new MemberPasswordHasher(new LegacyPasswordSecurity()); + private MemberPasswordHasher CreateSut() => new MemberPasswordHasher(new LegacyPasswordSecurity(), new JsonNetSerializer()); [Test] public void VerifyHashedPassword_GivenAnAspNetIdentity2PasswordHash_ThenExpectSuccessRehashNeeded() @@ -18,7 +19,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Security const string hash = "AJszAsQqxOYbASKfL3JVUu6cjU18ouizXDfX4j7wLlir8SWj2yQaTepE9e5bIohIsQ=="; var sut = CreateSut(); - var result = sut.VerifyHashedPassword(null, hash, password); + var result = sut.VerifyHashedPassword(new MemberIdentityUser(), hash, password); Assert.AreEqual(result, PasswordVerificationResult.SuccessRehashNeeded); } @@ -30,7 +31,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Security const string hash = "AQAAAAEAACcQAAAAEGF/tTVoL6ef3bQPZFYfbgKFu1CDQIAMgyY1N4EDt9jqdG/hsOX93X1U6LNvlIQ3mw=="; var sut = CreateSut(); - var result = sut.VerifyHashedPassword(null, hash, password); + var result = sut.VerifyHashedPassword(new MemberIdentityUser(), hash, password); Assert.AreEqual(result, PasswordVerificationResult.Success); } @@ -42,7 +43,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Security const string hash = "yDiU2YyuYZU4jz6F0fpErQ==BxNRHkXBVyJs9gwWF6ktWdfDwYf5bwm+rvV7tOcNNx8="; var sut = CreateSut(); - var result = sut.VerifyHashedPassword(null, hash, password); + var result = sut.VerifyHashedPassword(new MemberIdentityUser(), hash, password); Assert.AreEqual(result, PasswordVerificationResult.SuccessRehashNeeded); } @@ -54,7 +55,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Security var hash = Convert.ToBase64String(hashBytes); var sut = CreateSut(); - Assert.Throws(() => sut.VerifyHashedPassword(null, hash, "password")); + Assert.Throws(() => sut.VerifyHashedPassword(new MemberIdentityUser(), hash, "password")); } [TestCase("AJszAsQqxOYbASKfL3JVUu6cjU18ouizXDfX4j7wLlir8SWj2yQaTepE9e5bIohIsQ==")] @@ -65,7 +66,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Security const string invalidPassword = "nope"; var sut = CreateSut(); - var result = sut.VerifyHashedPassword(null, hash, invalidPassword); + var result = sut.VerifyHashedPassword(new MemberIdentityUser(), hash, invalidPassword); Assert.AreEqual(result, PasswordVerificationResult.Failed); } diff --git a/src/Umbraco.Web.BackOffice/Extensions/UmbracoApplicationBuilder.BackOffice.cs b/src/Umbraco.Web.BackOffice/Extensions/UmbracoApplicationBuilder.BackOffice.cs index dc113a99b0..59cefa0574 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/UmbracoApplicationBuilder.BackOffice.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/UmbracoApplicationBuilder.BackOffice.cs @@ -21,7 +21,7 @@ namespace Umbraco.Extensions /// /// /// - public static IUmbracoApplicationBuilder WithBackOffice(this IUmbracoApplicationBuilder builder) + public static IUmbracoMiddlewareBuilder WithBackOffice(this IUmbracoMiddlewareBuilder builder) { KeepAliveSettings keepAliveSettings = builder.ApplicationServices.GetRequiredService>().Value; IHostingEnvironment hostingEnvironment = builder.ApplicationServices.GetRequiredService(); diff --git a/src/Umbraco.Web.Common/ApplicationBuilder/IUmbracoApplicationBuilder.cs b/src/Umbraco.Web.Common/ApplicationBuilder/IUmbracoApplicationBuilder.cs index 34abdf70bd..68ba148f49 100644 --- a/src/Umbraco.Web.Common/ApplicationBuilder/IUmbracoApplicationBuilder.cs +++ b/src/Umbraco.Web.Common/ApplicationBuilder/IUmbracoApplicationBuilder.cs @@ -1,14 +1,17 @@ -using System; +using System; using Microsoft.AspNetCore.Builder; using Umbraco.Cms.Core.Services; namespace Umbraco.Cms.Web.Common.ApplicationBuilder { - public interface IUmbracoApplicationBuilder + public interface IUmbracoApplicationBuilder : IUmbracoMiddlewareBuilder { - IRuntimeState RuntimeState { get; } - IServiceProvider ApplicationServices { get; } - IApplicationBuilder AppBuilder { get; } + /// + /// Called to include umbraco middleware + /// + /// + /// + IUmbracoApplicationBuilder WithMiddleware(Action configureUmbraco); /// /// Final call during app building to configure endpoints diff --git a/src/Umbraco.Web.Common/ApplicationBuilder/IUmbracoEndpointBuilder.cs b/src/Umbraco.Web.Common/ApplicationBuilder/IUmbracoEndpointBuilder.cs index 4db74dea75..31507477ae 100644 --- a/src/Umbraco.Web.Common/ApplicationBuilder/IUmbracoEndpointBuilder.cs +++ b/src/Umbraco.Web.Common/ApplicationBuilder/IUmbracoEndpointBuilder.cs @@ -1,7 +1,4 @@ -using System; -using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Routing; -using Umbraco.Cms.Core.Services; namespace Umbraco.Cms.Web.Common.ApplicationBuilder { @@ -9,11 +6,8 @@ namespace Umbraco.Cms.Web.Common.ApplicationBuilder /// /// A builder to allow encapsulating the enabled routing features in Umbraco /// - public interface IUmbracoEndpointBuilder - { - IRuntimeState RuntimeState { get; } - IServiceProvider ApplicationServices { get; } - IEndpointRouteBuilder EndpointRouteBuilder { get; } - IApplicationBuilder AppBuilder { get; } + public interface IUmbracoEndpointBuilder : IUmbracoMiddlewareBuilder + { + IEndpointRouteBuilder EndpointRouteBuilder { get; } } } diff --git a/src/Umbraco.Web.Common/ApplicationBuilder/IUmbracoMiddlewareBuilder.cs b/src/Umbraco.Web.Common/ApplicationBuilder/IUmbracoMiddlewareBuilder.cs new file mode 100644 index 0000000000..78d7f28ab9 --- /dev/null +++ b/src/Umbraco.Web.Common/ApplicationBuilder/IUmbracoMiddlewareBuilder.cs @@ -0,0 +1,13 @@ +using System; +using Microsoft.AspNetCore.Builder; +using Umbraco.Cms.Core.Services; + +namespace Umbraco.Cms.Web.Common.ApplicationBuilder +{ + public interface IUmbracoMiddlewareBuilder + { + IRuntimeState RuntimeState { get; } + IServiceProvider ApplicationServices { get; } + IApplicationBuilder AppBuilder { get; } + } +} diff --git a/src/Umbraco.Web.Common/ApplicationBuilder/UmbracoApplicationBuilder.cs b/src/Umbraco.Web.Common/ApplicationBuilder/UmbracoApplicationBuilder.cs index 2bef61bbab..05fc38cc71 100644 --- a/src/Umbraco.Web.Common/ApplicationBuilder/UmbracoApplicationBuilder.cs +++ b/src/Umbraco.Web.Common/ApplicationBuilder/UmbracoApplicationBuilder.cs @@ -1,6 +1,7 @@ -using System; +using System; using Microsoft.AspNetCore.Builder; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Services; namespace Umbraco.Cms.Web.Common.ApplicationBuilder @@ -21,13 +22,44 @@ namespace Umbraco.Cms.Web.Common.ApplicationBuilder public IRuntimeState RuntimeState { get; } public IApplicationBuilder AppBuilder { get; } + public IUmbracoApplicationBuilder WithMiddleware(Action configureUmbraco) + { + IOptions startupOptions = ApplicationServices.GetRequiredService>(); + RunPostPipeline(startupOptions.Value); + + configureUmbraco(this); + + return this; + } + public void WithEndpoints(Action configureUmbraco) - => AppBuilder.UseEndpoints(endpoints => - { - var umbAppBuilder = (IUmbracoEndpointBuilder)ActivatorUtilities.CreateInstance( - ApplicationServices, - new object[] { AppBuilder, endpoints }); - configureUmbraco(umbAppBuilder); - }); + { + IOptions startupOptions = ApplicationServices.GetRequiredService>(); + RunPreEndpointsPipeline(startupOptions.Value); + + AppBuilder.UseEndpoints(endpoints => + { + var umbAppBuilder = (IUmbracoEndpointBuilder)ActivatorUtilities.CreateInstance( + ApplicationServices, + new object[] { AppBuilder, endpoints }); + configureUmbraco(umbAppBuilder); + }); + } + + private void RunPostPipeline(UmbracoPipelineOptions startupOptions) + { + foreach (IUmbracoPipelineFilter filter in startupOptions.PipelineFilters) + { + filter.OnPostPipeline(AppBuilder); + } + } + + private void RunPreEndpointsPipeline(UmbracoPipelineOptions startupOptions) + { + foreach (IUmbracoPipelineFilter filter in startupOptions.PipelineFilters) + { + filter.OnEndpoints(AppBuilder); + } + } } } diff --git a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilder.MembersIdentity.cs b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilder.MembersIdentity.cs index be312a920f..c6856f8f19 100644 --- a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilder.MembersIdentity.cs +++ b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilder.MembersIdentity.cs @@ -1,5 +1,3 @@ -using System; -using System.Linq; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.DependencyInjection; @@ -50,15 +48,7 @@ namespace Umbraco.Extensions services.AddScoped, MemberPasswordHasher>(); services.ConfigureOptions(); - - services.ConfigureApplicationCookie(x => - { - // TODO: We may want/need to configure these further - - x.LoginPath = null; - x.AccessDeniedPath = null; - x.LogoutPath = null; - }); + services.ConfigureOptions(); return builder; } diff --git a/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs b/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs index 1195f7dbac..fb12e979ab 100644 --- a/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/ApplicationBuilderExtensions.cs @@ -72,9 +72,6 @@ namespace Umbraco.Extensions // DO NOT PUT ANY UseEndpoints declarations here!! Those must all come very last in the pipeline, // endpoints are terminating middleware. All of our endpoints are declared in ext of IUmbracoApplicationBuilder - app.RunPostPipeline(startupOptions.Value); - app.RunPreEndpointsPipeline(startupOptions.Value); - return ActivatorUtilities.CreateInstance( app.ApplicationServices, new object[] { app }); @@ -88,22 +85,6 @@ namespace Umbraco.Extensions } } - private static void RunPostPipeline(this IApplicationBuilder app, UmbracoPipelineOptions startupOptions) - { - foreach (IUmbracoPipelineFilter filter in startupOptions.PipelineFilters) - { - filter.OnPostPipeline(app); - } - } - - private static void RunPreEndpointsPipeline(this IApplicationBuilder app, UmbracoPipelineOptions startupOptions) - { - foreach (IUmbracoPipelineFilter filter in startupOptions.PipelineFilters) - { - filter.OnEndpoints(app); - } - } - /// /// Returns true if Umbraco is greater than /// diff --git a/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs b/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs index 24a5b01832..ac45c932da 100644 --- a/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs +++ b/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs @@ -7,14 +7,12 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Web.Common.Security { - // TODO: This is only for the back office, does it need to be in common? - public class BackOfficeSecurity : IBackOfficeSecurity { private readonly IUserService _userService; private readonly IHttpContextAccessor _httpContextAccessor; - private object _currentUserLock = new object(); + private readonly object _currentUserLock = new object(); private IUser _currentUser; public BackOfficeSecurity( diff --git a/src/Umbraco.Web.Common/Security/ConfigureMemberCookieOptions.cs b/src/Umbraco.Web.Common/Security/ConfigureMemberCookieOptions.cs new file mode 100644 index 0000000000..ba5f0621b9 --- /dev/null +++ b/src/Umbraco.Web.Common/Security/ConfigureMemberCookieOptions.cs @@ -0,0 +1,39 @@ +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Routing; +using Umbraco.Cms.Core.Services; + +namespace Umbraco.Cms.Web.Common.Security +{ + public sealed class ConfigureMemberCookieOptions : IConfigureNamedOptions + { + private readonly IRuntimeState _runtimeState; + private readonly UmbracoRequestPaths _umbracoRequestPaths; + + public ConfigureMemberCookieOptions(IRuntimeState runtimeState, UmbracoRequestPaths umbracoRequestPaths) + { + _runtimeState = runtimeState; + _umbracoRequestPaths = umbracoRequestPaths; + } + + public void Configure(string name, CookieAuthenticationOptions options) + { + if (name == IdentityConstants.ApplicationScheme || name == IdentityConstants.ExternalScheme) + { + Configure(options); + } + } + + public void Configure(CookieAuthenticationOptions options) + { + // TODO: We may want/need to configure these further + + options.LoginPath = null; + options.AccessDeniedPath = null; + options.LogoutPath = null; + + options.CookieManager = new MemberCookieManager(_runtimeState, _umbracoRequestPaths); + } + } +} diff --git a/src/Umbraco.Web.Common/Security/ConfigureMemberIdentityOptions.cs b/src/Umbraco.Web.Common/Security/ConfigureMemberIdentityOptions.cs index 3abe5f0428..cc19670f83 100644 --- a/src/Umbraco.Web.Common/Security/ConfigureMemberIdentityOptions.cs +++ b/src/Umbraco.Web.Common/Security/ConfigureMemberIdentityOptions.cs @@ -6,14 +6,13 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Web.Common.Security { + public sealed class ConfigureMemberIdentityOptions : IConfigureOptions { private readonly MemberPasswordConfigurationSettings _memberPasswordConfiguration; public ConfigureMemberIdentityOptions(IOptions memberPasswordConfiguration) - { - _memberPasswordConfiguration = memberPasswordConfiguration.Value; - } + => _memberPasswordConfiguration = memberPasswordConfiguration.Value; public void Configure(IdentityOptions options) { diff --git a/src/Umbraco.Web.Common/Security/MemberCookieManager.cs b/src/Umbraco.Web.Common/Security/MemberCookieManager.cs new file mode 100644 index 0000000000..9e176d15c1 --- /dev/null +++ b/src/Umbraco.Web.Common/Security/MemberCookieManager.cs @@ -0,0 +1,70 @@ +using System; +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.Http; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Routing; +using Umbraco.Cms.Core.Services; + +namespace Umbraco.Cms.Web.Common.Security +{ + /// + /// A custom cookie manager for members to ensure that cookie auth does not occur for any back office requests + /// + public class MemberCookieManager : ChunkingCookieManager, ICookieManager + { + private readonly IRuntimeState _runtime; + private readonly UmbracoRequestPaths _umbracoRequestPaths; + + public MemberCookieManager(IRuntimeState runtime, UmbracoRequestPaths umbracoRequestPaths) + { + _runtime = runtime ?? throw new ArgumentNullException(nameof(runtime)); + _umbracoRequestPaths = umbracoRequestPaths ?? throw new ArgumentNullException(nameof(umbracoRequestPaths)); + } + + /// + /// Determines if we should authenticate the request + /// + /// true if the request should be authenticated + /// + /// We auth the request when it is not a back office request and when the runtime level is Run + /// + public bool ShouldAuthenticateRequest(string absPath) + { + // Do not authenticate the request if we are not running. + // Else this can cause problems especially if the members DB table needs upgrades + // because when authing, the member db table will be read and we'll get exceptions. + if (_runtime.Level != RuntimeLevel.Run) + { + return false; + } + + if (// check back office + _umbracoRequestPaths.IsBackOfficeRequest(absPath) + + // check installer + || _umbracoRequestPaths.IsInstallerRequest(absPath)) + { + return false; + } + + return true; + } + + /// + /// Explicitly implement this so that we filter the request + /// + /// + string ICookieManager.GetRequestCookie(HttpContext context, string key) + { + var absPath = context.Request.Path; + + return ShouldAuthenticateRequest(absPath) == false + + // Don't auth request, don't return a cookie + ? null + + // Return the default implementation + : GetRequestCookie(context, key); + } + } +} diff --git a/src/Umbraco.Web.Common/UmbracoHelper.cs b/src/Umbraco.Web.Common/UmbracoHelper.cs index 48de01b74c..98f2d08fdd 100644 --- a/src/Umbraco.Web.Common/UmbracoHelper.cs +++ b/src/Umbraco.Web.Common/UmbracoHelper.cs @@ -108,9 +108,7 @@ namespace Umbraco.Cms.Web.Common /// The alias. /// public async Task RenderMacroAsync(string alias) - { - return await _componentRenderer.RenderMacroAsync(AssignedContentItem?.Id ?? 0, alias, null); - } + => await _componentRenderer.RenderMacroAsync(AssignedContentItem?.Id ?? 0, alias, null); /// /// Renders the macro with the specified alias, passing in the specified parameters. @@ -119,9 +117,7 @@ namespace Umbraco.Cms.Web.Common /// The parameters. /// public async Task RenderMacroAsync(string alias, object parameters) - { - return await _componentRenderer.RenderMacroAsync(AssignedContentItem?.Id ?? 0, alias, parameters?.ToDictionary()); - } + => await _componentRenderer.RenderMacroAsync(AssignedContentItem?.Id ?? 0, alias, parameters?.ToDictionary()); /// /// Renders the macro with the specified alias, passing in the specified parameters. @@ -130,9 +126,7 @@ namespace Umbraco.Cms.Web.Common /// The parameters. /// public async Task RenderMacroAsync(string alias, IDictionary parameters) - { - return await _componentRenderer.RenderMacroAsync(AssignedContentItem?.Id ?? 0, alias, parameters); - } + => await _componentRenderer.RenderMacroAsync(AssignedContentItem?.Id ?? 0, alias, parameters); #endregion diff --git a/src/Umbraco.Web.UI.NetCore/Startup.cs b/src/Umbraco.Web.UI.NetCore/Startup.cs index 38db763230..2f97a15d8f 100644 --- a/src/Umbraco.Web.UI.NetCore/Startup.cs +++ b/src/Umbraco.Web.UI.NetCore/Startup.cs @@ -60,8 +60,11 @@ namespace Umbraco.Cms.Web.UI.NetCore } app.UseUmbraco() - .WithBackOffice() - .WithWebsite() + .WithMiddleware(u => + { + u.WithBackOffice(); + u.WithWebsite(); + }) .WithEndpoints(u => { u.UseInstallerEndpoints(); diff --git a/src/Umbraco.Web.Website/Extensions/UmbracoApplicationBuilder.Website.cs b/src/Umbraco.Web.Website/Extensions/UmbracoApplicationBuilder.Website.cs index 54c4ab4186..10cbbfdc4d 100644 --- a/src/Umbraco.Web.Website/Extensions/UmbracoApplicationBuilder.Website.cs +++ b/src/Umbraco.Web.Website/Extensions/UmbracoApplicationBuilder.Website.cs @@ -17,7 +17,7 @@ namespace Umbraco.Extensions /// /// /// - public static IUmbracoApplicationBuilder WithWebsite(this IUmbracoApplicationBuilder builder) + public static IUmbracoMiddlewareBuilder WithWebsite(this IUmbracoMiddlewareBuilder builder) { builder.AppBuilder.UseMiddleware(); return builder;